Traditional MSVC preprocessor does not have __VA_OPT__, even c++20 mode

[puetzk]

I just encountered a weird problem where none of my mocks would compile after switching a project from Qt 5.15 to Qt6; when I tracked it down, the problem was that the new MAKE_MOCK(name, auto (TArg) -> Treturn) syntax wasn't getting signatures right, because TROMPELOEIL_COUNT started returning 0.

This project had been compiling with -std:c++20 all along; the key thing that changed from Qt 5 to Qt6 is that CMake find_package machinery started adding the compiler flag -Zc:__cplusplus, so Qt can tell whether the compiler supports c++20 (regardless of the other components, like the preprocessor features).

Arguably MSVC is wrong here, of course, - if you don't support the new preprocessor features like __VA_OPT__, you don't really have complete c++20 support and shouldn't be claiming __cplusplus >= 202002L. But I think Microsoft's and Qt's choices are somewhat defensible - they don't do it by default, MS just offered individually-tweakable Zc: conformance options like -Zc:__cplusplus and -Zc:preprocessor if you want to shop ala carte. And Qt chose to force on as little as necessary to meet their needs. The annoying question is why cl /std:c++20 /permissive- still defaults to the traditional preprocessor... but that's just inertial.

But in any case, this combination makes the logic wrong at

https://github.com/rollbear/trompeloeil/blob/eaeb89c1ce9d354b0ba7eb921fd5712cdbd78adf/include/trompeloeil/mock.hpp#L125-L136

if _MSVC_TRADITIONAL is defined, and set to 1, that indicates traditional MSVC preprocessor regardless of what __cppstd says, and __VA_OPT__ is not available. So if you're bothering to cater to MSVC's traditional preprocessor at all, I think it this needs another clause:

#elif defined(_MSVC_TRADITIONAL) && _MSVC_TRADITIONAL==1 
#  define TROMPELOEIL_HAS_VA_OPT 0
#  define TROMPELOEIL_MSVC_PREPROCESSOR 1

Or you could refactor it more, into something like

#if defined(_MSVC_TRADITIONAL)
#  if _MSVC_TRADITIONAL==0 
#    if _MSC_VER >= 1940
#      define TROMPELOEIL_HAS_VA_OPT 1 
#    else 
#      define TROMPELOEIL_HAS_VA_OPT 0 
#    endif
#    define TROMPELOEIL_MSVC_PREPROCESSOR 0 
#  endif
#elif __cplusplus >= 202002L 
#  define TROMPELOEIL_HAS_VA_OPT 1 
#else 
#  define TROMPELOEIL_HAS_VA_OPT 0 
#endif

[puetzk]

Also, at least according to the docs https://learn.microsoft.com/en-us/cpp/preprocessor/preprocessor-experimental-overview?view=msvc-170#comma-elision-in-variadic-macros, __VA_OPT__ has been available (in the new preprocessor only) since Visual Studio 16.5, which is _MSV_VER 1925, not 1940 (visual studio 17.10) like the check currently uses.

