In C++20, mixing enums in expressions is deprecated

[Kenneth Porter]

<https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/c5054?view=msvc-170>

It's common to mix window flags from different enums to form a single flags
argument, like this:

fgSizer->Add( gauge, 0, wxALIGN_CENTER_VERTICAL|wxALL|wxEXPAND, 5 );

When I enable C++20 in Visual Studio, I get this warning:

warning C5054: operator '|': deprecated between enumerations of different
types

I think long-term it makes sense to move these constants out of their enums
and make them all const int, since they get passed to int parameters. User
code shouldn't see a difference. C++ doesn't use the enum for scope,
anyway, and the names already have the wx decoration for namespacing, so I
don't see any advantage to keeping them within enums.

[Vadim Zeitlin]

On Tue, 10 May 2022 12:33:58 -0700 Kenneth Porter wrote:

KP> It's common to mix window flags from different enums to form a single flags
KP> argument, like this:
KP>
KP> fgSizer->Add( gauge, 0, wxALIGN_CENTER_VERTICAL|wxALL|wxEXPAND, 5 );
KP>
KP> When I enable C++20 in Visual Studio, I get this warning:
KP>
KP> warning C5054: operator '|': deprecated between enumerations of different
KP> types

We already have a solution in place for this, which I've added for g++ and
clang that produce similar warnings when using -std=c++20, but it's guarded
by __cplusplus >= 202002L condition, see wxALLOW_COMBINING_ENUMS()
definition in wx/defs.h.

I thought C++20 did define __cplusplus properly in MSVC now, but
apparently I'm wrong and it still needs /Zc:__cplusplus for this, so we
need to check for it in another way, e.g. wxCHECK_VISUALC_VERSION(14) or
maybe check _MSVC_LANG. In fact, it looks like we ought to have
wxCHECK_CXX_STD() macro that would test either __cplusplus or _MSVC_LANG
depending on the compiler.

Anyhow, for now I recommend adding a check for _MSVC_LANG to the condition
before wxALLOW_COMBINING_ENUMS() definition and check if this fixes the
problem. If it does, any PRs based on this idea would be welcome, of
course. And if it doesn't, please let me know.

Thanks,
VZ

[Kenneth Porter]

--On Wednesday, May 11, 2022 3:23 AM +0200 Vadim Zeitlin

<va...@wxwidgets.org> wrote:

> Anyhow, for now I recommend adding a check for _MSVC_LANG to the
> condition before wxALLOW_COMBINING_ENUMS() definition and check if this
> fixes the problem. If it does, any PRs based on this idea would be
> welcome, of course. And if it doesn't, please let me know.

I changed it to this and got a clean compile:

#if defined(__cplusplus) && ((__cplusplus >= 202002L) || (_MSVC_LANG >=
202002L))

[Kenneth Porter]

--On Wednesday, May 11, 2022 3:23 AM +0200 Vadim Zeitlin
<va...@wxwidgets.org> wrote:

> I thought C++20 did define __cplusplus properly in MSVC now, but
> apparently I'm wrong and it still needs /Zc:__cplusplus for this, so we
> need to check for it in another way, e.g. wxCHECK_VISUALC_VERSION(14) or
> maybe check _MSVC_LANG. In fact, it looks like we ought to have
> wxCHECK_CXX_STD() macro that would test either __cplusplus or _MSVC_LANG
> depending on the compiler.

__cplusplus has the value 199711L for MSVS 2019 version 16.11.12.
_MSVC_LANG does have the desired value of 202002L.

[Vadim Zeitlin]

On Wed, 11 May 2022 08:48:45 -0700 Kenneth Porter wrote:

KP> --On Wednesday, May 11, 2022 3:23 AM +0200 Vadim Zeitlin
KP> <va...@wxwidgets.org> wrote:
KP>
KP> > I thought C++20 did define __cplusplus properly in MSVC now, but
KP> > apparently I'm wrong and it still needs /Zc:__cplusplus for this, so we
KP> > need to check for it in another way, e.g. wxCHECK_VISUALC_VERSION(14) or
KP> > maybe check _MSVC_LANG. In fact, it looks like we ought to have
KP> > wxCHECK_CXX_STD() macro that would test either __cplusplus or _MSVC_LANG
KP> > depending on the compiler.
KP>
KP> __cplusplus has the value 199711L for MSVS 2019 version 16.11.12.
KP> _MSVC_LANG does have the desired value of 202002L.

Thanks for testing, I've made a quick PR at

https://github.com/wxWidgets/wxWidgets/pull/22422

that should fix this problem and allow us to use wxCHECK_CXX_STD() in other
places later if necessary.

If nobody sees/finds any problems with it, I'll merge it in a few days.

Thanks,
VZ

[David Connet]

On 5/11/2022 8:48 AM, Kenneth Porter wrote:

__cplusplus has the value 199711L for MSVS 2019 version 16.11.12. _MSVC_LANG does have the desired value of 202002L.

That would be expected. That is MS's default value unless you specify "/Zc:__cplusplus". Once you specify that, whatever you specify via "/std:<14/17/20/latest>" will also properly set __cplusplus.

And TIL about _MSVC_LANG... (me googles) From MS's docs:

> The _MSVC_LANG predefined macro reports the standard version whether the /Zc:__cplusplus option is enabled or disabled. When /Zc:__cplusplus is enabled, __cplusplus has the same value as _MSVC_LANG.

Dave

[Kenneth Porter]

--On Wednesday, May 11, 2022 10:11 AM -0700 David Connet

<dc...@agilityrecordbook.com> wrote:

> That would be expected. That is MS's default value unless you specify
> "/Zc:__cplusplus". Once you specify that, whatever you specify via
> "/std:<14/17/20/latest>" will also properly set __cplusplus.

TIL that lots of legacy code breaks when this macro has the "right" value:

<https://www.reddit.com/r/cpp/comments/bqsyss/zc_cplusplus_by_default/>

<https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/>

So I guess to be safe we need to look at _MSVC_LANG, in case an app is
linking to another library that's still broken. Vadim's workaround looks
good.