From: Richard Smith Date: Thu, 24 Oct 2013 01:07:54 +0000 (+0000) Subject: PR17666: Instead of allowing an initial identifier argument in any attribute X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d92aa2d2880d1c30baff996cb4c4f0836dbc355e;p=clang 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 d20708e58c..8a26128150 100644 --- 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 fb63175ba9..203d152566 100644 --- 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 01c0694d03..842f191e11 100644 --- 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 0f480c1335..524754bd90 100644 --- 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(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/test/Parser/cxx-attributes.cpp b/test/Parser/cxx-attributes.cpp index 97380521f8..3c99ccdb3a 100644 --- a/test/Parser/cxx-attributes.cpp +++ b/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/test/Sema/mips16_attr_allowed.c b/test/Sema/mips16_attr_allowed.c index 58b4a865a2..c712e902f5 100644 --- 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 6e44ac4146..40b113fb76 100644 --- 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(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/utils/TableGen/TableGen.cpp b/utils/TableGen/TableGen.cpp index e6c703fc66..d573ecc09a 100644 --- 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 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/utils/TableGen/TableGenBackends.h b/utils/TableGen/TableGenBackends.h index d95d0afff1..426b0155a7 100644 --- 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);