-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[clang] Add an ABI_EXTENSION concept that is testable under -pedantic… #153506
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
…-errors This is a targeted change to allow us to specify that an extension test should return true even if -pedantic-errors is enabled. In the longer term we may want to consider changing the behavior of -pedantic-errors to not gate __has_extension checks, and become more in line with `-Wpedantic -Werror`.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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'm fine with this for the branch, sure.
|
@llvm/pr-subscribers-clang Author: Oliver Hunt (ojhunt) Changes…-errors This is a targeted change to allow us to specify that an extension test should return true even if -pedantic-errors is enabled. In the longer term we may want to consider changing the behavior of -pedantic-errors to not gate __has_extension checks, and become more in line with Full diff: https://github.com/llvm/llvm-project/pull/153506.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def
index c58e3f2400adc..04ed820b4213a 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -24,8 +24,8 @@
//
//===----------------------------------------------------------------------===//
-#if !defined(FEATURE) && !defined(EXTENSION)
-# error Define either the FEATURE or EXTENSION macro to handle features
+#if !defined(FEATURE) && !defined(EXTENSION) && !defined(ABI_EXTENSION)
+# error Define either the FEATURE, EXTENSION, or ABI_EXTENSION macro to handle features
#endif
#ifndef FEATURE
@@ -36,6 +36,10 @@
#define EXTENSION(Name, Predicate)
#endif
+#ifndef ABI_EXTENSION
+#define ABI_EXTENSION(Name, Predicate)
+#endif
+
FEATURE(speculative_load_hardening, LangOpts.SpeculativeLoadHardening)
FEATURE(address_sanitizer,
LangOpts.Sanitize.hasOneOf(SanitizerKind::Address |
@@ -148,7 +152,7 @@ FEATURE(thread_sanitizer, LangOpts.Sanitize.has(SanitizerKind::Thread))
FEATURE(dataflow_sanitizer, LangOpts.Sanitize.has(SanitizerKind::DataFlow))
FEATURE(scudo, LangOpts.Sanitize.hasOneOf(SanitizerKind::Scudo))
FEATURE(ptrauth_intrinsics, LangOpts.PointerAuthIntrinsics)
-EXTENSION(ptrauth_qualifier, LangOpts.PointerAuthIntrinsics)
+ABI_EXTENSION(ptrauth_qualifier, LangOpts.PointerAuthIntrinsics)
FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
FEATURE(ptrauth_vtable_pointer_address_discrimination, LangOpts.PointerAuthVTPtrAddressDiscrimination)
@@ -385,3 +389,4 @@ EXTENSION(cxx_type_aware_allocators, true)
#undef EXTENSION
#undef FEATURE
+#undef ABI_EXTENSION
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 685a9bbf2cde9..c925e1f8a1331 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -1183,11 +1183,15 @@ void DumpCompilerOptionsAction::ExecuteAction() {
OS << "\n\"extensions\" : [\n";
{
llvm::SmallString<128> Str;
-#define EXTENSION(Name, Predicate) \
+#define EXTENSION_OPTION(Name, Predicate) \
("\t{\"" #Name "\" : " + llvm::Twine(Predicate ? "true" : "false") + "},\n") \
.toVector(Str);
+#define EXTENSION(Name, Predicate) EXTENSION_OPTION(Name, Predicate)
+#define ABI_EXTENSION(Name, Predicate) EXTENSION_OPTION(Name, Predicate)
#include "clang/Basic/Features.def"
#undef EXTENSION
+#undef ABI_EXTENSION
+#undef EXTENSION_OPTION
// Remove the newline and comma from the last entry to ensure this remains
// valid JSON.
OS << Str.substr(0, Str.size() - 2);
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 6f12ac80d677e..7cae6acc081c0 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1109,19 +1109,26 @@ static bool HasExtension(const Preprocessor &PP, StringRef Extension) {
if (HasFeature(PP, Extension))
return true;
+ // Normalize the extension name, __foo__ becomes foo.
+ if (Extension.starts_with("__") && Extension.ends_with("__") &&
+ Extension.size() >= 4)
+ Extension = Extension.substr(2, Extension.size() - 4);
+
+ const LangOptions &LangOpts = PP.getLangOpts();
+#define ABI_EXTENSION(Name, Predicate) .Case(#Name, Predicate)
+ bool IsABIExtension = llvm::StringSwitch<bool>(Extension)
+#include "clang/Basic/Features.def"
+ .Default(false);
+#undef ABI_EXTENSION
+ if (IsABIExtension)
+ return true;
+
// If the use of an extension results in an error diagnostic, extensions are
// effectively unavailable, so just return false here.
if (PP.getDiagnostics().getExtensionHandlingBehavior() >=
diag::Severity::Error)
return false;
- const LangOptions &LangOpts = PP.getLangOpts();
-
- // Normalize the extension name, __foo__ becomes foo.
- if (Extension.starts_with("__") && Extension.ends_with("__") &&
- Extension.size() >= 4)
- Extension = Extension.substr(2, Extension.size() - 4);
-
// Because we inherit the feature list from HasFeature, this string switch
// must be less restrictive than HasFeature's.
#define EXTENSION(Name, Predicate) .Case(#Name, Predicate)
diff --git a/clang/test/Sema/ptrauth-qualifier.c b/clang/test/Sema/ptrauth-qualifier.c
index 5d932b724f07a..358666025fca5 100644
--- a/clang/test/Sema/ptrauth-qualifier.c
+++ b/clang/test/Sema/ptrauth-qualifier.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple arm64-apple-ios -std=c23 -fsyntax-only -verify -fptrauth-intrinsics %s
// RUN: %clang_cc1 -triple aarch64-linux-gnu -std=c23 -fsyntax-only -verify -fptrauth-intrinsics %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -std=c23 -fsyntax-only -verify -fptrauth-intrinsics -pedantic-errors -Wno-c2y-extensions %s
#if !__has_extension(ptrauth_qualifier)
// This error means that the __ptrauth qualifier availability test says that it
|
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 seems fine to me as well.
| // features, whereas EXTENSION(...) should be used for clang extensions. Note | ||
| // that many of the identifiers in this file don't follow this rule for backward | ||
| // compatibility reasons. | ||
| // |
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.
Maybe also mention ABI_EXTENSION here?
|
@AaronBallman I'm not sure this is the correct approach but will leave it up for now |
|
Closing, as for the current critical case (ptrauth) we're keeping the tests as a feature when the target is arm64e, and adding a |
…-errors
This is a targeted change to allow us to specify that an extension test should return true even if -pedantic-errors is enabled.
In the longer term we may want to consider changing the behavior of -pedantic-errors to not gate __has_extension checks, and become more in line with
-Wpedantic -Werror.