-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
boost: Fix compilation errors with newer Clang #24967
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit c5176d8boost/1.85.0@#ec251aafac59f5bd949608e7d8bb4ba3
boost/1.81.0@#95bfc471c650dc1868a7abc9f57d88b2
boost/1.76.0@#9e77c214096f776a17bb9671f56950f9
boost/1.83.0@#7885cc2a6e84573838fa661135b92c8e
boost/1.84.0@#c2a352e1279223b2c11a0a8c6cade6ec
boost/1.71.0@#fc36e024f4430248ffad76467557d473
boost/1.82.0@#8baa887d42e6ae1741a6e010c5313469
boost/1.75.0@#3259288ef22026bb48cfe8b71d4f0ef1
boost/1.80.0@#ffc804e0fee2dc6b48bdb0c2d7a47a6a
boost/1.77.0@#46e4531e7604024a41f9150204976dd3
boost/1.78.0@#1bf00df97ae975122dc3d2dae19c162d
boost/1.79.0@#361af037bbf5db52d590e7ea91457035
boost/1.74.0@#3c5db50295423013c379658858557768
boost/1.72.0@#b9b0afebefc4d88ec2f58266ae1f842d
boost/1.73.0@#4af995d81901ea3c9782c78c744ec815
|
HI @Artalus - thank you so much, and thanks for taking the time to validate this across multiple versions of Boost. However, please note that it is not Conan Center's goal to ensure a library compiles with all compilers - if the library itself does not support it - and we would simply point users to use the version of Boost that has this fix. We'd be happy to make an exception - we just need some reassurance that this does not cause breakages elsewhere ("it builds" doesn't always mean "it works") - however, we'd like to see some reassurance by the Boost MPL maintainers that it is safe to backport this fix all the way to version 1.71 - as we do not know if this has other side effects that we are unable to see simply by reviewing this PR. |
I went to ask Peter Dimov, one of the boost maintainers and the patches author, if the patch is good here boostorg/mpl#69 (comment) , and to quote,
That would be enough for me to keep the patch locally in our repo at least. |
Thanks so much for taking the time to do this! |
@Artalus Thank you for your pull request. However, I can not reproduce your error using the current Boost recipe in the master branch (commit c11f8dd), version 1.85.0 with Clang 18 on Linux and Conan 2.6.0. Here are both my full logs: Could your please provide more details about your error, so we can reproduce and track your current scenario, like:
|
How interesting! Despite the bug being formally fixed only by 1.86, and folks in the original thread complaining about it up until that release, I couldn't reproduce the problem with Conan's 1.85 either... I will check the other versions and update |
@Artalus That Conan version does not exist. I still need those details that I asked before. Please, check with you colleagues the exactly scenario. |
Apologies, I was just omitting the omnipresent trailing Conan 2.6.0, profile:
Full build log: |
@Artalus thank you for sharing those details, I'll try to reproduce the exactly same scenario. |
Apologies for the long delay!
@uilianries please note though, that these 3 fail with a completely different error!
This same error is present both in your I noticed that you had used slightly different profile. Specifically, your logs mention I have also rebuilt the whole version range with my So with all this in mind...
Shall I still adhere to this recommendation, or also include |
@Artalus I noted that difference too. Still, this patch will not fix the current situation for those old versions. I would suggest avoiding applying the patch for 1.71.0, 1.72.0, 1.73.0 as I commented before. In case someone needs it, it will have to fix other things first, and is not the case because we don't have issue associated. We can re-visit in future in case someone ask about, but I doubt due those versions age. |
c5176d8
to
eb2a73a
Compare
Conan v1 pipeline ✔️All green in build 3 (
Conan v2 pipeline ✔️
All green in build 3 (
|
Hooks produced the following warnings for commit efc6abdboost/1.79.0@#5f2b981b35851004df4720ed42aae516
boost/1.78.0@#0e43847e7fb9f92aa8a976cba67613f1
boost/1.77.0@#2918033c9485d94df5f0f12cb8762c83
boost/1.76.0@#f808264c86a836efde65998cc8462f2d
boost/1.74.0@#214f9708f99755f6492f837d0216dce0
boost/1.75.0@#9156f6927dc574041df89d2c78c22511
boost/1.80.0@#12e8fa25dcb3da76b7919e2543e28b95
|
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.
LGTM.
Summary
Changes to recipe: boost/[1.71 .. 1.85].
A new patch that fixes compiler errors, applied to all versions of boost <1.86
Motivation
If compiled with clang-16 or higher (18 on my Manjaro, 17 on my colleague's Ubuntu), building boost fails with this error:
This is a known issue with Boost boostorg/mpl#69 that got fixed only in the recently released version 1.86.
Details
To be honest, I don't exactly understand what and how the patch fixes 😅
I read through
69
's discussion, noticing first a commit innumeric_conversion
boostorg/numeric_conversion@50a1eae and then a PR inmpl
boostorg/mpl#77 that both referenced the issue.I combined both changes into a single patch, made sure it works both on 1.71 (oldest version in
conandata
), 1.85 (latest version) and 1.75 (the one we currently use), and thought it might be worthy to share this with bigger audience.Relates to conan-io/conan#15977 .