-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[fmt] Do not support Clang 20 + cppstd=20 due string literal evaluation #26177
Conversation
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Thank you for your comments, after digging a bit more, the problem is only related to the major versions of FMT: 8, 10, and 11, but when using Clang 20 only. As reported on FMT issue 4177, the clang 20 changed its compile-time evaluation for string literals, resulting in that reported error. The same only affects some versions of FMT. Still, the commit fmtlib/fmt@cacc310 is not enough, as commented in the same issue, but it also requires the PR fmtlib/fmt#4247. Those changes are in the master branch, but no milestone is pointed out; the latest fmt release is from July 2024, so we do not have that fix available in binaries. I would not backport those patches because Clang 20 is still in progress (https://clang.llvm.org/docs/ReleaseNotes.html) and may have a different result in the future. Plus, it would need to be backported for multiple versions in CCI. I built locally the FMT by using different versions, plus using Clang 20 and Clang 19 (all using compiler.cppstd=20):
|
After new test, the error only occurs with Clang 20, but using C++20 or later (I only tested using C++23, not 26). However, using C++17 all versions build without that reported error. I updated the restriction to only invalidate the case when using Clang 20 + C++20 (or later) Local build logs:
|
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Thanks @uilianries for the very thorough testing. Erring on the side of caution here: clang-20 has not been released yet, and neither has clang-19 for that matter - so I'm not sure we should add validations for unreleased versions - on the odd chance that it is a compiler bug that might be fixed before the release. I appreciate emsdk is a problem in this regard since it uses git snapshots rather than releases. I suspect the most recent fmt 11.x already fixes this - and the check in this PR would cover a version where this is fixed, so it would be undesirable. Not sure there is much for us to do in the recipe - these things happen when developers are on the bledeing edge: latest emsdk, at-head llvm-clang snapshot, latest fmt, c++20. If the developer's goal is to be trailblazers and discover bugs: great. If not, my advice to them would be to be a tad more conservative a use well known versions instead. |
Summary
Changes to recipe: fmt/[*]
Motivation
closes #26169
/cc @AbrilRBS
Details
It was reported by @skhaz on issue #26169, the same bug was reported to fmt directly: fmtlib/fmt#4264
Probably fixed by fmtlib/fmt#4177, but there is no milestone indicated for that hotfix.
The error ONLY occurs when building for C++20 (or newer) and using Clang 20. Here are some scenarios used for the validation: