Make parsing of parentheses mandatory when an attribute keyword takes arguments [to squash]

sdesmalen-arm · sdesmalen-arm · commit 7de32cad18d6 · 2024-01-12T11:35:33.000Z
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -256,13 +256,10 @@ class AttributeCommonInfo {
   }
 };
 
-enum class KeywordAttributeParseArgumentsKind { None, Optional, Required };
-
-inline KeywordAttributeParseArgumentsKind
-getKeywordAttributeParseArgumentsKind(tok::TokenKind Kind) {
+inline bool doesKeywordAttributeTakeArgs(tok::TokenKind Kind) {
   switch (Kind) {
   default:
-    return KeywordAttributeParseArgumentsKind::None;
+    return false;
 #define KEYWORD_ATTRIBUTE(NAME, HASARG)                                        \
   case tok::kw_##NAME:                                                         \
     return HASARG;
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
@@ -6786,9 +6786,9 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
     } else if (Tok.isRegularKeywordAttribute()) {
       // For consistency with attribute parsing.
       Diag(Tok, diag::err_keyword_not_allowed) << Tok.getIdentifierInfo();
-      auto ParseArgsKind = getKeywordAttributeParseArgumentsKind(Tok.getKind());
+      bool TakesArgs = doesKeywordAttributeTakeArgs(Tok.getKind());
       ConsumeToken();
-      if (ParseArgsKind != KeywordAttributeParseArgumentsKind::None) {
+      if (TakesArgs) {
         BalancedDelimiterTracker T(*this, tok::l_paren);
         if (!T.consumeOpen())
           T.skipToEnd();
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -1890,10 +1890,9 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
         if (!SkipUntil(tok::r_paren, StopAtSemi))
           break;
       } else if (Tok.isRegularKeywordAttribute()) {
-        auto ParseArgsKind =
-            getKeywordAttributeParseArgumentsKind(Tok.getKind());
+        bool TakesArgs = doesKeywordAttributeTakeArgs(Tok.getKind());
         ConsumeToken();
-        if (ParseArgsKind != KeywordAttributeParseArgumentsKind::None) {
+        if (TakesArgs) {
           BalancedDelimiterTracker T(*this, tok::l_paren);
           if (!T.consumeOpen())
             T.skipToEnd();
@@ -4545,18 +4544,16 @@ void Parser::ParseCXX11AttributeSpecifierInternal(ParsedAttributes &Attrs,
     SourceLocation Loc = Tok.getLocation();
     IdentifierInfo *AttrName = Tok.getIdentifierInfo();
     ParsedAttr::Form Form = ParsedAttr::Form(Tok.getKind());
-    KeywordAttributeParseArgumentsKind ParseArgsKind =
-        getKeywordAttributeParseArgumentsKind(Tok.getKind());
+    bool TakesArgs = doesKeywordAttributeTakeArgs(Tok.getKind());
     ConsumeToken();
-    if (ParseArgsKind == KeywordAttributeParseArgumentsKind::Required &&
-        !Tok.is(tok::l_paren))
-      Diag(Tok.getLocation(), diag::err_expected_lparen_after) << AttrName;
-    if (ParseArgsKind != KeywordAttributeParseArgumentsKind::None &&
-        Tok.is(tok::l_paren))
-      ParseAttributeArgsCommon(AttrName, Loc, Attrs, EndLoc,
-                               /*ScopeName*/ nullptr,
-                               /*ScopeLoc*/ Loc, Form);
-    else
+    if (TakesArgs) {
+      if (!Tok.is(tok::l_paren))
+        Diag(Tok.getLocation(), diag::err_expected_lparen_after) << AttrName;
+      else
+        ParseAttributeArgsCommon(AttrName, Loc, Attrs, EndLoc,
+                                 /*ScopeName*/ nullptr,
+                                 /*ScopeLoc*/ Loc, Form);
+    } else
       Attrs.addNew(AttrName, Loc, nullptr, Loc, nullptr, 0, Form);
     return;
   }
@@ -4724,8 +4721,7 @@ SourceLocation Parser::SkipCXX11Attributes() {
       T.skipToEnd();
       EndLoc = T.getCloseLocation();
     } else if (Tok.isRegularKeywordAttribute() &&
-               getKeywordAttributeParseArgumentsKind(Tok.getKind()) ==
-                   KeywordAttributeParseArgumentsKind::None) {
+               !doesKeywordAttributeTakeArgs(Tok.getKind())) {
       EndLoc = Tok.getLocation();
       ConsumeToken();
     } else {
diff --git a/clang/lib/Parse/ParseTentative.cpp b/clang/lib/Parse/ParseTentative.cpp
@@ -895,8 +895,7 @@ bool Parser::TrySkipAttributes() {
       // expected in the case of the Objective-C message send syntax.
       ConsumeBracket();
     } else if (Tok.isRegularKeywordAttribute() &&
-               getKeywordAttributeParseArgumentsKind(Tok.getKind()) ==
-                   KeywordAttributeParseArgumentsKind::None) {
+               !doesKeywordAttributeTakeArgs(Tok.getKind())) {
       ConsumeToken();
     } else {
       ConsumeToken();
diff --git a/clang/test/Parser/c2x-attribute-keywords.c b/clang/test/Parser/c2x-attribute-keywords.c
@@ -117,3 +117,10 @@ void f11(void) {
 struct __arm_inout("za") S4 *s; // expected-error {{'__arm_inout' cannot appear here}}
 struct S5 {};
 int c = sizeof(struct __arm_inout("za") S5); // expected-error {{'__arm_inout' cannot appear here}}
+
+void invalid_parentheses1() __arm_inout; // expected-error {{expected '(' after ''__arm_inout''}}
+void invalid_parentheses2() __arm_inout(; // expected-error {{expected string literal as argument of '__arm_inout' attribute}}
+void invalid_parentheses3() __arm_inout((); // expected-error {{expected string literal as argument of '__arm_inout' attribute}}
+void invalid_parentheses4() __arm_inout); // expected-error {{expected '(' after ''__arm_inout''}} \
+                                          // expected-error {{expected function body after function declarator}}
+void invalid_parentheses5() __arm_inout(());  // expected-error {{expected string literal as argument of '__arm_inout' attribute}}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -356,16 +356,10 @@ void valid_state_attrs_in_in2(void) __arm_in("za", "za");
 
 // expected-cpp-error@+2 {{missing state for '__arm_in'}}
 // expected-error@+1 {{missing state for '__arm_in'}}
-void invalid_state_attrs_no_arg1(void) __arm_in;
-// expected-cpp-error@+2 {{missing state for '__arm_in'}}
-// expected-error@+1 {{missing state for '__arm_in'}}
-void invalid_state_attrs_no_arg2(void) __arm_in();
-// expected-cpp-error@+2 {{missing state for '__arm_new'}}
-// expected-error@+1 {{missing state for '__arm_new'}}
-__arm_new void invalid_state_attrs_no_arg2(void);
+void invalid_state_attrs_no_arg1(void) __arm_in();
 // expected-cpp-error@+2 {{missing state for '__arm_new'}}
 // expected-error@+1 {{missing state for '__arm_new'}}
-__arm_new() void invalid_state_attrs_no_arg3(void);
+__arm_new() void invalid_state_attrs_no_arg2(void);
 
 // expected-cpp-error@+2 {{conflicting attributes for state 'za'}}
 // expected-error@+1 {{conflicting attributes for state 'za'}}
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3561,27 +3561,12 @@ void EmitClangRegularKeywordAttributeInfo(RecordKeeper &Records,
       if (!isRegularKeywordAttribute(S))
         continue;
       std::vector<Record *> Args = R->getValueAsListOfDefs("Args");
-      bool HasArgs = false;
-      bool HasOptionalArgs = false;
-      for (const auto *Arg : Args) {
-        if (Arg->getValueAsBit("Fake"))
-          continue;
-        HasArgs = true;
-        if (Arg->getValueAsBit("Optional"))
-          HasOptionalArgs = true;
-        else
-          break;
-      }
+      bool HasArgs = llvm::any_of(
+          Args, [](const Record *Arg) { return !Arg->getValueAsBit("Fake"); });
 
       OS << "KEYWORD_ATTRIBUTE("
-         << S.getSpellingRecord().getValueAsString("Name") << ", ";
-      if (HasOptionalArgs)
-        OS << "KeywordAttributeParseArgumentsKind::Optional";
-      else if (HasArgs)
-        OS << "KeywordAttributeParseArgumentsKind::Required";
-      else
-        OS << "KeywordAttributeParseArgumentsKind::None";
-      OS << ")\n";
+         << S.getSpellingRecord().getValueAsString("Name") << ", "
+         << (HasArgs ? "true" : "false") << ")\n";
     }
   OS << "#undef KEYWORD_ATTRIBUTE\n";
 }
