wxWidgets icon indicating copy to clipboard operation
wxWidgets copied to clipboard

Default to STL builds, deprecate non-STL

Open StefanBruens opened this issue 3 years ago • 4 comments

Currently, the build defaults to non-STL, but this historical default should be revised.

https://github.com/wxWidgets/wxWidgets/blob/974a7658da8da235b205618924db0df345ab5189/build/cmake/options.cmake#L79

Reasoning for the change:

  • Use of std::string is encouraged for application use already, so the STL typically comes for free: https://github.com/wxWidgets/wxWidgets/blob/974a7658da8da235b205618924db0df345ab5189/interface/wx/string.h#L14-L18
  • wxString behaviour/API differs slightly between STL/Non-STL (e.g. implicit char* conversion), and code must be written to cope with both variants.

IMHO, the non-STL build does not provide any benefit today, but adds to code-complexity.

I propose to:

  1. Change default to STL (immediately, wx 3.3 dev)
  2. Deprecate non-STL (immediately, wx 3.3 dev)
  3. Remove non-STL (e.g. wx 3.4.0)

StefanBruens avatar Aug 29 '22 19:08 StefanBruens

I thought about doing this many times before, but unfortunately this just breaks way too much existing code to be practical. With some rare exceptions (typically, applications that already use --enable-stl explicitly), any non-trivial wx program is going to have hundreds if not thousands of places relying on implicit conversion of wxString to const char*. Just try compiling any of them with wxNO_IMPLICIT_WXSTRING_ENCODING defined.

The only way forward is to add a new API integrating with the existing one but not compatible with it (a.k.a. wxTNG, a.k.a. wyWidgets, a.k.a. mythical unicorn talked about for 20 years but that nobody has ever seen). But we're not going to break all the existing code using wx, not even in 3.4.

vadz avatar Aug 29 '22 19:08 vadz

@vadz I think your assumption is incorrect.

On Tumbleweed, we build various ports of wxWidgets, and the following packages use a GTK 3.0 version with STL: 4pane, CorsixTH, DVDStyler, FreeFileSync, PlayOnLinux, PrusaSlicer, aegisub, amanda, codelite, cwstudio, diff-pdf, elixir, erlang, erlang-rebar-testsuite, espeakedit, fityk, fwknop-gui, gnuplot, grass, hugin, kicad, leptonica, limesuite, mathgl, mediainfo, megaglest, meson:test, openbabel, paraview, plee-the-bear, plplot, poedit, python-matplotlib:test, python-pyface, python-pyscreenshot:test, python-traitsui, python-wxPython:python310, python-wxPython:python38, python-wxPython:python39, qgis, scummvm-tools, spek, superpaper, tintii, urbanlightscape, votca, wxEDID, wxEphe, wxMaxima, wxlua, wxsqlite3, wxsvg, xchm, xylib

The only package currently using the non-STL GTK 3.0 port is Audacity, and I haven't checked if this is actually necessary.

There have been several incompatible changes in wxWidgets in the past, and I think deprecating the non-STL version is a good move towards standardizaton.

StefanBruens avatar Aug 29 '22 20:08 StefanBruens

OK, I have to admit that I'm very surprised that all these projects compile with the STL build, but it's indeed quite encouraging. Maybe we could consider changing it, although I'm still not really sure about it.

In any case, we have a lot of other changes to make first, but we need to decide whether we do it or not before 3.3.0.

vadz avatar Aug 29 '22 22:08 vadz

Maybe a good first step would be to set wxNO_IMPLICIT_WXSTRING_ENCODING by default, and allow to override it at build time of dependent projects. This of course requires the library still includes all the required functions, and only hides the relevant function signatures in the headers.

StefanBruens avatar Aug 29 '22 22:08 StefanBruens

I have another proposal: what if we provided implicit wxString conversions to char* and wchar_t* in all builds (only if wxNO_IMPLICIT_WXSTRING_ENCODING is not defined, of course)? I.e. basically still set wxUSE_STD_STRING_CONV_IN_WXSTRING=0 by default but enable all the rest of STL support, including wxUSE_STD_CONTAINERS=1. This would be backwards-incompatible with the existing STL build, but IME programs currently using STL build are much less likely to rely on implicit conversions than the programs not using it.

The main problem with this I see is extra friction when using std::wstring, as ToStdWstring() would need to be always used explicitly. The loss of implicit conversion to std::string is less important as using utf8_string() is a much better idea anyhow.

vadz avatar Apr 08 '23 22:04 vadz

FYI: we build CodeLite for all platforms using --enable-stl or when using CMake -DwxUSE_STL=1 for very long time now...

https://docs.codelite.org/build/build_wx_widgets/#windows

eranif avatar Apr 08 '23 23:04 eranif

@eranif Does CodeLite rely on implicit conversions from wxString to std::string? I.e. does it compile if you add -DwxNO_UNSAFE_WXSTRING_CONV to the compilation flags?

vadz avatar Apr 09 '23 00:04 vadz

We do not rely on implicit conversions (we have users that are building CodeLite using their distro wxWidgets packages, so we can't rely on the fact that ENABLE_STL is enabled). We explicitly converting wxString -> std::string where needed

eranif avatar Apr 09 '23 08:04 eranif

I'd like to encourage anybody interested in this to test #23439 before it's merged into master, so that we could fix anything that can be fixed (some problems are unfortunately unavoidable) before doing it. TIA!

vadz avatar Apr 12 '23 15:04 vadz

FYI: STL containers are used by default now after fb17a5ac38b1bb0bcd8414a148d56fff34537101. I also intend to merge #23449 soon, which will remove the entire notion of STL and non-STL builds.

vadz avatar Apr 15 '23 23:04 vadz

#23449 is now merged, so there is no more separate STL build and I believe this issue can be closed now.

See also #23440 and #23462.

vadz avatar Apr 18 '23 13:04 vadz