Skip to content

[build] Add /Zc:__cplusplus switch to fix __cplusplus on MSVC#6422

Closed
AlrIsmail wants to merge 1 commit intoPointCloudLibrary:masterfrom
AlrIsmail:fix-msvc-cplusplus-reporting
Closed

[build] Add /Zc:__cplusplus switch to fix __cplusplus on MSVC#6422
AlrIsmail wants to merge 1 commit intoPointCloudLibrary:masterfrom
AlrIsmail:fix-msvc-cplusplus-reporting

Conversation

@AlrIsmail
Copy link
Copy Markdown

Fixes #3494.

This PR adds the /Zc:__cplusplus compiler flag when building with MSVC 15.7 or newer to ensure the __cplusplus macro correctly reports the C++ standard version.

Key Changes:

  • Appended /Zc:__cplusplus to global CMAKE_CXX_FLAGS for MSVC 15.7+.
  • Added PCLCONFIG_COMPILE_OPTIONS to pcl_pclconfig.cmake for storing/exporting compile options.
  • Updated PCLConfig.cmake.in so downstream projects inherit the flag.

Verification: Verified CMake logic via mock tests for older and newer MSVC versions, ensuring alignment with existing PCL build patterns.

@AlrIsmail AlrIsmail changed the title [build] Add /Zc:__cplusplus switch to fix __cplusplus on MSVC (#3494) [build] Add /Zc:__cplusplus switch to fix __cplusplus on MSVC Apr 12, 2026
@mvieth
Copy link
Copy Markdown
Member

mvieth commented Apr 14, 2026

First of all, thank you for the pull request.
However, I think we should briefly discuss whether we still want to do this. PCL itself should already correctly handle the fact that __cplusplus does not report the real C++ standard on MSVC. Examples:

#if defined(__cplusplus) && ((!defined(_MSC_VER) && __cplusplus < ${PCL__cplusplus}) || (defined(_MSC_VER) && _MSC_VER < ${PCL_REQUIRES_MSC_VER}) || (defined(_MSVC_LANG) && _MSVC_LANG < ${PCL__cplusplus}))

#if (__cplusplus >= 201703L) || (defined(_MSC_VER) && (_MSC_VER >= 1910) && (_MSVC_LANG >= 201703L))

So within PCL, I do not see an immediate benefit of adding the /Zc:__cplusplus compiler option.

Additionally, I am not sure if it is a good idea to force that option onto downstream projects (that use PCL via PCLConfig.cmake). I have not looked that deep into it, but I assume there is a reason why it is opt-in, instead of __cplusplus always being the correct version. https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/#please-look-for-usage-in-your-code suggests that some code/projects may expect that always __cplusplus == 199711L.

Honestly, I think the most careful option would be to continue as before: Not using /Zc:__cplusplus, and expecting that __cplusplus is not the actual C++ standard on MSVC.
@larshg What is your opinion on this?

@larshg
Copy link
Copy Markdown
Contributor

larshg commented Apr 14, 2026

From what I can gather it seems to be best just to continue as it is now and close the two as "won't change".

Its only if MS at some point "enforce" the use of /Zc:__cplusplus and deprecates ie. _MSVC_LANG.
So if we were to add it, it could be a option which is default off.

But as it doesn't really gives us any benefits, I would just leave it as is and close the issue, as it is a relatively quick change to add if required (updates from _MSVC_LANG to __cplusplus >= 201703L and equivalent would take time, though I don't know how many places it is used).

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Apr 14, 2026

I count three places where _MSVC_LANG is used, two in pcl_macros.h and one in pcl_config.h.in. Should be quick to change those if/when needed, although I don't really think that _MSVC_LANG will ever be deprecated.

Then I will close this pull request and the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add /Zc:__cplusplus switch to fix __cplusplus on MSVC

3 participants