But it's quite possible you did that on purpose; in those early versions it's only enabled when also under /std:c++latest, and even then there does seem (from a quick try on godbolt: https://godbolt.org/z/W6G5vb3nK) to be some brokenness.

Microsoft C/C++ language conformance by Visual Studio version says

Feature	Supported
P0306R4 Adding VA_OPT for comma omission and comma deletion	VS 2019 16.5. To provide better backward compatibility, VA_OPT is enabled under /Zc:preprocessor across all language versions.

But that is definitely not the case for older verisons, which seem to only support it under /std:c++20 or /std:c++latest. That wording seems to have been added in Feb 2024, which would line up with the 17.10 previews. So I guess that's why 1940 - prior to that you'd also need to know the chosen /std: (and I'm not sure you can, unless somebody has set /Zc:__cplusplus).

So a really pedantic answer could use accept Visual Studio 16.7 /std:c++latest /Zc:__cplusplus /Zc:preprocessor (strict __cplusplus > check, rather than the usual >=, because we want to reject /std:c++17 and accept /std:c++latest with the first batch of post-c++17 additions, like __VA_OPT)

#if defined(_MSVC_TRADITIONAL)
#  if _MSVC_TRADITIONAL==0 
#    if (_MSC_VER >= 1940) || (_MSVC_VERSION > 1925 &&  __cplusplus > 20173L)
#      define TROMPELOEIL_HAS_VA_OPT 1 
#    else 
#      define TROMPELOEIL_HAS_VA_OPT 0 
#    endif
#    define TROMPELOEIL_MSVC_PREPROCESSOR 0 
#  endif
#elif __cplusplus >= 202002L 
#  define TROMPELOEIL_HAS_VA_OPT 1 
#else 
#  define TROMPELOEIL_HAS_VA_OPT 0 
#endif

But I'm not sure if there's enough audience left on Visual Studio 2019 to care... espescially when you also still have an alternate TROMPELOEIL_COUNT for that works when TROMPELOEIL_HAS_VA_OPT=0

[puetzk]

I did find https://developercommunity.visualstudio.com/t/VA_OPT-should-be-enabled-by-std:c/10565399?sort=newest&ftype=problem confirming the original issue (that __VA_OPT__ is not available, even in c++20 mode, unless you've also set /Zc:preprocessor, and this is not on by default.

[puetzk]

That wording seems to have been added in Feb 2024, which would line up with the 17.10 previews

I found specific confirmation that this change (enabling __VA_OPT even for older /std: options) was made in 17.10: https://developercommunity.visualstudio.com/t/Zc:preprocessor-VA_OPT-is-not-enabl/10205604

I had also forgotten about the existence of _MSVC_LANG (even though trompeloeil is already using it:

https://github.com/rollbear/trompeloeil/blob/eaeb89c1ce9d354b0ba7eb921fd5712cdbd78adf/include/trompeloeil/mock.hpp#L91-L94

So that would make a feature test for __VA_OPT__ IN MSVC, honoring both /Zc:preprocessor and /std:, but independent of /Zc:__cplusplus, would then be:

#if defined(_MSVC_TRADITIONAL)
#  if _MSVC_TRADITIONAL==0 
#    if (_MSC_VER >= 1940) || (_MSVC_VER > 1925 &&  _MSVC_LANG > 201703L)
#      define TROMPELOEIL_HAS_VA_OPT 1 
#    else 
#      define TROMPELOEIL_HAS_VA_OPT 0 
#    endif
#    define TROMPELOEIL_MSVC_PREPROCESSOR 0
#  endif
#elif __cplusplus >= 202002L 
#  define TROMPELOEIL_HAS_VA_OPT 1 
#else 
#  define TROMPELOEIL_HAS_VA_OPT 0 
#endif

Excluding version 16.5 itself because that one has it but seems buggy: https://godbolt.org/z/PPrhh4fPa

__VA_OPT__ itself does do something in 16.5's /std:c++latest /Zc:preprocessor, but it's not quite right - the surrounding parentheses doesn't seem to get eliminated. It seems to work in 16.7 and up (godbolt doesn't have 16.6 for some reason?). But I assume the fix is probably between 16.5 and 16.6, rather than 16.6->16.7, given that 16.6 preview 1 is when MS made the big announcement that the new conforming preprocessor was ready for use https://devblogs.microsoft.com/cppblog/announcing-full-support-for-a-c-c-conformant-preprocessor-in-msvc/ (and in fact specifically called out support for __VA_OPT__).

[rollbear]

Oh. Sorry.

No, this was most certainly not an intentional break. There have been very few such, and it was a long time since it last happened, and it's always included with an apology for the inconvenience in the ChangeLog. I'll see what I can do. I am a little bit hampered by not having a working MSVC environment myself.

If you can make a reproducer in the github actions CI, that'd be awesome, because it'd ensure I can't accidentally reintroduce the problem, and I'd also have a simple way of ensuring that I have fixed it. No need, obviously.

I'll see what I can do.

[puetzk]

If you can make a reproducer in the github actions CI

Done, see dcb489c448f266c910a8c6a538beed6c6112b043 and https://github.com/puetzk/trompeloeil/actions/runs/14110751749

I am a little bit hampered by not having a working MSVC environment myself.

If you don't have a windows machine at all, understandable. If have windows and want MSVC, do note you can have Visual Studio Community 2022 or Build Tools for Visual Studio 2022 (if you just want the compiler/linker tools but not the whole IDE) for free: https://visualstudio.microsoft.com/license-terms/vs2022-ga-community/

Anyone (even an otherwise-too-large enterprise) is allowed to use it under the community license for Open source work:

1.b.i: Any number of your users may use the software to develop and test applications released under Open Source Initiative (OSI) approved open source software licenses.

And trompeloeil's BSL-1.0 is indeed OSI approved. So you can have MSVC if you want...

I'll see what I can do.

Thanks! I opened a PR with proposed fix in #345

[rollbear]

Thank you. Merged. I'll keep this open until a release has been tagged.

[puetzk]

Ok. No big rush for a release if you've got other stuff cooking, I've already turned on /Zc:preprocessor for the project where we hit this, and am happy to keep that enabled as an also-desirable modernization.

But I'm aware of some other Microsoft SDKs (e.g. Event tracing for windows) that are incompatible with this (stille xpect/require the traditional MSVC preprocessor), so after tracking down all the details in the course of figuring out what happened it seemed worth fixing compatibility anyway.