CMAKE_CXX_STANDARD confusion

[Vadim Zeitlin]

Hello,

I think current handling of CMAKE_CXX_STANDARD is wrong, but I'm not sure
how fix it. The first problem I see is that we have a chain of conditions
setting wxCXX_STANDARD_DEFAULT in build/cmake/options.cmake, but the first
test is _always_ true, as CMAKE_CXX_STANDARD is unconditionally set to 11
in the top-level CMakeLists.txt, so we could just as well avoid all these
complications and set wxBUILD_CXX_STANDARD default value to 11.

The second problem, which might not be one, so maybe this is more of a
question, is why do we use wxBUILD_CXX_STANDARD at all instead of just
using CMAKE_CXX_STANDARD? I.e. if you specify -DCMAKE_CXX_STANDARD=14 on
CMake command line right now, it's not taken into account at all because it
is overridden by the unconditional assignment above, and you have to define
wxBUILD_CXX_STANDARD instead, which seems rather user-unfriendly. By now we
will have to keep wxBUILD_CXX_STANDARD for compatibility, of course, but
why couldn't we honour the standard (sic) CMAKE_CXX_STANDARD variable if it
is defined?

Final problem, which is also perhaps a question, is that we
unconditionally set CMAKE_CXX_STANDARD to 11 instead of defaulting to
the compiler default C++ standard. And it is rather unexpected to still get
C++11 by default even with compilers defaulting to C++17, IMHO. I think it
would make more sense to use the compiler default and just check that it at
least supports C++11 (which should be the case for all but the most ancient
compilers by now, so maybe we could even omit this).

I.e. I think that we shouldn't define CMAKE_CXX_STANDARD at all, and
use COMPILER_DEFAULT as default value of wxBUILD_CXX_STANDARD.

Am I missing something here or should I make these changes? I have a
feeling that the current logic made sense before, when we still supported
C++98, but it doesn't seem to make sense any more. But please let me know
if I'm just misunderstanding something.

Thanks,
VZ

[Maarten Bent]

Hi,

Forcing c++11 is indeed a problem. It should not be set when a command-line value is provided.
This used to be handled by the conditions in options.cmake, but after forcing it to 11 this has become useless.

So I think you should remove `set(CMAKE_CXX_STANDARD 11)` in the main CMakeLists.txt.
Probably keep CMAKE_CXX_STANDARD_REQUIRED. If I understand correctly, it ensures the compiler will use the version, if specified.
And also remove the special case for APPLE in options.cmake.

Then it will always use the compiler default.
And if you specify a version via the command-line, either with CMAKE_CXX_STANDARD or wxBUILD_CXX_STANDARD, it will use this version.
wxBUILD_CXX_STANDARD is a bit obsolete, but I still find it useful for quickly changing the c++ version in cmake-gui.

The only drawback I can think of is when a c++98 compiler is used, it probably won't give an error when configuring the project.

Maarten

[Vadim Zeitlin]

On Sat, 30 Dec 2023 22:26:20 +0100 Maarten Bent wrote:

MB> Forcing c++11 is indeed a problem. It should not be set when a
MB> command-line value is provided.
MB> This used to be handled by the conditions in options.cmake, but after
MB> forcing it to 11 this has become useless.

Thanks for confirming this!

MB> So I think you should remove `set(CMAKE_CXX_STANDARD 11)` in the main
MB> CMakeLists.txt.

I'll do this.

MB> Probably keep CMAKE_CXX_STANDARD_REQUIRED. If I understand correctly, it
MB> ensures the compiler will use the version, if specified.

I couldn't find what exactly this does, i.e. how does it "require" it and
whether it happens immediately, when this is executed (I suspect not?) or
later (but when?). I'll try to experiment more with this, but if anybody
already knows the answer, please let me know.

MB> And also remove the special case for APPLE in options.cmake.

Yes, I forgot to mention it, but I indeed don't see any point in having
this any longer.

MB> The only drawback I can think of is when a c++98 compiler is used, it
MB> probably won't give an error when configuring the project.

This is why I'd like to have a test checking for C++11 support. We can
always just write one manually, but if we can somehow manage to use
CMAKE_CXX_STANDARD_REQUIRED to check for it even if CMAKE_CXX_STANDARD is
not set, it would be even better.

Thanks again,
VZ

[john]

On Dec 30, 2023, at 19:18, Vadim Zeitlin <va...@wxwidgets.org> wrote:

On Sat, 30 Dec 2023 22:26:20 +0100 Maarten Bent wrote:

