-
Notifications
You must be signed in to change notification settings - Fork 15k
[Clang] Set the FTM for trivial relocation #142936
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
Conversation
The language of side seems fairly stable. Setting the feature test macro will ease implementation in standard libraries.
|
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesThe language of side seems fairly stable. Full diff: https://github.com/llvm/llvm-project/pull/142936.diff 3 Files Affected:
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 69a91eef6aedb..ef1a508a96c35 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -773,7 +773,7 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
Builder.defineMacro("__cpp_pack_indexing", "202311L");
Builder.defineMacro("__cpp_deleted_function", "202403L");
Builder.defineMacro("__cpp_variadic_friend", "202403L");
- // Builder.defineMacro("__cpp_trivial_relocatability", "202502L");
+ Builder.defineMacro("__cpp_trivial_relocatability", "202502L");
if (LangOpts.Char8)
Builder.defineMacro("__cpp_char8_t", "202207L");
diff --git a/clang/test/Lexer/cxx-features.cpp b/clang/test/Lexer/cxx-features.cpp
index 8c1867d5c7365..ced5bcaf0db16 100644
--- a/clang/test/Lexer/cxx-features.cpp
+++ b/clang/test/Lexer/cxx-features.cpp
@@ -49,6 +49,10 @@
#error "wrong value for __cpp_placeholder_variables"
#endif
+#if check(trivial_relocatability, 202502, 202502, 202502, 202502, 202502, 202502, 202502)
+#error "wrong value for __cpp_trivial_relocatability"
+#endif
+
// --- C++23 features ---
#if check(auto_cast, 0, 0, 0, 0, 0, 202110, 202110)
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index dff57689e84b9..cbcf462970b7f 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -280,12 +280,7 @@ <h2 id="cxx26">C++2c implementation status</h2>
<tr>
<td>Trivial Relocatability</pre></td>
<td><a href="https://wg21.link/P2786">P2786R13</a></td>
- <td class="partial" align="center">
- <details>
- <summary>Clang 21 (Partial)</summary>
- The feature test macro (<code>__cpp_trivial_relocatability</code>) has not yet been set.
- </details>
- </td>
+ <td class="unreleased" align="center">Clang 21</td>
</tr>
<tr>
<td><pre>#embed</pre></td>
|
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 has been in the tree for less than a month and we've not had a release out to the public with it yet, right? That seems a bit premature if we were waiting for some user experience before flipping the switch. I don't have a good intuition for how much use these facilities have gotten so far.
If @ldionne thinks this makes his life (or other standard libraries) easier though, it maybe be reasonable.
|
I'm OK with this, but @ldionne should be the one to approve. |
|
@EricWF we decided to hold off (we keep finding minor issues so it needs more backing) - can you use either the major clang version or __has_builtin as a discriminant for the time being? |
|
@cor3ntin I think Apple Clang still complicates version detection, and we can't use I don't think @ldionne would feel comfortable shipping actual libc++ changes if the compiler doesn't feel comfortable with it's own implementation. |
The reason clang is not setting the FTM in 21 is out of an abundance of caution, and because we found some minor bugs in the past couple of months, but it is certainly usable and doesn't have more bugs than other new features :) |
|
@erichkeane any objection merging that ? |
Go for it, I haven't seen any churn on this in months, and it is the beginning of the cycle. No better time! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/30969 Here is the relevant piece of the build log for the reference |
The language of side seems fairly stable.
Setting the feature test macro will ease implementation in standard libraries.