-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add c++20 compile option #6391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add c++20 compile option #6391
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds C++20 compiler support to PCL and synchronizes the CUDA standard with the C++ standard to prevent compilation issues.
Changes:
- Added C++20 support with appropriate compiler feature flags and version checks
- Fixed
PCL_CXX_COMPILE_FEATURESto dynamically reflect the selected C++ standard instead of being hardcoded to C++17 - Synchronized
CMAKE_CUDA_STANDARDwithCMAKE_CXX_STANDARDto prevent mismatches that cause compilation errors
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set(CMAKE_CXX_EXTENSIONS OFF) | ||
| if("${CMAKE_CXX_STANDARD}" GREATER_EQUAL 17) | ||
| set(PCL_CXX_COMPILE_FEATURES cxx_std_17) | ||
| if("${CMAKE_CXX_STANDARD}" GREATER_EQUAL 20) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition GREATER_EQUAL 20 will match C++ standards beyond C++20 (such as C++23, C++26, etc.), but sets C++20-specific values for PCL__cplusplus (202002L) and PCL_REQUIRES_MSC_VER (1928). This creates an inconsistency: if a user sets CMAKE_CXX_STANDARD to 23, line 21 would set PCL_CXX_COMPILE_FEATURES to cxx_std_23, but PCL__cplusplus would remain at the C++20 value (202002L). This could cause incorrect compiler version checks in pcl_config.h.in. Consider either changing to EQUAL 20 with additional conditions for newer standards, or update the values to be appropriate for the highest standard being checked.
| if("${CMAKE_CXX_STANDARD}" GREATER_EQUAL 20) | |
| if("${CMAKE_CXX_STANDARD}" EQUAL 20) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one we can just leave as is. PCL__cplusplus is only used to check for a minimum requirement.
| endif() | ||
|
|
||
| set(CMAKE_CUDA_STANDARD 17 CACHE STRING "The target CUDA/C++ standard. PCL requires CUDA/C++ 14 or higher.") | ||
| set(PCL_CXX_COMPILE_FEATURES cxx_std_${CMAKE_CXX_STANDARD}) |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CMAKE_CUDA_STANDARD is no longer defined as a CACHE variable. While the PR description explains this enforces synchronization with CMAKE_CXX_STANDARD to prevent compilation issues, this is a breaking change for users who were previously setting CMAKE_CUDA_STANDARD separately via CMake command line (e.g., -DCMAKE_CUDA_STANDARD=20). Their setting will now be ignored. Consider documenting this breaking change in release notes or migration guides, or alternatively, implementing a check that warns users if they attempt to set CMAKE_CUDA_STANDARD to a different value than CMAKE_CXX_STANDARD.
| set(PCL_CXX_COMPILE_FEATURES cxx_std_${CMAKE_CXX_STANDARD}) | |
| set(PCL_CXX_COMPILE_FEATURES cxx_std_${CMAKE_CXX_STANDARD}) | |
| # Warn users if they attempted to set CMAKE_CUDA_STANDARD independently; it is | |
| # intentionally synchronized with CMAKE_CXX_STANDARD in this project. | |
| if(DEFINED CMAKE_CUDA_STANDARD AND NOT CMAKE_CUDA_STANDARD STREQUAL "${CMAKE_CXX_STANDARD}") | |
| message(WARNING | |
| "CMAKE_CUDA_STANDARD is synchronized with CMAKE_CXX_STANDARD in this project. " | |
| "The provided CMAKE_CUDA_STANDARD ('${CMAKE_CUDA_STANDARD}') will be ignored in " | |
| "favor of CMAKE_CXX_STANDARD='${CMAKE_CXX_STANDARD}'.") | |
| endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one we could consider adding.
|
Sorry, I do not yet understand the necessity of this change (apart from |
|
@mvieth I agree. I was looking at using a few C++20 features, and I was starting with this change. I agree that I don't think this change actually fixes anything. |
This adds C++20 compiler features. Previously you could select a newer C++ version, but it always set the
PCL_CXX_COMPILE_FEATURESto 17.This also updates the
CMAKE_CUDA_STANDARDto use the same standard as theCXX_STANDARD. TheCMAKE_CUDA_STANDARDcould previously be set to a newer value thanCXX_STANDARD, but compilation issues occur frompcl_config.h.inif theCMAKE_CUDA_STANDARDis ever less than theCMAKE_CXX_STANDARD. This change enforces they both use the same standard.Testing
This was tested by successfully building with the
CMAKE_CXX_STANDARDset to both 17 and 20.