MB> Forcing c++11 is indeed a problem. It should not be set when a
MB> command-line value is provided.
MB> This used to be handled by the conditions in options.cmake, but after
MB> forcing it to 11 this has become useless.

Thanks for confirming this!

MB> So I think you should remove `set(CMAKE_CXX_STANDARD 11)` in the main
MB> CMakeLists.txt.

I'll do this.

MB> Probably keep CMAKE_CXX_STANDARD_REQUIRED. If I understand correctly, it
MB> ensures the compiler will use the version, if specified.

I couldn't find what exactly this does, i.e. how does it "require" it and
whether it happens immediately, when this is executed (I suspect not?) or
later (but when?). I'll try to experiment more with this, but if anybody
already knows the answer, please let me know.

It does nothing if CMAKE_CXX_STANDARD isn't set. If it is set and CMAKE_CXX_STANDARD_REQUIRED is true (in the cmake meaning of true) then configuration will fail if the compiler doesn't support the standard called for by CMAKE_CXX_STANDARD. See https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD_REQUIRED.html#prop_tgt:CXX_STANDARD_REQUIRED.

MB> And also remove the special case for APPLE in options.cmake.

Yes, I forgot to mention it, but I indeed don't see any point in having
this any longer.

MB> The only drawback I can think of is when a c++98 compiler is used, it
MB> probably won't give an error when configuring the project.

This is why I'd like to have a test checking for C++11 support. We can
always just write one manually, but if we can somehow manage to use
CMAKE_CXX_STANDARD_REQUIRED to check for it even if CMAKE_CXX_STANDARD is
not set, it would be even better.

Cmake provides this via target_compile_features, see https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html#id6. AFAICT there's no way to set it globally, but you could set a variable to the desired standard and pass that to target_compile_features for every target. https://cmake.org/cmake/help/latest/manual/cmake-compile-features.html is worth reading in its entirety: It's not very long and gives a lot of insight into how cmake configures the compiler.

A caution about all of this: There is exactly one backwards compatibility issue in C++: auto_ptr. It was deprecated in C++11 and removed in C++17, so any code containing it will fail to compile if the standard is C++17 or later. I hope that there aren't any auto_ptrs in wx, but I think it would be wise to make sure, converting any you find to std::unique_ptr.

Regards,

John Ralls

[Vadim Zeitlin]

On Sun, 31 Dec 2023 09:38:19 -0800 john wrote:

j> > MB> Probably keep CMAKE_CXX_STANDARD_REQUIRED. If I understand correctly, it
j> > MB> ensures the compiler will use the version, if specified.
j> >
j> > I couldn't find what exactly this does, i.e. how does it "require" it and
j> > whether it happens immediately, when this is executed (I suspect not?) or
j> > later (but when?). I'll try to experiment more with this, but if anybody
j> > already knows the answer, please let me know.
j>
j> It does nothing if CMAKE_CXX_STANDARD isn't set. If it is set and
j> CMAKE_CXX_STANDARD_REQUIRED is true (in the cmake meaning of true) then
j> configuration will fail if the compiler doesn't support the standard
j> called for by CMAKE_CXX_STANDARD. See
j> https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD_REQUIRED.html#prop_tgt:CXX_STANDARD_REQUIRED.

Thanks, I guess we should have our own small test checking for C++11
support then. It could be pretty simple, as we can just check for MSVS >=
2015 or any compiler defining __cplusplus as >= 201103L.

j> Cmake provides this via target_compile_features, see
j> https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html#id6.
j> AFAICT there's no way to set it globally, but you could set a variable
j> to the desired standard and pass that to target_compile_features for
j> every target.

This seems more complicated than a simple C++11 test, so I don't think we
should do this, but it's good to know, thanks.

j> A caution about all of this: There is exactly one backwards
j> compatibility issue in C++: auto_ptr.

wx doesn't use std::auto_ptr<>, it had its own wxScopedPtr<> before and
then switched to std::unique_ptr<>. There are other things
deprecated/removed in C++17/C++20 but I can/know how to deal with them,
it's just CMake that I'm uncomfortable with. But I'll try to get better
next year...

Regards,
VZ

[Vadim Zeitlin]

On Sat, 30 Dec 2023 22:26:20 +0100 Maarten Bent wrote:

MB> Forcing c++11 is indeed a problem. It should not be set when a
MB> command-line value is provided.

I've finally done this in https://github.com/wxWidgets/wxWidgets/pull/24188

Please let me know if you have any comments/suggestions for improvement.

TIA!
VZ