PR17666: Instead of allowing an initial identifier argument in any attribute

which we don't think can't have one, only allow it in the tiny number of
attributes which opts into this weird parse rule.

I've manually checked that the handlers for all these attributes can in fact
cope with an identifier as the argument. This is still somewhat terrible; we
should move more fully towards picking the parsing rules based on the
attribute, and make the Parse -> Sema interface more type-safe.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193295 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Parse/CMakeLists.txt b/include/clang/Parse/CMakeLists.txt
index d20708e58c55d73f2b7d62e1fb00b5490fef711a..8a261281501bc2f687d549c657da47b6b2d5cb39 100644 (file)
--- a/include/clang/Parse/CMakeLists.txt
+++ b/include/clang/Parse/CMakeLists.txt
@@ -1,7 +1,7 @@
-clang_tablegen(AttrExprArgs.inc -gen-clang-attr-expr-args-list
+clang_tablegen(AttrIdentifierArg.inc -gen-clang-attr-identifier-arg-list
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
   SOURCE ../Basic/Attr.td
-  TARGET ClangAttrExprArgs)
+  TARGET ClangAttrIdentifierArg)
 
 clang_tablegen(AttrLateParsed.inc -gen-clang-attr-late-parsed-list
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../

diff --git a/include/clang/Parse/Makefile b/include/clang/Parse/Makefile
index fb63175ba9b39a22ade4679f1630de81b0f9205a..203d1525663d9018658151fc2a995c579641f32e 100644 (file)
--- a/include/clang/Parse/Makefile
+++ b/include/clang/Parse/Makefile
@@ -1,19 +1,19 @@
 CLANG_LEVEL := ../../..
 TD_SRC_DIR = $(PROJ_SRC_DIR)/../Basic
-BUILT_SOURCES = AttrExprArgs.inc AttrLateParsed.inc
+BUILT_SOURCES = AttrIdentifierArg.inc AttrLateParsed.inc
 
 TABLEGEN_INC_FILES_COMMON = 1
 
 include $(CLANG_LEVEL)/Makefile
 
-$(ObjDir)/AttrExprArgs.inc.tmp : $(TD_SRC_DIR)/Attr.td $(CLANG_TBLGEN) \
+$(ObjDir)/AttrIdentifierArg.inc.tmp : $(TD_SRC_DIR)/Attr.td $(CLANG_TBLGEN) \
                                    $(ObjDir)/.dir
-       $(Echo) "Building Clang attribute expression arguments table with tblgen"
-       $(Verb) $(ClangTableGen) -gen-clang-attr-expr-args-list -o $(call SYSPATH, $@) \
+       $(Echo) "Building Clang attribute identifier argument table with tblgen"
+       $(Verb) $(ClangTableGen) -gen-clang-attr-identifier-arg-list -o $(call SYSPATH, $@) \
                -I $(PROJ_SRC_DIR)/../../ $<
 
 $(ObjDir)/AttrLateParsed.inc.tmp : $(TD_SRC_DIR)/Attr.td $(CLANG_TBLGEN) \
                                    $(ObjDir)/.dir
        $(Echo) "Building Clang attribute late-parsed table with tblgen"
        $(Verb) $(ClangTableGen) -gen-clang-attr-late-parsed-list -o $(call SYSPATH, $@) \
-               -I $(PROJ_SRC_DIR)/../../ $<
\ No newline at end of file
+               -I $(PROJ_SRC_DIR)/../../ $<

diff --git a/lib/Parse/CMakeLists.txt b/lib/Parse/CMakeLists.txt
index 01c0694d0359e20325be7eb38fe5fcb98cd16331..842f191e1160c2ec7079245db072609e6e0ae119 100644 (file)
--- a/lib/Parse/CMakeLists.txt
+++ b/lib/Parse/CMakeLists.txt
@@ -17,7 +17,7 @@ add_clang_library(clangParse
 
 add_dependencies(clangParse
   ClangAttrClasses
-  ClangAttrExprArgs
+  ClangAttrIdentifierArg
   ClangAttrLateParsed
   ClangAttrList
   ClangAttrParsedAttrList

diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 0f480c133536089cc1bc5a9d05f4dee706e00215..524754bd9079e7fe5711a3d7aa3665d4c040cb0d 100644 (file)
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -172,20 +172,22 @@ void Parser::ParseGNUAttributes(ParsedAttributes &attrs,
       }
     }
     if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen))
-      SkipUntil(tok::r_paren, false);
+      SkipUntil(tok::r_paren);
     SourceLocation Loc = Tok.getLocation();
-    if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) {
-      SkipUntil(tok::r_paren, false);
-    }
+    if (ExpectAndConsume(tok::r_paren, diag::err_expected_rparen))
+      SkipUntil(tok::r_paren);
     if (endLoc)
       *endLoc = Loc;
   }
 }
 
