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

Richard Smith richard-llvm at metafoo.co.uk
Wed Oct 23 18:07:54 PDT 2013


Author: rsmith
Date: Wed Oct 23 20:07:54 2013
New Revision: 193295

URL: http://llvm.org/viewvc/llvm-project?rev=193295&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Parse/CMakeLists.txt
    cfe/trunk/include/clang/Parse/Makefile
    cfe/trunk/lib/Parse/CMakeLists.txt
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/test/Parser/cxx-attributes.cpp
    cfe/trunk/test/Sema/mips16_attr_allowed.c
    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
    cfe/trunk/utils/TableGen/TableGen.cpp
    cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/include/clang/Parse/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/CMakeLists.txt?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/Parse/CMakeLists.txt Wed Oct 23 20:07:54 2013
@@ -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}/../../

Modified: cfe/trunk/include/clang/Parse/Makefile
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Makefile?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Makefile (original)
+++ cfe/trunk/include/clang/Parse/Makefile Wed Oct 23 20:07:54 2013
@@ -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)/../../ $<

Modified: cfe/trunk/lib/Parse/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/CMakeLists.txt?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/CMakeLists.txt (original)
+++ cfe/trunk/lib/Parse/CMakeLists.txt Wed Oct 23 20:07:54 2013
@@ -17,7 +17,7 @@ add_clang_library(clangParse
 
 add_dependencies(clangParse
   ClangAttrClasses
-  ClangAttrExprArgs
+  ClangAttrIdentifierArg
   ClangAttrLateParsed
   ClangAttrList
   ClangAttrParsedAttrList

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Oct 23 20:07:54 2013
@@ -172,20 +172,22 @@ void Parser::ParseGNUAttributes(ParsedAt
       }
     }
     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(Ident
 
   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(Ident
     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(Ident
     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(Ident
   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(Ident
         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(Ident
         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 {

Modified: cfe/trunk/test/Parser/cxx-attributes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-attributes.cpp?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx-attributes.cpp (original)
+++ cfe/trunk/test/Parser/cxx-attributes.cpp Wed Oct 23 20:07:54 2013
@@ -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];
+}

Modified: cfe/trunk/test/Sema/mips16_attr_allowed.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/mips16_attr_allowed.c?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/test/Sema/mips16_attr_allowed.c (original)
+++ cfe/trunk/test/Sema/mips16_attr_allowed.c Wed Oct 23 20:07:54 2013
@@ -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}}

Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Wed Oct 23 20:07:54 2013
@@ -1163,10 +1163,18 @@ void EmitClangAttrClass(RecordKeeper &Re
   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(RecordKee
        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";
     }
   }
 }

Modified: cfe/trunk/utils/TableGen/TableGen.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGen.cpp?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/TableGen.cpp (original)
+++ cfe/trunk/utils/TableGen/TableGen.cpp Wed Oct 23 20:07:54 2013
@@ -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,
   case GenClangAttrClasses:
     EmitClangAttrClass(Records, OS);
     break;
-  case GenClangAttrExprArgsList:
-    EmitClangAttrExprArgsList(Records, OS);
+  case GenClangAttrIdentifierArgList:
+    EmitClangAttrIdentifierArgList(Records, OS);
     break;
   case GenClangAttrImpl:
     EmitClangAttrImpl(Records, OS);

Modified: cfe/trunk/utils/TableGen/TableGenBackends.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGenBackends.h?rev=193295&r1=193294&r2=193295&view=diff
==============================================================================
--- cfe/trunk/utils/TableGen/TableGenBackends.h (original)
+++ cfe/trunk/utils/TableGen/TableGenBackends.h Wed Oct 23 20:07:54 2013
@@ -30,7 +30,7 @@ void EmitClangASTNodes(RecordKeeper &RK,
                        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);





More information about the cfe-commits mailing list