Fix an accepts-invalid issue with [[]] attributes in the type position in C

[aaron.ballman]

Summary

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.

[erichkeane]

I guess I don't see how this patch 'works' here. I don't see anything preventing the type-location in C, but not C++?

clang/test/Sema/attr-c2x.c

14

Do we have a test to validate that the previous syntax isn't allowed? I don't see any being added, but I might be missing it.

[aaron.ballman]

I guess I don't see how this patch 'works' here. I don't see anything preventing the type-location in C, but not C++?

See comments in the code, I left one where the interesting bit is.

clang/lib/Sema/SemaType.cpp

8091

This bit right here is the important bit for diagnosing in the type position (similar changes are fixing similar issues). isCXX11Attribute() tests that the attribute syntax is AS_CXX11 exclusively and ignores that there's a AS_C2x -- the fix is to check for either parsed syntax.

clang/test/Sema/attr-c2x.c

14

It looks like we don't have any coverage that you can't apply overloadable to a type, I can add it.

[aaron.ballman]

Added a test that overloadable should fail when in the type position.

[aaron.ballman]

Thanks for the review, Erich! I've committed in bc7cc2074b7b7043e05cb46346f1368eb4ae9949

clang/include/clang/Basic/AttributeCommonInfo.h

public:
   }
 
   bool isCXX11Attribute() const {
     return SyntaxUsed == AS_CXX11 || isAlignasAttribute();
   }
 
   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;
   }
 
   bool isContextSensitiveKeywordAttribute() const {
     return SyntaxUsed == AS_ContextSensitiveKeyword;
   }
 

clang/lib/Parse/Parser.cpp

if (Tok.isNot(tok::l_brace) &&
     if (Tok.isNot(tok::l_brace))
       return nullptr;
   }
 
   // Check to make sure that any normal attributes are allowed to be on
   // 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;
   }
 
   // In delayed template parsing mode, for function template we consume the
   // tokens and store them for late parsing at the end of the translation unit.
   if (getLangOpts().DelayedTemplateParsing && Tok.isNot(tok::equal) &&
       TemplateInfo.Kind == ParsedTemplateInfo::Template &&
       Actions.canDelayFunctionBody(D)) {

clang/lib/Sema/SemaDeclAttr.cpp

 static void handleAnalyzerNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   // The checking path for 'noreturn' and 'analyzer_noreturn' are different
   // because 'analyzer_noreturn' does not impact the type.
   if (!isFunctionOrMethodOrBlock(D)) {
     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;
       return;
     }
   }
 
   D->addAttr(::new (S.Context) AnalyzerNoReturnAttr(S.Context, AL));
static void handleWarnUnusedResult(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
   if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
     if (MD->getReturnType()->isVoidType()) {
       S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 1;
       return;
     }
 
   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))
       S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str)
           << AL << "functions, classes, or enumerations";
 
     // If this is spelled as the standard C++17 attribute, but not in C++17,
     // warn about using it as an extension. If there are attribute arguments,
static void handleDeprecatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   // Handle the cases where the attribute has a text message.
   StringRef Str, Replacement;
   if (AL.isArgExpr(0) && AL.getArgAsExpr(0) &&
       !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))
     return;
 
   if (!S.getLangOpts().CPlusPlus14 && AL.isCXX11Attribute() && !AL.isGNUScope())
     S.Diag(AL.getLoc(), diag::ext_cxx14_attr) << AL;
 
static void handleNoSanitizeSpecificAttr(Sema &S, Decl *D,
   // NoSanitizeAttr object; but we need to calculate the correct spelling list
   // index rather than incorrectly assume the index for NoSanitizeSpecificAttr
   // has the same spellings as the index for NoSanitizeAttr. We don't have a
   // general way to "translate" between the two, so this hack attempts to work
   // around the issue with hard-coded indicies. This is critical for calling
   // 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;
   Info.setAttributeSpellingListIndex(TranslatedSpellingIndex);
   D->addAttr(::new (S.Context)
                  NoSanitizeAttr(S.Context, Info, &SanitizerName, 1));
 }
 

clang/lib/Sema/SemaType.cpp

 static void distributeFunctionTypeAttrFromDeclSpec(TypeProcessingState &state,
                                                    ParsedAttr &attr,
                                                    QualType &declSpecType) {
   state.saveDeclSpecAttrs();
 
   // 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;
   }
 
   // Try to distribute to the innermost.
   if (distributeFunctionTypeAttrToInnermost(
           state, attr, state.getCurrentAttributes(), declSpecType))
