aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, rsmith.
aaron.ballman requested review of this revision.
Herald added a project: clang.
A user reported an issue to me via email that Clang was accepting some code
that GCC was rejecting. After investigation, it turned out to be a general
problem of us failing to properly reject attributes written in the type
position in C when they don't apply to types. The root cause was a terminology
issue -- we sometimes use "CXX11Attr" to mean [[]] in C++11 mode and sometimes
[[]] in general -- and this came back to bite us because in this particular
case, it really meant [[]] in C++ mode.
I fixed the issue by introducing a new function
`AttributeCommonInfo::isStandardAttributeSyntax()` to represent [[]] in either
C or C++ mode. I toyed with `isDoubleSquareBracketAttributeSyntax()` instead,
but because of attributes like `alignas`, it didn't feel like a particularly
good match.
This fix pointed out that we've had the issue in some of our existing tests,
which have all been corrected. This resolves
https://bugs.llvm.org/show_bug.cgi?id=50954.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105287
Files:
clang/include/clang/Basic/AttributeCommonInfo.h
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/ast-dump-c-attr.c
clang/test/Sema/attr-availability-square-brackets.c
clang/test/Sema/attr-c2x.c
clang/test/Sema/attr-deprecated-c2x.c
clang/test/Sema/attr-external-source-symbol.c
clang/test/Sema/c2x-maybe_unused-errors.c
Index: clang/test/Sema/c2x-maybe_unused-errors.c
===================================================================
--- clang/test/Sema/c2x-maybe_unused-errors.c
+++ clang/test/Sema/c2x-maybe_unused-errors.c
@@ -10,3 +10,6 @@
int a;
};
+void func(void) {
+ int a[10] [[maybe_unused]]; // expected-error {{'maybe_unused' attribute cannot be applied to types}}
+}
Index: clang/test/Sema/attr-external-source-symbol.c
===================================================================
--- clang/test/Sema/attr-external-source-symbol.c
+++ clang/test/Sema/attr-external-source-symbol.c
@@ -18,14 +18,14 @@
};
}
-void threeClauses2() [[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration)]];
+[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration)]] void threeClauses2();
-void twoClauses2() [[clang::external_source_symbol(language="Swift", defined_in="module")]];
+[[clang::external_source_symbol(language="Swift", defined_in="module")]] void twoClauses2();
-void fourClauses2()
-[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration)]]; // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
+[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration)]] // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
+void fourClauses2();
-void oneClause2() [[clang::external_source_symbol(generated_declaration)]];
+[[clang::external_source_symbol(generated_declaration)]] void oneClause2();
-void noArguments2()
-[[clang::external_source_symbol]]; // expected-error {{'external_source_symbol' attribute takes at least 1 argument}}
+[[clang::external_source_symbol]] // expected-error {{'external_source_symbol' attribute takes at least 1 argument}}
+void noArguments2();
Index: clang/test/Sema/attr-deprecated-c2x.c
===================================================================
--- clang/test/Sema/attr-deprecated-c2x.c
+++ clang/test/Sema/attr-deprecated-c2x.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only --std=c2x
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c2x
-int f() [[deprecated]]; // expected-note 2 {{'f' has been explicitly marked deprecated here}}
-void g() [[deprecated]];// expected-note {{'g' has been explicitly marked deprecated here}}
+[[deprecated]] int f(); // expected-note 2 {{'f' has been explicitly marked deprecated here}}
+[[deprecated]] void g();// expected-note {{'g' has been explicitly marked deprecated here}}
void g();
extern int var [[deprecated]]; // expected-note 2 {{'var' has been explicitly marked deprecated here}}
@@ -22,7 +22,7 @@
return var; // expected-warning {{'var' is deprecated}}
}
-int old_fn() [[deprecated]];// expected-note {{'old_fn' has been explicitly marked deprecated here}}
+[[deprecated]] int old_fn();// expected-note {{'old_fn' has been explicitly marked deprecated here}}
int old_fn();
int (*fn_ptr)() = old_fn; // expected-warning {{'old_fn' is deprecated}}
@@ -52,3 +52,7 @@
void test4(void) {
i = 12; // expected-warning {{'i' is deprecated: this is the message}}
}
+
+// Ensure that deprecated only accepts one argument, not the replacement
+// argument supported as a GNU extension.
+[[deprecated("message", "replacement not supported")]] void test5(void); // expected-error {{'deprecated' attribute takes no more than 1 argument}}
Index: clang/test/Sema/attr-c2x.c
===================================================================
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -11,16 +11,16 @@
D0 = 1, D1 = 8
};
-void foo(void *c) [[clang::overloadable]];
-void foo(char *c) [[clang::overloadable]];
+[[clang::overloadable]] void foo(void *c);
+[[clang::overloadable]] void foo(char *c);
void context_okay(void *context [[clang::swift_context]]) [[clang::swiftcall]];
void context_okay2(void *context [[clang::swift_context]], void *selfType, char **selfWitnessTable) [[clang::swiftcall]];
-void *f1(void) [[clang::ownership_returns(foo)]];
-void *f2() [[clang::ownership_returns(foo)]]; // expected-warning {{'ownership_returns' attribute only applies to non-K&R-style functions}}
+[[clang::ownership_returns(foo)]] void *f1(void);
+[[clang::ownership_returns(foo)]] void *f2(); // expected-warning {{'ownership_returns' attribute only applies to non-K&R-style functions}}
-void foo2(void) [[clang::unavailable("not available - replaced")]]; // expected-note {{'foo2' has been explicitly marked unavailable here}}
+[[clang::unavailable("not available - replaced")]] void foo2(void); // expected-note {{'foo2' has been explicitly marked unavailable here}}
void bar(void) {
foo2(); // expected-error {{'foo2' is unavailable: not available - replaced}}
}
Index: clang/test/Sema/attr-availability-square-brackets.c
===================================================================
--- clang/test/Sema/attr-availability-square-brackets.c
+++ clang/test/Sema/attr-availability-square-brackets.c
@@ -1,11 +1,12 @@
// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
-void f0() [[clang::availability(macosx,introduced=10.4,deprecated=10.2)]]; // expected-warning{{feature cannot be deprecated in macOS version 10.2 before it was introduced in version 10.4; attribute ignored}}
-void f1() [[clang::availability(ios,obsoleted=2.1,deprecated=3.0)]]; // expected-warning{{feature cannot be obsoleted in iOS version 2.1 before it was deprecated in version 3.0; attribute ignored}}
-void f2() [[clang::availability(ios,introduced=2.1,deprecated=2.1)]];
+[[clang::availability(macosx,introduced=10.4,deprecated=10.2)]] void f0(); // expected-warning{{feature cannot be deprecated in macOS version 10.2 before it was introduced in version 10.4; attribute ignored}}
+[[clang::availability(ios,obsoleted=2.1,deprecated=3.0)]] void f1(); // expected-warning{{feature cannot be obsoleted in iOS version 2.1 before it was deprecated in version 3.0; attribute ignored}}
+[[clang::availability(ios,introduced=2.1,deprecated=2.1)]] void f2();
+[[clang::availability(macosx,introduced=8.0,deprecated=9.0, message="use CTFontCopyFullName")]]
extern void
-ATSFontGetName(const char *oName) [[clang::availability(macosx,introduced=8.0,deprecated=9.0, message="use CTFontCopyFullName")]]; // expected-note {{'ATSFontGetName' has been explicitly marked deprecated here}}
+ATSFontGetName(const char *oName); // expected-note {{'ATSFontGetName' has been explicitly marked deprecated here}}
void test_10095131() {
ATSFontGetName("Hello"); // expected-warning {{'ATSFontGetName' is deprecated: first deprecated in macOS 9.0 - use CTFontCopyFullName}}
Index: clang/test/AST/ast-dump-c-attr.c
===================================================================
--- clang/test/AST/ast-dump-c-attr.c
+++ clang/test/AST/ast-dump-c-attr.c
@@ -52,8 +52,3 @@
void Test11 [[deprecated]](void);
// CHECK: FunctionDecl{{.*}}Test11
// CHECK-NEXT: DeprecatedAttr 0x{{[^ ]*}} <col:15> "" ""
-
-void Test12(void) [[deprecated]] {}
-// CHECK: FunctionDecl{{.*}}Test12
-// CHECK-NEXT: CompoundStmt
-// CHECK-NEXT: DeprecatedAttr 0x{{[^ ]*}} <col:21> "" ""
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -634,7 +634,7 @@
// C++11 attributes before the decl specifiers actually appertain to
// the declarators. Move them straight there. We don't support the
// 'put them wherever you like' semantics we allow for GNU attributes.
- if (attr.isCXX11Attribute()) {
+ if (attr.isStandardAttributeSyntax()) {
moveAttrFromListToList(attr, state.getCurrentAttributes(),
state.getDeclarator().getAttributes());
return;
@@ -687,9 +687,9 @@
// non-owning copy and iterate over that.
ParsedAttributesView AttrsCopy{state.getDeclarator().getAttributes()};
for (ParsedAttr &attr : AttrsCopy) {
- // Do not distribute C++11 attributes. They have strict rules for what
+ // Do not distribute [[]] attributes. They have strict rules for what
// they appertain to.
- if (attr.isCXX11Attribute())
+ if (attr.isStandardAttributeSyntax())
continue;
switch (attr.getKind()) {
@@ -8058,7 +8058,7 @@
if (attr.isInvalid())
continue;
- if (attr.isCXX11Attribute()) {
+ if (attr.isStandardAttributeSyntax()) {
// [[gnu::...]] attributes are treated as declaration attributes, so may
// not appertain to a DeclaratorChunk. If we handle them as type
// attributes, accept them in that position and diagnose the GCC
@@ -8087,8 +8087,8 @@
// otherwise, add it to the FnAttrs list for rechaining.
switch (attr.getKind()) {
default:
- // A C++11 attribute on a declarator chunk must appertain to a type.
- if (attr.isCXX11Attribute() && TAL == TAL_DeclChunk) {
+ // A [[]] attribute on a declarator chunk must appertain to a type.
+ if (attr.isStandardAttributeSyntax() && TAL == TAL_DeclChunk) {
state.getSema().Diag(attr.getLoc(), diag::err_attribute_not_type_attr)
<< attr;
attr.setUsedAsTypeAttr();
@@ -8096,7 +8096,7 @@
break;
case ParsedAttr::UnknownAttribute:
- if (attr.isCXX11Attribute() && TAL == TAL_DeclChunk)
+ if (attr.isStandardAttributeSyntax() && TAL == TAL_DeclChunk)
state.getSema().Diag(attr.getLoc(),
diag::warn_unknown_attribute_ignored)
<< attr << attr.getRange();
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2093,7 +2093,7 @@
ValueDecl *VD = dyn_cast<ValueDecl>(D);
if (!VD || (!VD->getType()->isBlockPointerType() &&
!VD->getType()->isFunctionPointerType())) {
- S.Diag(AL.getLoc(), AL.isCXX11Attribute()
+ S.Diag(AL.getLoc(), AL.isStandardAttributeSyntax()
? diag::err_attribute_wrong_decl_type
: diag::warn_attribute_wrong_decl_type)
<< AL << ExpectedFunctionMethodOrBlock;
@@ -2863,7 +2863,7 @@
}
StringRef Str;
- if ((AL.isCXX11Attribute() || AL.isC2xAttribute()) && !AL.getScopeName()) {
+ if (AL.isStandardAttributeSyntax() && !AL.getScopeName()) {
// The standard attribute cannot be applied to variable declarations such
// as a function pointer.
if (isa<VarDecl>(D))
@@ -7280,8 +7280,8 @@
!S.checkStringLiteralArgumentAttr(AL, 0, Str))
return;
- // Only support a single optional message for Declspec and CXX11.
- if (AL.isDeclspecAttribute() || AL.isCXX11Attribute())
+ // Support a single optional message only for Declspec and [[]] spellings.
+ if (AL.isDeclspecAttribute() || AL.isStandardAttributeSyntax())
AL.checkAtMostNumArgs(S, 1);
else if (AL.isArgExpr(1) && AL.getArgAsExpr(1) &&
!S.checkStringLiteralArgumentAttr(AL, 1, Replacement))
@@ -7348,7 +7348,7 @@
// getSpelling() or prettyPrint() on the resulting semantic attribute object
// without failing assertions.
unsigned TranslatedSpellingIndex = 0;
- if (AL.isC2xAttribute() || AL.isCXX11Attribute())
+ if (AL.isStandardAttributeSyntax())
TranslatedSpellingIndex = 1;
AttributeCommonInfo Info = AL;
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1214,7 +1214,7 @@
// a definition. Late parsed attributes are checked at the end.
if (Tok.isNot(tok::equal)) {
for (const ParsedAttr &AL : D.getAttributes())
- if (AL.isKnownToGCC() && !AL.isCXX11Attribute())
+ if (AL.isKnownToGCC() && !AL.isStandardAttributeSyntax())
Diag(AL.getLoc(), diag::warn_attribute_on_function_definition) << AL;
}
Index: clang/include/clang/Basic/AttributeCommonInfo.h
===================================================================
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -155,6 +155,12 @@
bool isC2xAttribute() const { return SyntaxUsed == AS_C2x; }
+ /// The attribute is spelled [[]] in either C or C++ mode, including standard
+ /// attributes spelled with a keyword, like alignas.
+ bool isStandardAttributeSyntax() const {
+ return isCXX11Attribute() || isC2xAttribute();
+ }
+
bool isKeywordAttribute() const {
return SyntaxUsed == AS_Keyword || SyntaxUsed == AS_ContextSensitiveKeyword;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
