Re: [lmi] Converting std::string to wxString

[Vadim Zeitlin]

On Mon, 6 Feb 2017 19:37:57 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2017-02-06 17:50, Vadim Zeitlin wrote:
GC> > On Mon, 6 Feb 2017 16:57:16 +0000 Greg Chicares <address@hidden> wrote:
GC> [...]
GC> > GC> But let me ask this: if I just do whatever seems simplest to me, can I
GC> > GC> trust the compiler to give a warning whenever I've oversimplified?
GC> > GC> "Code for simplicity, but don't commit anything that doesn't compile"
GC> > GC> is a rule I can memorize.
GC> [...]
GC> >  I actually do have a solution for this: we've been asked many times 
(maybe
GC> > even by you too?) to remove implicit wxString conversions to "const char*"
GC> > and "const wchar_t*" in the default build or to "std::string" and
GC> > "std::wstring" in the wxUSE_STL==1 one. We can't do it by default because
GC> > it would break too much existing code, especially in !wxUSE_STL case, but 
I
GC> > agree that it would be a good idea to allow opting in to this safer
GC> > behaviour, e.g. allow predefining some wxDISABLE_IMPLICIT_WXSTRING_CONV to
GC> > ensure that the code implicitly converting wxString to std::string doesn't
GC> > compile. If we did this, then your rule above would work without any
GC> > exceptions -- but only if/when you updated to the latest wxWidgets 
version.
GC> > 
GC> >  Please let me know if this is something you'd be ready to do.
GC> 
GC> Yes.

 Hello again,

 I've done this now in wxWidgets with this change:

https://github.com/wxWidgets/wxWidgets/commit/e16d899f6b9206a692f6dd8fa8cc1e580d26920b

which introduces wxUSE_UNSAFE_WXSTRING_CONV, setting which to 0 allows to
disable unsafe implicit wxString conversions to "char*" or std::string at
library build time, and wxNO_UNSAFE_WXSTRING_CONV, which can be defined
during application build to do the same thing, regardless of how the
library itself was built. I think the latter will be more useful in
general and it would definitely be more convenient for me (because I use
wxWidgets for several different projects, not all of which can be compiled
without these implicit conversions), so I'd like to just add
-DwxNO_UNSAFE_WXSTRING_CONV to REQUIRED_CPPFLAGS in workhorse.make. But if
(as I suspect) you prefer to do it at the library level, it is also
possible to add --disable-unsafe_conv_in_wxstring to config_options in
install_wx.make.

 But before doing either of these, you will also need to apply the changes
at https://github.com/vadz/lmi/pull/54 or similar because currently lmi
wouldn't compile without these implicit conversions as it does rely on them
in a few places. In a couple of those, we don't actually need conversions
at all and it's better to just change the type of the variables used (as
usual, I explained why in the commit messages, but please let me know if
you need more details). For the rest of them, I've added explicit calls to
wxString::ToStdString(). Please note that I used wxConvUTF8 for them: this
is definitely the right thing to do, as any other conversion (well, except
UTF-7, but nobody uses this one any more) would still be susceptible to
data loss if wxString contained a character not representable in the
current locale encoding. However this is also definitely not all we need to
do because we also need to handle these UTF-8 encoded strings correctly.
I.e. with this change, entering some Unicode symbol into the GUI won't
result in completely losing it *and* everything else in the same string,
but it could still mangle it. I know that lmi doesn't use Unicode, so maybe
you don't consider this to be a problem, but I think that it's quite
important to fix this too, especially as nothing prevents users from
entering or pasting (maybe even accidentally) Unicode strings in the GUI,
so please let me know if I should spend some time on straightening this
out.

 Finally, please note that PR 54 can be applied even right now, i.e. while
it is required before wxNO_UNSAFE_WXSTRING_CONV can be enabled, applying it
now won't break anything and at least the two first commits in it are
useful in their own right.

 Please let me know if you have any questions about all this,
VZ

P.S. I'll reply to xmlwrapp-related part separately later.