-/// \brief Determine whether the given attribute has all expression arguments.
-static bool attributeHasExprArgs(const IdentifierInfo &II) {
-  return llvm::StringSwitch<bool>(II.getName())
-#include "clang/Parse/AttrExprArgs.inc"
+/// \brief Determine whether the given attribute has an identifier argument.
+static bool attributeHasIdentifierArg(const IdentifierInfo &II) {
+  StringRef Name = II.getName();
+  if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
+    Name = Name.drop_front(2).drop_back(2);
+  return llvm::StringSwitch<bool>(Name)
+#include "clang/Parse/AttrIdentifierArg.inc"
            .Default(false);
 }
 
@@ -210,8 +212,11 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
 
   assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('");
 
+  AttributeList::Kind AttrKind =
+      AttributeList::getKind(AttrName, ScopeName, AttributeList::AS_GNU);
+
   // Availability attributes have their own grammar.
-  if (AttrName->isStr("availability")) {
+  if (AttrKind == AttributeList::AT_Availability) {
     ParseAvailabilityAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
     return;
   }
@@ -222,12 +227,13 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
     return;
   }
   // Type safety attributes have their own grammar.
-  if (AttrName->isStr("type_tag_for_datatype")) {
+  if (AttrKind == AttributeList::AT_TypeTagForDatatype) {
     ParseTypeTagForDatatypeAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
     return;
   }
 
-  ConsumeParen(); // ignore the left paren loc for now
+  // Ignore the left paren location for now.
+  ConsumeParen();
 
   bool BuiltinType = false;
   ArgsVector ArgExprs;
@@ -260,17 +266,28 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
     break;
 
   case tok::identifier: {
-    if (AttrName->isStr("vec_type_hint")) {
+    if (AttrKind == AttributeList::AT_VecTypeHint) {
       T = ParseTypeName(&TypeRange);
       TypeParsed = true;
       break;
     }
-    // If this attribute doesn't want an 'identifier' argument, then this
-    // argument should be parsed as an expression.
-    if (attributeHasExprArgs(*AttrName))
-      break;
 
-    ArgExprs.push_back(ParseIdentifierLoc());
+    // If this attribute wants an 'identifier' argument, make it so.
+    if (attributeHasIdentifierArg(*AttrName))
+      ArgExprs.push_back(ParseIdentifierLoc());
+
+    // __attribute__((iboutletcollection)) expects an identifier then
+    // some other (ignored) things.
+    if (AttrKind == AttributeList::AT_IBOutletCollection)
+      ArgExprs.push_back(ParseIdentifierLoc());
+
+    // If we don't know how to parse this attribute, but this is the only
+    // token in this argument, assume it's meant to be an identifier.
+    if (AttrKind == AttributeList::UnknownAttribute) {
+      const Token &Next = NextToken();
+      if (Next.is(tok::r_paren) || Next.is(tok::comma))
+        ArgExprs.push_back(ParseIdentifierLoc());
+    }
   } break;
 
   default:
@@ -280,7 +297,7 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
   bool isInvalid = false;
   bool isParmType = false;
 
-  if (!BuiltinType && !AttrName->isStr("vec_type_hint") &&
+  if (!BuiltinType && AttrKind != AttributeList::AT_VecTypeHint &&
       (!ArgExprs.empty() ? Tok.is(tok::comma) : Tok.isNot(tok::r_paren))) {
     // Eat the comma.
     if (!ArgExprs.empty())
@@ -298,8 +315,8 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
         break;
       ConsumeToken(); // Eat the comma, move to the next argument
     }
-  }
-  else if (Tok.is(tok::less) && AttrName->isStr("iboutletcollection")) {
+  } else if (Tok.is(tok::less) &&
+             AttrKind == AttributeList::AT_IBOutletCollection) {
     if (!ExpectAndConsume(tok::less, diag::err_expected_less_after, "<",
                           tok::greater)) {
       while (Tok.is(tok::identifier)) {
@@ -315,7 +332,7 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
         Diag(Tok, diag::err_iboutletcollection_with_protocol);
       SkipUntil(tok::r_paren, false, true); // skip until ')'
     }
-  } else if (AttrName->isStr("vec_type_hint")) {
+  } else if (AttrKind == AttributeList::AT_VecTypeHint) {
     if (T.get() && !T.isInvalid())
       isParmType = true;
     else {

diff --git a/test/Parser/cxx-attributes.cpp b/test/Parser/cxx-attributes.cpp
index 97380521f8728e05664b0011cce80a8632b89cf1..3c99ccdb3a78f412aa19504710a50144c24b0df4 100644 (file)
--- a/test/Parser/cxx-attributes.cpp
+++ b/test/Parser/cxx-attributes.cpp
@@ -11,3 +11,9 @@ template <typename T> class X {
   template <typename S> void X<S>::f() __attribute__((locks_excluded())); // expected-error{{nested name specifier 'X<S>::' for declaration does not refer into a class, class template or class template partial specialization}} \
                                                                           // expected-warning{{attribute locks_excluded ignored, because it is not attached to a declaration}}
 };
+
+namespace PR17666 {
+  const int A = 1;
+  typedef int __attribute__((__aligned__(A))) T1;
+  int check1[__alignof__(T1) == 1 ? 1 : -1];
+}

diff --git a/test/Sema/mips16_attr_allowed.c b/test/Sema/mips16_attr_allowed.c
index 58b4a865a22d58a3dc0ea3d78dbdcc7fedeb4eea..c712e902f5fb2288d18976ed106d8a200dd0496a 100644 (file)
--- a/test/Sema/mips16_attr_allowed.c
+++ b/test/Sema/mips16_attr_allowed.c
@@ -13,8 +13,8 @@ void foo16_();
 void foo32__() __attribute__((nomips16));
 void foo32__() __attribute__((mips16));
 
-void foo32a() __attribute__((nomips16(xyz))) ; // expected-error {{'nomips16' attribute takes no arguments}}
-void __attribute__((mips16(xyz))) foo16a(); // expected-error {{'mips16' attribute takes no arguments}}
+void foo32a() __attribute__((nomips16(0))) ; // expected-error {{'nomips16' attribute takes no arguments}}
+void __attribute__((mips16(1))) foo16a(); // expected-error {{'mips16' attribute takes no arguments}}
 
 void __attribute__((nomips16(1, 2))) foo32b(); // expected-error {{'nomips16' attribute takes no arguments}}
 void __attribute__((mips16(1, 2))) foo16b(); // expected-error {{'mips16' attribute takes no arguments}}

diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp
index 6e44ac41461c1ea95af203df691dbea49cac7f6b..40b113fb7669a46cb2fcd7ab0b809cee71da8867 100644 (file)
--- a/utils/TableGen/ClangAttrEmitter.cpp
+++ b/utils/TableGen/ClangAttrEmitter.cpp
@@ -1163,10 +1163,18 @@ void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS) {
   OS << "#endif\n";
 }
 
-// Emits the all-arguments-are-expressions property for attributes.
-void EmitClangAttrExprArgsList(RecordKeeper &Records, raw_ostream &OS) {
+static bool isIdentifierArgument(Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+         llvm::StringSwitch<bool>(Arg->getSuperClasses().back()->getName())
+             .Case("IdentifierArgument", true)
+             .Case("EnumArgument", true)
+             .Default(false);
+}
+
+// Emits the first-argument-is-identifier property for attributes.
+void EmitClangAttrIdentifierArgList(RecordKeeper &Records, raw_ostream &OS) {
   emitSourceFileHeader("llvm::StringSwitch code to match attributes with "
-                       "expression arguments", OS);
+                       "an identifier argument", OS);
 
   std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
 
@@ -1174,35 +1182,19 @@ void EmitClangAttrExprArgsList(RecordKeeper &Records, raw_ostream &OS) {
        I != E; ++I) {
     Record &Attr = **I;
 
-    // Determine whether the first argument is something that is always
-    // an expression.
+    // Determine whether the first argument is an identifier.
     std::vector<Record *> Args = Attr.getValueAsListOfDefs("Args");
-    if (Args.empty() || Args[0]->getSuperClasses().empty())
-      continue;
-
-    // Check whether this is one of the argument kinds that implies an
-    // expression.
-    // FIXME: Aligned is weird.
-    if (!llvm::StringSwitch<bool>(Args[0]->getSuperClasses().back()->getName())
-          .Case("AlignedArgument", true)
-          .Case("BoolArgument", true)
-          .Case("DefaultIntArgument", true)
-          .Case("FunctionArgument", true)
-          .Case("IntArgument", true)
-          .Case("ExprArgument", true)
-          .Case("StringArgument", true)
-          .Case("UnsignedArgument", true)
-          .Case("VariadicUnsignedArgument", true)
-          .Case("VariadicExprArgument", true)
-          .Default(false))
+    if (Args.empty() || !isIdentifierArgument(Args[0]))
       continue;
 
+    // All these spellings take an identifier argument.
     std::vector<Record*> Spellings = Attr.getValueAsListOfDefs("Spellings");
-
+    std::set<std::string> Emitted;
     for (std::vector<Record*>::const_iterator I = Spellings.begin(),
          E = Spellings.end(); I != E; ++I) {
-      OS << ".Case(\"" << (*I)->getValueAsString("Name") << "\", "
-         << "true" << ")\n";
+      if (Emitted.insert((*I)->getValueAsString("Name")).second)
+        OS << ".Case(\"" << (*I)->getValueAsString("Name") << "\", "
+           << "true" << ")\n";
     }
   }
 }

diff --git a/utils/TableGen/TableGen.cpp b/utils/TableGen/TableGen.cpp
index e6c703fc66529fada9e023d3b17dc1d7e6d3666d..d573ecc09abec57526a9761cd32fabcf9f0eb1f0 100644 (file)
--- a/utils/TableGen/TableGen.cpp
+++ b/utils/TableGen/TableGen.cpp
@@ -24,7 +24,7 @@ using namespace clang;
 
 enum ActionType {
   GenClangAttrClasses,
-  GenClangAttrExprArgsList,
+  GenClangAttrIdentifierArgList,
   GenClangAttrImpl,
   GenClangAttrList,
   GenClangAttrPCHRead,
@@ -60,9 +60,10 @@ cl::opt<ActionType> Action(
     cl::values(
         clEnumValN(GenClangAttrClasses, "gen-clang-attr-classes",
                    "Generate clang attribute clases"),
-        clEnumValN(GenClangAttrExprArgsList, "gen-clang-attr-expr-args-list",
-                   "Generate a clang attribute expression "
-                   "arguments list"),
+        clEnumValN(GenClangAttrIdentifierArgList,
+                   "gen-clang-attr-identifier-arg-list",
+                   "Generate a list of attributes that take an "
+                   "identifier as their first argument"),
         clEnumValN(GenClangAttrImpl, "gen-clang-attr-impl",
                    "Generate clang attribute implementations"),
         clEnumValN(GenClangAttrList, "gen-clang-attr-list",
@@ -82,15 +83,15 @@ cl::opt<ActionType> Action(
         clEnumValN(GenClangAttrTemplateInstantiate,
                    "gen-clang-attr-template-instantiate",
                    "Generate a clang template instantiate code"),
-                    clEnumValN(GenClangAttrParsedAttrList,
-                               "gen-clang-attr-parsed-attr-list",
-                               "Generate a clang parsed attribute list"),
-                    clEnumValN(GenClangAttrParsedAttrImpl,
-                               "gen-clang-attr-parsed-attr-impl",
-                               "Generate the clang parsed attribute helpers"),
-                    clEnumValN(GenClangAttrParsedAttrKinds,
-                               "gen-clang-attr-parsed-attr-kinds",
-                               "Generate a clang parsed attribute kinds"),
+        clEnumValN(GenClangAttrParsedAttrList,
+                   "gen-clang-attr-parsed-attr-list",
+                   "Generate a clang parsed attribute list"),
+        clEnumValN(GenClangAttrParsedAttrImpl,
+                   "gen-clang-attr-parsed-attr-impl",
+                   "Generate the clang parsed attribute helpers"),
+        clEnumValN(GenClangAttrParsedAttrKinds,
+                   "gen-clang-attr-parsed-attr-kinds",
+                   "Generate a clang parsed attribute kinds"),
         clEnumValN(GenClangAttrDump, "gen-clang-attr-dump",
                    "Generate clang attribute dumper"),
         clEnumValN(GenClangDiagsDefs, "gen-clang-diags-defs",
@@ -141,8 +142,8 @@ bool ClangTableGenMain(raw_ostream &OS, RecordKeeper &Records) {
   case GenClangAttrClasses:
     EmitClangAttrClass(Records, OS);
     break;
-  case GenClangAttrExprArgsList:
-    EmitClangAttrExprArgsList(Records, OS);
+  case GenClangAttrIdentifierArgList:
+    EmitClangAttrIdentifierArgList(Records, OS);
     break;
   case GenClangAttrImpl:
     EmitClangAttrImpl(Records, OS);

diff --git a/utils/TableGen/TableGenBackends.h b/utils/TableGen/TableGenBackends.h
index d95d0afff1175761d391a4ecaeeeda1e888317e9..426b0155a78283e022eaa22676d10faa188e4ec6 100644 (file)
--- a/utils/TableGen/TableGenBackends.h
+++ b/utils/TableGen/TableGenBackends.h
@@ -30,7 +30,7 @@ void EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
                        const std::string &N, const std::string &S);
 
 void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrExprArgsList(RecordKeeper &Records, raw_ostream &OS);
+void EmitClangAttrIdentifierArgList(RecordKeeper &Records, raw_ostream &OS);
 void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS);
 void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS);
 void EmitClangAttrPCHRead(RecordKeeper &Records, raw_ostream &OS);