From 66e7168f8da55c7e250a0db5b9f2789ffa76b171 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 24 Oct 2013 01:07:54 +0000 Subject: [PATCH] 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. llvm-svn: 193295 --- clang/include/clang/Parse/CMakeLists.txt | 4 +- clang/include/clang/Parse/Makefile | 10 ++-- clang/lib/Parse/CMakeLists.txt | 2 +- clang/lib/Parse/ParseDecl.cpp | 59 +++++++++++++++-------- clang/test/Parser/cxx-attributes.cpp | 6 +++ clang/test/Sema/mips16_attr_allowed.c | 4 +- clang/utils/TableGen/ClangAttrEmitter.cpp | 44 +++++++---------- clang/utils/TableGen/TableGen.cpp | 31 ++++++------ clang/utils/TableGen/TableGenBackends.h | 2 +- 9 files changed, 89 insertions(+), 73 deletions(-) diff --git a/clang/include/clang/Parse/CMakeLists.txt b/clang/include/clang/Parse/CMakeLists.txt index d20708e58c55..8a261281501b 100644 --- a/clang/include/clang/Parse/CMakeLists.txt +++ b/clang/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/clang/include/clang/Parse/Makefile b/clang/include/clang/Parse/Makefile index fb63175ba9b3..203d1525663d 100644 --- a/clang/include/clang/Parse/Makefile +++ b/clang/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/clang/lib/Parse/CMakeLists.txt b/clang/lib/Parse/CMakeLists.txt index 01c0694d0359..842f191e1160 100644 --- a/clang/lib/Parse/CMakeLists.txt +++ b/clang/lib/Parse/CMakeLists.txt @@ -17,7 +17,7 @@ add_clang_library(clangParse add_dependencies(clangParse ClangAttrClasses - ClangAttrExprArgs + ClangAttrIdentifierArg ClangAttrLateParsed ClangAttrList ClangAttrParsedAttrList diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 0f480c133536..524754bd9079 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/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(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(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/clang/test/Parser/cxx-attributes.cpp b/clang/test/Parser/cxx-attributes.cpp index 97380521f872..3c99ccdb3a78 100644 --- a/clang/test/Parser/cxx-attributes.cpp +++ b/clang/test/Parser/cxx-attributes.cpp @@ -11,3 +11,9 @@ template class X { template void X::f() __attribute__((locks_excluded())); // expected-error{{nested name specifier 'X::' 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/clang/test/Sema/mips16_attr_allowed.c b/clang/test/Sema/mips16_attr_allowed.c index 58b4a865a22d..c712e902f5fb 100644 --- a/clang/test/Sema/mips16_attr_allowed.c +++ b/clang/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/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 6e44ac41461c..40b113fb7669 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/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(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 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 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(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 Spellings = Attr.getValueAsListOfDefs("Spellings"); - + std::set Emitted; for (std::vector::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/clang/utils/TableGen/TableGen.cpp b/clang/utils/TableGen/TableGen.cpp index e6c703fc6652..d573ecc09abe 100644 --- a/clang/utils/TableGen/TableGen.cpp +++ b/clang/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 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 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/clang/utils/TableGen/TableGenBackends.h b/clang/utils/TableGen/TableGenBackends.h index d95d0afff117..426b0155a782 100644 --- a/clang/utils/TableGen/TableGenBackends.h +++ b/clang/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);