static void distributeTypeAttrsFromDeclarator(TypeProcessingState &state,
   // Collect all the type attributes from the declarator itself.
   assert(!state.getDeclarator().getAttributes().empty() &&
          "declarator has no attrs!");
   // The called functions in this loop actually remove things from the current
   // list, so iterating over the existing list isn't possible.  Instead, make a
   // 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()) {
     OBJC_POINTER_TYPE_ATTRS_CASELIST:
       distributeObjCPointerTypeAttrFromDeclarator(state, attr, declSpecType);
       break;
 
     FUNCTION_TYPE_ATTRS_CASELIST:
static void processTypeAttrs(TypeProcessingState &state, QualType &type,
   state.setParsedNoDeref(false);
 
   for (ParsedAttr &attr : AttrsCopy) {
 
     // Skip attributes that were marked to be invalid.
     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
       // incompatibility.
       if (attr.isGNUScope()) {
         bool IsTypeAttr = attr.isTypeAttr();
         if (TAL == TAL_DeclChunk) {
           state.getSema().Diag(attr.getLoc(),
if (attr.isStandardAttributeSyntax()) {
         continue;
       }
     }
 
     // If this is an attribute we can handle, do so now,
     // 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();
       }
       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();
       break;
 
     case ParsedAttr::IgnoredAttribute:
       break;
 

clang/test/AST/ast-dump-c-attr.c

 // CHECK:      FunctionDecl{{.*}}Test9
 // CHECK-NEXT:   ParmVarDecl{{.*}}Test10
 // CHECK-NEXT:     DeprecatedAttr 0x{{[^ ]*}} <col:40> "" ""
 // CHECK-NEXT:   DeprecatedAttr 0x{{[^ ]*}} <col: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> "" ""

clang/test/Sema/attr-availability-square-brackets.c

 // 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}}
 }
 
 enum
 [[clang::availability(macos, unavailable)]]
 {

clang/test/Sema/attr-c2x.c

 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=c2x %s
 
 struct S {};
 struct S * [[clang::address_space(1)]] Foo;
 
 enum [[clang::enum_extensibility(open)]] EnumOpen {
   C0 = 1, C1 = 10
 };
 
 enum [[clang::flag_enum]] EnumFlag {
   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}}
 }
 
 [[nodiscard]] int without_underscores(void);
 [[__nodiscard__]] int underscores(void);
 
 // Match GCC's behavior for C attributes as well.

clang/test/Sema/attr-deprecated-c2x.c

-// 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}}
 
 int a() {
   int (*ptr)() = f; // expected-warning {{'f' is deprecated}}
   f(); // expected-warning {{'f' is deprecated}}
 
   // test if attributes propagate to functions
   g(); // expected-warning {{'g' is deprecated}}
 
   return var; // expected-warning {{'var' is deprecated}}
 }
 
 // test if attributes propagate to variables
 extern int var;
 int w() {
   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}}
 
 int old_fn() {
   return old_fn()+1;  // no warning, deprecated functions can use deprecated symbols.
 }
 
 struct foo {
struct [[deprecated, // expected-note {{'bar_dep' has been explicitly marked deprecated here}}
          invalid_attribute]] bar_dep ;  // expected-warning {{unknown attribute 'invalid_attribute' ignored}}
 
 struct bar_dep *test3;   // expected-warning {{'bar_dep' is deprecated}}
 
 [[deprecated("this is the message")]] int i; // expected-note {{'i' has been explicitly marked deprecated here}}
 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}}

clang/test/Sema/attr-external-source-symbol.c

 
 void namedDeclsOnly() {
   int (^block)(void) = ^ (void)
     __attribute__((external_source_symbol(language="Swift"))) { // expected-warning {{'external_source_symbol' attribute only applies to named declarations}}
       return 1;
   };
 }
 
-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();

clang/test/Sema/c2x-maybe_unused-errors.c

 // RUN: %clang_cc1 -fsyntax-only -Wunused -std=c2x -verify %s
 
 struct [[maybe_unused]] S1 { // ok
   int a [[maybe_unused]];
 };
 struct [[maybe_unused, maybe_unused]] S2 { // ok
   int a;
 };
 struct [[maybe_unused("Wrong")]] S3 { // expected-error {{'maybe_unused' cannot have an argument list}}
   int a;
 };
 
+void func(void) {
+  int a[10] [[maybe_unused]]; // expected-error {{'maybe_unused' attribute cannot be applied to types}}
+}