[clang] Defer handling of function type attributes in type aliases #167426
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-format Author: None (PiJoules) ChangesFunction type attributes like cfi_unchecked_callee and regparm couldn't be used in This would result in the warning This patch defers the function attribute handling until we have the full type and allows function type attributes to be attached to the alias with some tests for cfi_unchecked_callee and regparm. This does however change the diagnostic for Full diff: https://github.com/llvm/llvm-project/pull/167426.diff 7 Files Affected:
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index eb8b1352d1be1..5e2d22f910d04 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5421,6 +5421,31 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
processTypeAttrs(state, T, TAL_DeclChunk, DeclType.getAttrs(),
S.CUDA().IdentifyTarget(D.getAttributes()));
+ // Handle attributes on the declaration specifier that were deferred up
+ // until now when we have the full type of the declarator.
+ if (D.getContext() == DeclaratorContext::AliasDecl) {
+ CUDAFunctionTarget CFT =
+ S.CUDA().IdentifyTarget(D.getDeclSpec().getAttributes());
+ for (ParsedAttr &attr : D.getDeclSpec().getAttributes()) {
+ if (!(attr.isStandardAttributeSyntax() ||
+ attr.isRegularKeywordAttribute()) ||
+ !attr.isTypeAttr() || !attr.isUsedAsTypeAttr())
+ continue;
+
+ switch (attr.getKind()) {
+ FUNCTION_TYPE_ATTRS_CASELIST: {
+ if (!handleFunctionTypeAttr(state, attr, T, CFT)) {
+ diagnoseBadTypeAttribute(state.getSema(), attr, T);
+ attr.setInvalid();
+ }
+ break;
+ }
+ default:
+ break;
+ }
+ }
+ }
+
if (DeclType.Kind != DeclaratorChunk::Paren) {
if (ExpectNoDerefChunk && !IsNoDerefableChunk(DeclType))
S.Diag(DeclType.Loc, diag::warn_noderef_on_non_pointer_or_array);
@@ -9078,7 +9103,14 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
// appertain to and hence should not use the "distribution" logic below.
if (attr.isStandardAttributeSyntax() ||
attr.isRegularKeywordAttribute()) {
- if (!handleFunctionTypeAttr(state, attr, type, CFT)) {
+ // If we are in a C++ type alias via `using` where the attribute is
+ // placed after the alias name but before the `=`, the type here may
+ // refer to just the return type of a function rather than the function
+ // as a whole. In this specific case, defer handling the function type
+ // attribute until we have the full type of the declarator.
+ bool InAliasDecl =
+ state.getDeclarator().getContext() == DeclaratorContext::AliasDecl;
+ if (!handleFunctionTypeAttr(state, attr, type, CFT) && !InAliasDecl) {
diagnoseBadTypeAttribute(state.getSema(), attr, type);
attr.setInvalid();
}
diff --git a/clang/test/AST/cfi-unchecked-callee.cpp b/clang/test/AST/cfi-unchecked-callee.cpp
index af84996835930..9bce8c1aa660c 100644
--- a/clang/test/AST/cfi-unchecked-callee.cpp
+++ b/clang/test/AST/cfi-unchecked-callee.cpp
@@ -7,3 +7,6 @@ void func(void);
// CHECK-NEXT: FunctionDecl {{0x[a-z0-9]*}} prev [[PTR]] {{.*}}func 'void () __attribute__((cfi_unchecked_callee))'
void func(void) {}
+
+// CHECK: TypeAliasDecl {{0x[a-z0-9]*}} {{.*}}CfiCheckFunction 'void () __attribute__((cfi_unchecked_callee))'
+using CfiCheckFunction [[clang::cfi_unchecked_callee]] = void(void);
diff --git a/clang/test/AST/regparm.cpp b/clang/test/AST/regparm.cpp
new file mode 100644
index 0000000000000..0dc3fcfb717f3
--- /dev/null
+++ b/clang/test/AST/regparm.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -ast-dump %s | FileCheck %s
+
+// CHECK: TypeAliasDecl {{0x[a-z0-9]*}} {{.*}}T1 'void (int) __attribute__((regparm (2)))'
+using T1 [[gnu::regparm(2)]] = void(int);
diff --git a/clang/test/Format/regression-test.cpp b/clang/test/Format/regression-test.cpp
new file mode 100644
index 0000000000000..cc227730740d0
--- /dev/null
+++ b/clang/test/Format/regression-test.cpp
@@ -0,0 +1,7 @@
+// RUN: grep -Ev "// *[A-Z-]+" %s | clang-format --style='{BasedOnStyle: google, ColumnLimit: 0}' | FileCheck %s -strict-whitespace
+
+// CHECK: ::test_anonymous::FunctionApplication& ::test_anonymous::FunctionApplication::operator=(const ::test_anonymous::FunctionApplication& other) noexcept {
+::test_anonymous::FunctionApplication& ::test_anonymous::FunctionApplication::operator=(const ::test_anonymous::FunctionApplication& other) noexcept {
+ storage_ = other.CloneStorage_();
+ return *this;
+}
diff --git a/clang/test/Frontend/cfi-unchecked-callee-attribute.cpp b/clang/test/Frontend/cfi-unchecked-callee-attribute.cpp
index a5a17dd5a4d82..4c0b5fe882ac3 100644
--- a/clang/test/Frontend/cfi-unchecked-callee-attribute.cpp
+++ b/clang/test/Frontend/cfi-unchecked-callee-attribute.cpp
@@ -238,3 +238,5 @@ void lambdas() {
CFI_UNCHECKED_CALLEE
void func(void);
void func(void) {} // No warning expected.
+
+using CfiCheckFunction [[clang::cfi_unchecked_callee]] = void(void* entry, const void* diag_data);
diff --git a/clang/test/Frontend/regparm.cpp b/clang/test/Frontend/regparm.cpp
new file mode 100644
index 0000000000000..d1416116be3f7
--- /dev/null
+++ b/clang/test/Frontend/regparm.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown %s -verify
+// expected-no-diagnostics
+
+using T1 [[gnu::regparm(2)]] = void(int);
diff --git a/clang/test/Parser/cxx0x-keyword-attributes.cpp b/clang/test/Parser/cxx0x-keyword-attributes.cpp
index be7423cc7ecee..70b6d0acb615a 100644
--- a/clang/test/Parser/cxx0x-keyword-attributes.cpp
+++ b/clang/test/Parser/cxx0x-keyword-attributes.cpp
@@ -137,10 +137,10 @@ using ATTR_USE alignas(4)ATTR_USE ns::i; // expected-warning 2 {{ISO C+
expected-error 2 {{'ATTR_NAME' only applies to non-K&R-style functions}}
using ATTR_USE alignas(4) ATTR_USE foobar = int; // expected-error {{'ATTR_NAME' cannot appear here}} \
expected-error {{'alignas' attribute only applies to}} \
- expected-error 2 {{'ATTR_NAME' only applies to function types}}
+ expected-error 2 {{'ATTR_NAME' only applies to non-K&R-style functions}}
ATTR_USE using T = int; // expected-error {{'ATTR_NAME' cannot appear here}}
-using T ATTR_USE = int; // expected-error {{'ATTR_NAME' only applies to function types}}
+using T ATTR_USE = int; // expected-error {{'ATTR_NAME' only applies to non-K&R-style functions}}
template<typename T> using U ATTR_USE = T; // expected-error {{'ATTR_NAME' only applies to function types}}
using ns::i ATTR_USE; // expected-warning {{ISO C++}} \
expected-error {{'ATTR_NAME' only applies to non-K&R-style functions}}
@@ -162,7 +162,7 @@ struct using_in_struct : using_in_struct_base {
using ATTR_USE ns::i; // expected-warning {{ISO C++}} \
expected-error {{'ATTR_NAME' cannot appear here}} \
expected-error {{'ATTR_NAME' only applies to non-K&R-style functions}}
-using T ATTR_USE = int; // expected-error {{'ATTR_NAME' only applies to function types}}
+using T ATTR_USE = int; // expected-error {{'ATTR_NAME' only applies to non-K&R-style functions}}
auto trailing() -> ATTR_USE const int; // expected-error {{'ATTR_NAME' cannot appear here}}
auto trailing() -> const ATTR_USE int; // expected-error {{'ATTR_NAME' cannot appear here}}
|
Function type attributes like cfi_unchecked_callee and regparm couldn't be used in `using` type aliases via the syntax: ``` using T1 [[clang::cfi_unchecked_callee]] = void(int); ``` This would result in the warning `'cfi_unchecked_callee' only applies to function types; type here is 'void' [-Wignored-attributes]` where the type is incorrect. This only seems to occur when the type is a function type and we're only handling the return type. This patch defers the function attribute handling until we have the full type and allows function type attributes to be attached to the alias with some tests for cfi_unchecked_callee and regparm. This does however change the diagnostic for `__arm_streaming` in a type alias to a slightly different one.
PiJoules
force-pushed
the
cfi-unchecked-callee-type-alias
branch
from
a622990 to
47b8788
Compare
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
This doesn't really FEEL correct here, but I don't know enough about this section of how Declarators + attributes are handled, so @AaronBallman is going to have to review this.
|
I'm not qualified to review this, but I wonder why you insist on the syntax: It seems that attributes in this position appertain to the type alias itself (see here), for example, you can use it to mark a type alias as deprecated, see Godbolt demo: https://godbolt.org/z/88oodMqes The syntax for what you want to do seems to be: Some other alternatives that I tried (T2, T3) do not seem to work: https://godbolt.org/z/YefKfafjb |
Yeah the use case is if we already have a function type without this attribute, then we can attach the attribute to the alias but without needing to change the original aliasee type. There's nothing stopping us from using the |
It's a type attribute, so it goes on the function type. Putting it on the alias declaration makes it a declaration attribute, not a type attribute. If you make it have type attribute effects on the type alias declaration, now the alias declaration effectively has a different type than the type it references, which I expect will lead to confusion (in addition to giving type semantics to a declaration attribute, which is not something we should be doing with this attribute syntax). I think the correct thing is: |
6 participants
Function type attributes like cfi_unchecked_callee and regparm couldn't be used in
usingtype aliases via the syntax:This would result in the warning
'cfi_unchecked_callee' only applies to function types; type here is 'void' [-Wignored-attributes]where the type is incorrect. This only seems to occur when the type is a function type and we're only handling the return type.This patch defers the function attribute handling until we have the full type and allows function type attributes to be attached to the alias with some tests for cfi_unchecked_callee and regparm. This does however change the diagnostic for
__arm_streamingin a type alias to a slightly different one.