From 6880f492365cc4fa4c941aa83688635003ee7498 Mon Sep 17 00:00:00 2001 From: Michael Han Date: Wed, 3 Oct 2012 01:56:22 +0000 Subject: [PATCH] Improve C++11 attribute parsing. - General C++11 attributes were previously parsed and ignored. Now they are parsed and stored in AST. - Add support to parse arguments of attributes that in 'gnu' namespace. - Differentiate unknown attributes and known attributes that can't be applied to statements when emitting diagnostic. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@165082 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Parse/Parser.h | 6 +- lib/Parse/ParseDecl.cpp | 23 ++++-- lib/Parse/ParseDeclCXX.cpp | 65 +++++++++------- lib/Sema/SemaStmtAttr.cpp | 9 ++- .../dcl.dcl/dcl.attr/dcl.attr.grammar/p6.cpp | 3 +- test/Parser/cxx0x-attributes.cpp | 19 ++++- test/Parser/cxx11-stmt-attributes.cpp | 77 ++++++++++++------- test/Parser/objcxx11-attributes.mm | 18 +++-- ...switch-implicit-fallthrough-per-method.cpp | 2 +- 9 files changed, 145 insertions(+), 77 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index d143a1a0c5..25fe4cf4aa 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1848,7 +1848,10 @@ private: void ParseGNUAttributeArgs(IdentifierInfo *AttrName, SourceLocation AttrNameLoc, ParsedAttributes &Attrs, - SourceLocation *EndLoc); + SourceLocation *EndLoc, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, + AttributeList::Syntax Syntax); void MaybeParseCXX0XAttributes(Declarator &D) { if (getLangOpts().CPlusPlus0x && isCXX11AttributeSpecifier()) { @@ -1878,6 +1881,7 @@ private: SourceLocation *EndLoc = 0); void ParseCXX11Attributes(ParsedAttributesWithRange &attrs, SourceLocation *EndLoc = 0); + IdentifierInfo *TryParseCXX11AttributeIdentifier(SourceLocation &Loc); void MaybeParseMicrosoftAttributes(ParsedAttributes &attrs, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 674cd2d22f..e90d0eb9fe 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -154,7 +154,8 @@ void Parser::ParseGNUAttributes(ParsedAttributes &attrs, Eof.setLocation(Tok.getLocation()); LA->Toks.push_back(Eof); } else { - ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc); + ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc, + 0, AttrNameLoc, AttributeList::AS_GNU); } } else { attrs.addNew(AttrName, AttrNameLoc, 0, AttrNameLoc, @@ -173,11 +174,15 @@ void Parser::ParseGNUAttributes(ParsedAttributes &attrs, } -/// Parse the arguments to a parameterized GNU attribute +/// Parse the arguments to a parameterized GNU attribute or +/// a C++11 attribute in "gnu" namespace. void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName, SourceLocation AttrNameLoc, ParsedAttributes &Attrs, - SourceLocation *EndLoc) { + SourceLocation *EndLoc, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, + AttributeList::Syntax Syntax) { assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('"); @@ -278,9 +283,9 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName, SourceLocation RParen = Tok.getLocation(); if (!ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) { AttributeList *attr = - Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), 0, AttrNameLoc, - ParmName, ParmLoc, ArgExprs.data(), ArgExprs.size(), - AttributeList::AS_GNU); + Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), + ScopeName, ScopeLoc, ParmName, ParmLoc, + ArgExprs.data(), ArgExprs.size(), Syntax); if (BuiltinType && attr->getKind() == AttributeList::AT_IBOutletCollection) Diag(Tok, diag::err_iboutletcollection_builtintype); } @@ -923,7 +928,8 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA, if (HasFunScope) Actions.ActOnReenterFunctionContext(Actions.CurScope, D); - ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, &endLoc); + ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, &endLoc, + 0, LA.AttrNameLoc, AttributeList::AS_GNU); if (HasFunScope) { Actions.ActOnExitFunctionContext(); @@ -935,7 +941,8 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA, } else { // If there are multiple decls, then the decl cannot be within the // function scope. - ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, &endLoc); + ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, &endLoc, + 0, LA.AttrNameLoc, AttributeList::AS_GNU); } } else { Diag(Tok, diag::warn_attribute_no_decl) << LA.AttrName.getName(); diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp index 6fee37dc02..7e1e8ce10e 100644 --- a/lib/Parse/ParseDeclCXX.cpp +++ b/lib/Parse/ParseDeclCXX.cpp @@ -2879,6 +2879,21 @@ IdentifierInfo *Parser::TryParseCXX11AttributeIdentifier(SourceLocation &Loc) { } } +static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName, + IdentifierInfo *ScopeName) { + switch (AttributeList::getKind(AttrName, ScopeName, + AttributeList::AS_CXX11)) { + case AttributeList::AT_CarriesDependency: + case AttributeList::AT_FallThrough: + case AttributeList::AT_NoReturn: { + return true; + } + + default: + return false; + } +} + /// ParseCXX11AttributeSpecifier - Parse a C++11 attribute-specifier. Currently /// only parses standard attributes. /// @@ -2963,46 +2978,38 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, } } + bool StandardAttr = IsBuiltInOrStandardCXX11Attribute(AttrName,ScopeName); bool AttrParsed = false; - switch (AttributeList::getKind(AttrName, ScopeName, - AttributeList::AS_CXX11)) { - // No arguments - case AttributeList::AT_CarriesDependency: - // FIXME: implement generic support of attributes with C++11 syntax - // see Parse/ParseDecl.cpp: ParseGNUAttributes - case AttributeList::AT_FallThrough: - case AttributeList::AT_NoReturn: { - if (Tok.is(tok::l_paren)) { - Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments) - << AttrName->getName(); - break; + + // Parse attribute arguments + if (Tok.is(tok::l_paren)) { + if (ScopeName && ScopeName->getName() == "gnu") { + ParseGNUAttributeArgs(AttrName, AttrLoc, attrs, endLoc, + ScopeName, ScopeLoc, AttributeList::AS_CXX11); + AttrParsed = true; + } else { + if (StandardAttr) + Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments) + << AttrName->getName(); + + // FIXME: handle other formats of c++11 attribute arguments + ConsumeParen(); + SkipUntil(tok::r_paren, false); } + } + if (!AttrParsed) attrs.addNew(AttrName, SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrLoc, AttrLoc), ScopeName, ScopeLoc, 0, SourceLocation(), 0, 0, AttributeList::AS_CXX11); - AttrParsed = true; - break; - } - - // Silence warnings - default: break; - } - - // Skip the entire parameter clause, if any - if (!AttrParsed && Tok.is(tok::l_paren)) { - ConsumeParen(); - // SkipUntil maintains the balancedness of tokens. - SkipUntil(tok::r_paren, false); - } if (Tok.is(tok::ellipsis)) { - if (AttrParsed) - Diag(Tok, diag::err_cxx11_attribute_forbids_ellipsis) - << AttrName->getName(); ConsumeToken(); + + Diag(Tok, diag::err_cxx11_attribute_forbids_ellipsis) + << AttrName->getName(); } } diff --git a/lib/Sema/SemaStmtAttr.cpp b/lib/Sema/SemaStmtAttr.cpp index 3c15b7a8af..b268b4502c 100644 --- a/lib/Sema/SemaStmtAttr.cpp +++ b/lib/Sema/SemaStmtAttr.cpp @@ -48,11 +48,16 @@ static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const AttributeList &A, static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList &A, SourceRange Range) { switch (A.getKind()) { + case AttributeList::UnknownAttribute: + S.Diag(A.getLoc(), A.isDeclspecAttribute() ? + diag::warn_unhandled_ms_attribute_ignored : + diag::warn_unknown_attribute_ignored) << A.getName(); + return 0; case AttributeList::AT_FallThrough: return handleFallThroughAttr(S, St, A, Range); default: - // if we're here, then we parsed an attribute, but didn't recognize it as a - // statement attribute => it is declaration attribute + // if we're here, then we parsed a known attribute, but didn't recognize + // it as a statement attribute => it is declaration attribute S.Diag(A.getRange().getBegin(), diag::warn_attribute_invalid_on_stmt) << A.getName()->getName() << St->getLocStart(); return 0; diff --git a/test/CXX/dcl.dcl/dcl.attr/dcl.attr.grammar/p6.cpp b/test/CXX/dcl.dcl/dcl.attr/dcl.attr.grammar/p6.cpp index f9702ba7cc..416aeb49a7 100644 --- a/test/CXX/dcl.dcl/dcl.attr/dcl.attr.grammar/p6.cpp +++ b/test/CXX/dcl.dcl/dcl.attr/dcl.attr.grammar/p6.cpp @@ -6,7 +6,8 @@ int p[10]; void f() { int x = 42, y[5]; // FIXME: Produce a better diagnostic for this case. - int(p[[x] { return x; }()]); // expected-error {{expected ']'}} + int(p[[x] { return x; }()]); // expected-error {{expected ']'}} \ + // expected-warning {{unknown attribute 'x' ignored}} y[[] { return 2; }()] = 2; // expected-error {{consecutive left square brackets}} } diff --git a/test/Parser/cxx0x-attributes.cpp b/test/Parser/cxx0x-attributes.cpp index a0b8467bcc..5790e10044 100644 --- a/test/Parser/cxx0x-attributes.cpp +++ b/test/Parser/cxx0x-attributes.cpp @@ -44,10 +44,15 @@ int & [[]] ref_attr = after_attr; int && [[]] rref_attr = 0; int array_attr [1] [[]]; alignas(8) int aligned_attr; -[[test::valid(for 42 [very] **** '+' symbols went on a trip and had a "good"_time; the end.)]] - int garbage_attr; -[[,,,static, class, namespace,, inline, constexpr, mutable,, bi\ -tand, bitor::compl(!.*_ Cx.!U^*R),,,]] int more_garbage_attr; +[[test::valid(for 42 [very] **** '+' symbols went on a trip and had a "good"_time; the end.)]] int garbage_attr; // expected-warning {{unknown attribute 'valid' ignored}} +[[,,,static, class, namespace,, inline, constexpr, mutable,, bitand, bitor::compl(!.*_ Cx.!U^*R),,,]] int more_garbage_attr; // expected-warning {{unknown attribute 'static' ignored}} \ + // expected-warning {{unknown attribute 'class' ignored}} \ + // expected-warning {{unknown attribute 'namespace' ignored}} \ + // expected-warning {{unknown attribute 'inline' ignored}} \ + // expected-warning {{unknown attribute 'constexpr' ignored}} \ + // expected-warning {{unknown attribute 'mutable' ignored}} \ + // expected-warning {{unknown attribute 'bitand' ignored}} \ + // expected-warning {{unknown attribute 'compl' ignored}} [[u8"invalid!"]] int invalid_string_attr; // expected-error {{expected ']'}} void fn_attr () [[]]; void noexcept_fn_attr () noexcept [[]]; @@ -212,3 +217,9 @@ enum class __attribute__((visibility("hidden"))) SecretKeepers { one, /* rest are deprecated */ two, three }; enum class [[]] EvenMoreSecrets {}; + +namespace arguments { + // FIXME: remove the sema warnings after migrating existing gnu attributes to c++11 syntax. + void f(const char*, ...) [[gnu::format(printf, 1, 2)]]; // expected-warning {{unknown attribute 'format' ignored}} + void g() [[unknown::foo(currently arguments of attributes from unknown namespace other than 'gnu' namespace are ignored... blah...)]]; // expected-warning {{unknown attribute 'foo' ignored}} +} diff --git a/test/Parser/cxx11-stmt-attributes.cpp b/test/Parser/cxx11-stmt-attributes.cpp index fab56218eb..f26db7989f 100644 --- a/test/Parser/cxx11-stmt-attributes.cpp +++ b/test/Parser/cxx11-stmt-attributes.cpp @@ -2,53 +2,78 @@ void foo(int i) { - [[unknown_attribute]] ; - [[unknown_attribute]] { } - [[unknown_attribute]] if (0) { } - [[unknown_attribute]] for (;;); - [[unknown_attribute]] do { - [[unknown_attribute]] continue; + [[unknown_attribute]] ; // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] { } // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] if (0) { } // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] for (;;); // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] do { // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] continue; // expected-warning {{unknown attribute 'unknown_attribute' ignored}} } while (0); - [[unknown_attribute]] while (0); + [[unknown_attribute]] while (0); // expected-warning {{unknown attribute 'unknown_attribute' ignored}} - [[unknown_attribute]] switch (i) { - [[unknown_attribute]] case 0: - [[unknown_attribute]] default: - [[unknown_attribute]] break; + [[unknown_attribute]] switch (i) { // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] case 0: // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] default: // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] break; // expected-warning {{unknown attribute 'unknown_attribute' ignored}} } - [[unknown_attribute]] goto here; - [[unknown_attribute]] here: + [[unknown_attribute]] goto here; // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + [[unknown_attribute]] here: // expected-warning {{unknown attribute 'unknown_attribute' ignored}} - [[unknown_attribute]] try { + [[unknown_attribute]] try { // expected-warning {{unknown attribute 'unknown_attribute' ignored}} } catch (...) { } - [[unknown_attribute]] return; - + [[unknown_attribute]] return; // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + alignas(8) ; // expected-warning {{attribute aligned cannot be specified on a statement}} [[noreturn]] { } // expected-warning {{attribute noreturn cannot be specified on a statement}} [[noreturn]] if (0) { } // expected-warning {{attribute noreturn cannot be specified on a statement}} [[noreturn]] for (;;); // expected-warning {{attribute noreturn cannot be specified on a statement}} [[noreturn]] do { // expected-warning {{attribute noreturn cannot be specified on a statement}} - [[unavailable]] continue; // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here + [[unavailable]] continue; // expected-warning {{unknown attribute 'unavailable' ignored}} + } while (0); + [[unknown_attributqqq]] while (0); // expected-warning {{unknown attribute 'unknown_attributqqq' ignored}} + // TODO: remove 'qqq' part and enjoy 'empty loop body' warning here (DiagnoseEmptyLoopBody) + + [[unknown_attribute]] while (0); // expected-warning {{unknown attribute 'unknown_attribute' ignored}} + + [[unused]] switch (i) { // expected-warning {{unknown attribute 'unused' ignored}} + [[uuid]] case 0: // expected-warning {{unknown attribute 'uuid' ignored}} + [[visibility]] default: // expected-warning {{unknown attribute 'visibility' ignored}} + [[carries_dependency]] break; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + } + + [[fastcall]] goto there; // expected-warning {{unknown attribute 'fastcall' ignored}} + [[noinline]] there: // expected-warning {{unknown attribute 'noinline' ignored}} + + [[lock_returned]] try { // expected-warning {{unknown attribute 'lock_returned' ignored}} + } catch (...) { + } + + [[weakref]] return; // expected-warning {{unknown attribute 'weakref' ignored}} + + [[carries_dependency]] ; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] { } // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] if (0) { } // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] for (;;); // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] do { // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] continue; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} ignored}} } while (0); - [[unknown_attributqqq]] while (0); // TODO: remove 'qqq' part and enjoy 'empty loop body' warning here (DiagnoseEmptyLoopBody) - [[unknown_attribute]] while (0); // no warning here yet, just an unknown attribute + [[carries_dependency]] while (0); // expected-warning {{attribute carries_dependency cannot be specified on a statement}} - [[unused]] switch (i) { // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here - [[uuid]] case 0: // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here - [[visibility]] default: // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here + [[carries_dependency]] switch (i) { // expected-warning {{attribute carries_dependency cannot be specified on a statement}} ignored}} + [[carries_dependency]] case 0: // expected-warning {{attribute carries_dependency cannot be specified on a statement}} + [[carries_dependency]] default: // expected-warning {{attribute carries_dependency cannot be specified on a statement}} [[carries_dependency]] break; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} } - [[fastcall]] goto there; // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here - [[noinline]] there: // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here + [[carries_dependency]] goto here; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} - [[lock_returned]] try { // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here + [[carries_dependency]] try { // expected-warning {{attribute carries_dependency cannot be specified on a statement}} } catch (...) { } - [[weakref]] return; // TODO: only noreturn, alignas and carries_dependency are parsed in C++ 11 syntax at the moment, hence no warning here + [[carries_dependency]] return; // expected-warning {{attribute carries_dependency cannot be specified on a statement}} } diff --git a/test/Parser/objcxx11-attributes.mm b/test/Parser/objcxx11-attributes.mm index 1b9bf66fad..ad54208286 100644 --- a/test/Parser/objcxx11-attributes.mm +++ b/test/Parser/objcxx11-attributes.mm @@ -31,9 +31,13 @@ void f(X *noreturn) { // An attribute is OK. [[]]; - [[int(), noreturn]]; // expected-warning {{attribute noreturn cannot be specified on a statement}} - [[class, test(foo 'x' bar),,,]]; - [[bitand, noreturn]]; // expected-warning {{attribute noreturn cannot be specified on a statement}} + [[int(), noreturn]]; // expected-warning {{unknown attribute 'int' ignored}} \ + // expected-warning {{attribute noreturn cannot be specified on a statement}} + [[class, test(foo 'x' bar),,,]]; // expected-warning {{unknown attribute 'test' ignored}}\ + // expected-warning {{unknown attribute 'class' ignored}} + + [[bitand, noreturn]]; // expected-warning {{attribute noreturn cannot be specified on a statement}} \ + expected-warning {{unknown attribute 'bitand' ignored}} // FIXME: Suppress vexing parse warning [[noreturn]]int(e)(); // expected-warning {{function declaration}} expected-note {{replace parentheses with an initializer}} @@ -52,7 +56,11 @@ void f(X *noreturn) { } template void f(Ts ...x) { - [[test::foo(bar, baz)...]]; - [[used(x)...]]; + [[test::foo(bar, baz)...]]; // expected-error {{attribute 'foo' cannot be used as an attribute pack}} \ + // expected-warning {{unknown attribute 'foo' ignored}} + + [[used(x)...]]; // expected-error {{attribute 'used' cannot be used as an attribute pack}} \ + // expected-warning {{unknown attribute 'used' ignored}} + [[x...] { return [ X alloc ]; }() init]; } diff --git a/test/SemaCXX/switch-implicit-fallthrough-per-method.cpp b/test/SemaCXX/switch-implicit-fallthrough-per-method.cpp index 7c52e5138b..009c8180b1 100644 --- a/test/SemaCXX/switch-implicit-fallthrough-per-method.cpp +++ b/test/SemaCXX/switch-implicit-fallthrough-per-method.cpp @@ -42,7 +42,7 @@ void unscoped(int n) { switch (n % 2) { case 0: // FIXME: This should be typo-corrected, probably. - [[fallthrough]]; + [[fallthrough]]; // expected-warning{{unknown attribute 'fallthrough' ignored}} case 2: // expected-warning{{unannotated fall-through}} expected-note{{clang::fallthrough}} expected-note{{break;}} [[clang::fallthrough]]; case 1: -- 2.40.0