-
Notifications
You must be signed in to change notification settings - Fork 192
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
Work around boost failure to build in clang 15 #5495
Work around boost failure to build in clang 15 #5495
Conversation
cmake/SetupCxxFlags.cmake
Outdated
set(CMAKE_CXX_FLAGS "-D_HAS_AUTO_PTR_ETC=0 ${CMAKE_CXX_FLAGS}") | ||
endif() | ||
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.
I use if/elseif here, even though it duplicates a line, to avoid lines over 80 characters. I tried continuing the if line with , but make complained
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.
I believe CMake ignores newlines outside of strings (except as generic whitespace), so you should be able to wrap it just like C++.
(To any other readers: The previous comment contains a backslash before the last comma, but GitHub isn't displaying it.)
cmake/SetupCxxFlags.cmake
Outdated
if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "15.0.0") | ||
set(CMAKE_CXX_FLAGS "-D_HAS_AUTO_PTR_ETC=0 ${CMAKE_CXX_FLAGS}") | ||
elseif(CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL "15.0.0") | ||
set(CMAKE_CXX_FLAGS "-D_HAS_AUTO_PTR_ETC=0 ${CMAKE_CXX_FLAGS}") |
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.
Add this to SetupBoost.cmake please, to the boost target using target_compile_definitions
.
14adcf6
to
39b93eb
Compare
Ready for another look when you get a chance...thanks for the reviews! It's such a short PR that I squashed immediately. |
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 only became a problem with recent versions of boost (the code builds fine with boost 1.74 and clang 15 in the container, though I don't see an obvious reason why). It is fixed in boost 1.83. Therefore I suggest to add the condition BOOST_VERSION VERSION_LESS 1.83.0
.
39b93eb
to
bea8588
Compare
Note that brew has not yet updated boost to 1.83.0, so until they do, this workaround still matters for macOS users running the latest AppleClang |
if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "15.0.0" OR | ||
CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL "15.0.0") |
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.
Note that GREATER_EQUAL
also exists, and this could also be combined with the above if
(not requesting this).
Thanks for fixing this @geoffrey4444 ! |
Proposed changes
Workaround for boost not building with clang 15: adds
-D_HAS_AUTO_PTR_ETC=0
toCMAKE_CXXX_FLAGS
if clang version >= 15.0.0. This fix is necessary to build with latest Xcode/AppleClang.Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments