cmake: fix a redundant flag causing the build to fail with newer GCC versions#10940
cmake: fix a redundant flag causing the build to fail with newer GCC versions#10940SquareRobin wants to merge 1 commit into
Conversation
Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
There was a problem hiding this comment.
Code Review
This pull request updates the compiler options in src/CMakeLists.txt by changing -Wp,-D_GLIBCXX_ASSERTIONS to -D_GLIBCXX_ASSERTIONS. The reviewer recommended wrapping this macro in a C++ generator expression to ensure it is only applied to C++ source files and not C or assembly files.
| -Wformat-security | ||
| -Wno-sign-compare | ||
| -Wp,-D_GLIBCXX_ASSERTIONS | ||
| -D_GLIBCXX_ASSERTIONS |
There was a problem hiding this comment.
Since _GLIBCXX_ASSERTIONS is a C++ standard library (libstdc++) specific macro, it is best practice to restrict it to C++ compilation using a generator expression. This prevents the flag from being passed to C or assembly source files in the project (such as those in subdirectories like ABC or linenoise).
$<$<COMPILE_LANGUAGE:CXX>:-D_GLIBCXX_ASSERTIONS>
There was a problem hiding this comment.
Welcome to OpenROAD! Thanks for opening your first PR.
Before we review:
- Contribution Guide: https://openroad.readthedocs.io/en/latest/contrib/contributing.html
- Build Instructions: https://openroad.readthedocs.io/en/latest/contrib/BuildWithCMake.html
Please ensure:
- CI passes
- Code is properly formatted
- Tests are included where applicable
A maintainer will review shortly!
Summary
Passing
-D_GLIBCXX_ASSERTIONSdirectly to the preprocessor results in an error compiling with GCC 14 or 15 (14.3.1 and 15.3.0) , simply removing-Wpfixes this and causes no compile time errors/warnings. Tests also pass with the flag changed.Type of Change
Impact
Adds support for GCC 14 and 15 (probably other versions, too) while causing no observable functionality changes.
Verification
ran cmake manually).