<div dir="ltr"><div>Thanks for doing this!</div><div><br></div>Looks like this won't reject the ill-formed construct [[deprecated()]]. (Both [[gnu::deprecated()]] and __attribute__((deprecated())) are OK, IIRC.)</div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Mon, Mar 31, 2014 at 10:32 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: aaronballman<br>
Date: Mon Mar 31 12:32:39 2014<br>
New Revision: 205226<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=205226&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=205226&view=rev</a><br>
Log:<br>
Introduced an attribute syntax-neutral method for parsing attribute arguments that is currently being used by GNU and C++-style attributes. This allows C++11 attributes with argument lists to be handled properly, fixing the "deprecated", "type_visibility", and capability-related attributes with arguments.<br>

<br>
Modified:<br>
    cfe/trunk/include/clang/Parse/Parser.h<br>
    cfe/trunk/lib/Parse/ParseDecl.cpp<br>
    cfe/trunk/lib/Parse/ParseDeclCXX.cpp<br>
    cfe/trunk/test/Parser/cxx0x-attributes.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Parse/Parser.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=205226&r1=205225&r2=205226&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=205226&r1=205225&r2=205226&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/include/clang/Parse/Parser.h (original)<br>
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Mar 31 12:32:39 2014<br>
@@ -1996,6 +1996,16 @@ private:<br>
   /// locations where attributes are not allowed.<br>
   void DiagnoseAndSkipCXX11Attributes();<br>
<br>
+  /// \brief Parses syntax-generic attribute arguments for attributes which are<br>
+  /// known to the implementation, and adds them to the given ParsedAttributes<br>
+  /// list with the given attribute syntax.<br>
+  void ParseAttributeArgsCommon(IdentifierInfo *AttrName,<br>
+                                SourceLocation AttrNameLoc,<br>
+                                ParsedAttributes &Attrs, SourceLocation *EndLoc,<br>
+                                IdentifierInfo *ScopeName,<br>
+                                SourceLocation ScopeLoc,<br>
+                                AttributeList::Syntax Syntax);<br>
+<br>
   void MaybeParseGNUAttributes(Declarator &D,<br>
                                LateParsedAttrList *LateAttrs = 0) {<br>
     if (Tok.is(tok::kw___attribute)) {<br>
@@ -2053,6 +2063,13 @@ private:<br>
                                     SourceLocation *EndLoc = 0);<br>
   void ParseCXX11Attributes(ParsedAttributesWithRange &attrs,<br>
                             SourceLocation *EndLoc = 0);<br>
+  /// \brief Parses a C++-style attribute argument list. Returns true if this<br>
+  /// results in adding an attribute to the ParsedAttributes list.<br>
+  bool ParseCXX11AttributeArgs(IdentifierInfo *AttrName,<br>
+                               SourceLocation AttrNameLoc,<br>
+                               ParsedAttributes &Attrs, SourceLocation *EndLoc,<br>
+                               IdentifierInfo *ScopeName,<br>
+                               SourceLocation ScopeLoc);<br>
<br>
   IdentifierInfo *TryParseCXX11AttributeIdentifier(SourceLocation &Loc);<br>
<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=205226&r1=205225&r2=205226&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=205226&r1=205225&r2=205226&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Mar 31 12:32:39 2014<br>
@@ -259,6 +259,65 @@ void Parser::ParseAttributeWithTypeArg(I<br>
                  0, AttrNameLoc, 0, 0, AttributeList::AS_GNU);<br>
 }<br>
<br>
+void Parser::ParseAttributeArgsCommon(<br>
+    IdentifierInfo *AttrName, SourceLocation AttrNameLoc,<br>
+    ParsedAttributes &Attrs, SourceLocation *EndLoc, IdentifierInfo *ScopeName,<br>
+    SourceLocation ScopeLoc, AttributeList::Syntax Syntax) {<br>
+  // Ignore the left paren location for now.<br>
+  ConsumeParen();<br>
+<br>
+  ArgsVector ArgExprs;<br>
+  if (Tok.is(tok::identifier)) {<br>
+    // If this attribute wants an 'identifier' argument, make it so.<br>
+    bool IsIdentifierArg = attributeHasIdentifierArg(*AttrName);<br>
+    AttributeList::Kind AttrKind =<br>
+        AttributeList::getKind(AttrName, ScopeName, Syntax);<br>
+<br>
+    // If we don't know how to parse this attribute, but this is the only<br>
+    // token in this argument, assume it's meant to be an identifier.<br>
+    if (AttrKind == AttributeList::UnknownAttribute ||<br>
+        AttrKind == AttributeList::IgnoredAttribute) {<br>
+      const Token &Next = NextToken();<br>
+      IsIdentifierArg = Next.is(tok::r_paren) || Next.is(tok::comma);<br>
+    }<br>
+<br>
+    if (IsIdentifierArg)<br>
+      ArgExprs.push_back(ParseIdentifierLoc());<br>
+  }<br>
+<br>
+  if (!ArgExprs.empty() ? Tok.is(tok::comma) : Tok.isNot(tok::r_paren)) {<br>
+    // Eat the comma.<br>
+    if (!ArgExprs.empty())<br>
+      ConsumeToken();<br>
+<br>
+    // Parse the non-empty comma-separated list of expressions.<br>
+    do {<br>
+      std::unique_ptr<EnterExpressionEvaluationContext> Unevaluated;<br>
+      if (attributeParsedArgsUnevaluated(*AttrName))<br>
+        Unevaluated.reset(<br>
+            new EnterExpressionEvaluationContext(Actions, Sema::Unevaluated));<br>
+<br>
+      ExprResult ArgExpr(ParseAssignmentExpression());<br>
+      if (ArgExpr.isInvalid()) {<br>
+        SkipUntil(tok::r_paren, StopAtSemi);<br>
+        return;<br>
+      }<br>
+      ArgExprs.push_back(ArgExpr.release());<br>
+      // Eat the comma, move to the next argument<br>
+    } while (TryConsumeToken(tok::comma));<br>
+  }<br>
+<br>
+  SourceLocation RParen = Tok.getLocation();<br>
+  if (!ExpectAndConsume(tok::r_paren)) {<br>
+    SourceLocation AttrLoc = ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc;<br>
+    Attrs.addNew(AttrName, SourceRange(AttrLoc, RParen), ScopeName, ScopeLoc,<br>
+                 ArgExprs.data(), ArgExprs.size(), Syntax);<br>
+  }<br>
+<br>
+  if (EndLoc)<br>
+    *EndLoc = RParen;<br>
+}<br>
+<br>
 /// Parse the arguments to a parameterized GNU attribute or<br>
 /// a C++11 attribute in "gnu" namespace.<br>
 void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,<br>
@@ -314,58 +373,8 @@ void Parser::ParseGNUAttributeArgs(Ident<br>
     }<br>
   }<br>
<br>
-  // Ignore the left paren location for now.<br>
-  ConsumeParen();<br>
-<br>
-  ArgsVector ArgExprs;<br>
-<br>
-  if (Tok.is(tok::identifier)) {<br>
-    // If this attribute wants an 'identifier' argument, make it so.<br>
-    bool IsIdentifierArg = attributeHasIdentifierArg(*AttrName);<br>
-<br>
-    // If we don't know how to parse this attribute, but this is the only<br>
-    // token in this argument, assume it's meant to be an identifier.<br>
-    if (AttrKind == AttributeList::UnknownAttribute ||<br>
-        AttrKind == AttributeList::IgnoredAttribute) {<br>
-      const Token &Next = NextToken();<br>
-      IsIdentifierArg = Next.is(tok::r_paren) || Next.is(tok::comma);<br>
-    }<br>
-<br>
-    if (IsIdentifierArg)<br>
-      ArgExprs.push_back(ParseIdentifierLoc());<br>
-  }<br>
-<br>
-  if (!ArgExprs.empty() ? Tok.is(tok::comma) : Tok.isNot(tok::r_paren)) {<br>
-    // Eat the comma.<br>
-    if (!ArgExprs.empty())<br>
-      ConsumeToken();<br>
-<br>
-    // Parse the non-empty comma-separated list of expressions.<br>
-    do {<br>
-      std::unique_ptr<EnterExpressionEvaluationContext> Unevaluated;<br>
-      if (attributeParsedArgsUnevaluated(*AttrName))<br>
-        Unevaluated.reset(new EnterExpressionEvaluationContext(Actions,<br>
-        Sema::Unevaluated));<br>
-<br>
-      ExprResult ArgExpr(ParseAssignmentExpression());<br>
-      if (ArgExpr.isInvalid()) {<br>
-        SkipUntil(tok::r_paren, StopAtSemi);<br>
-        return;<br>
-      }<br>
-      ArgExprs.push_back(ArgExpr.release());<br>
-      // Eat the comma, move to the next argument<br>
-    } while (TryConsumeToken(tok::comma));<br>
-  }<br>
-<br>
-  SourceLocation RParen = Tok.getLocation();<br>
-  if (!ExpectAndConsume(tok::r_paren)) {<br>
-    SourceLocation AttrLoc = ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc;<br>
-    Attrs.addNew(AttrName, SourceRange(AttrLoc, RParen), ScopeName, ScopeLoc,<br>
-                 ArgExprs.data(), ArgExprs.size(), Syntax);<br>
-  }<br>
-<br>
-  if (EndLoc)<br>
-    *EndLoc = RParen;<br>
+  ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,<br>
+                           ScopeLoc, Syntax);<br>
 }<br>
<br>
 /// \brief Parses a single argument for a declspec, including the<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=205226&r1=205225&r2=205226&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=205226&r1=205225&r2=205226&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Mar 31 12:32:39 2014<br>
@@ -15,7 +15,9 @@<br>
 #include "RAIIObjectsForParser.h"<br>
 #include "clang/AST/ASTContext.h"<br>
 #include "clang/AST/DeclTemplate.h"<br>
+#include "clang/Basic/Attributes.h"<br>
 #include "clang/Basic/CharInfo.h"<br>
+#include "clang/Basic/TargetInfo.h"<br>
 #include "clang/Basic/OperatorKinds.h"<br>
 #include "clang/Parse/ParseDiagnostic.h"<br>
 #include "clang/Sema/DeclSpec.h"<br>
@@ -3197,8 +3199,50 @@ static bool IsBuiltInOrStandardCXX11Attr<br>
   }<br>
 }<br>
<br>
-/// ParseCXX11AttributeSpecifier - Parse a C++11 attribute-specifier. Currently<br>
-/// only parses standard attributes.<br>
+/// ParseCXX11AttributeArgs -- Parse a C++11 attribute-argument-clause.<br>
+///<br>
+/// [C++11] attribute-argument-clause:<br>
+///         '(' balanced-token-seq ')'<br>
+///<br>
+/// [C++11] balanced-token-seq:<br>
+///         balanced-token<br>
+///         balanced-token-seq balanced-token<br>
+///<br>
+/// [C++11] balanced-token:<br>
+///         '(' balanced-token-seq ')'<br>
+///         '[' balanced-token-seq ']'<br>
+///         '{' balanced-token-seq '}'<br>
+///         any token but '(', ')', '[', ']', '{', or '}'<br>
+bool Parser::ParseCXX11AttributeArgs(IdentifierInfo *AttrName,<br>
+                                     SourceLocation AttrNameLoc,<br>
+                                     ParsedAttributes &Attrs,<br>
+                                     SourceLocation *EndLoc,<br>
+                                     IdentifierInfo *ScopeName,<br>
+                                     SourceLocation ScopeLoc) {<br>
+  assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");<br>
+<br>
+  // If the attribute isn't known, we will not attempt to parse any<br>
+  // arguments.<br>
+  if (!hasAttribute(AttrSyntax::CXX, ScopeName, AttrName,<br>
+                    getTargetInfo().getTriple(), getLangOpts())) {<br>
+    // Eat the left paren, then skip to the ending right paren.<br>
+    ConsumeParen();<br>
+    SkipUntil(tok::r_paren);<br>
+    return false;<br>
+  }<br>
+<br>
+  if (ScopeName && ScopeName->getName() == "gnu")<br>
+    // GNU-scoped attributes have some special cases to handle GNU-specific<br>
+    // behaviors.<br>
+    ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,<br>
+                          ScopeLoc, AttributeList::AS_CXX11, 0);<br>
+  else<br>
+    ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,<br>
+                             ScopeLoc, AttributeList::AS_CXX11);<br>
+  return true;<br>
+}<br>
+<br>
+/// ParseCXX11AttributeSpecifier - Parse a C++11 attribute-specifier.<br>
 ///<br>
 /// [C++11] attribute-specifier:<br>
 ///         '[' '[' attribute-list ']' ']'<br>
@@ -3222,19 +3266,6 @@ static bool IsBuiltInOrStandardCXX11Attr<br>
 ///<br>
 /// [C++11] attribute-namespace:<br>
 ///         identifier<br>
-///<br>
-/// [C++11] attribute-argument-clause:<br>
-///         '(' balanced-token-seq ')'<br>
-///<br>
-/// [C++11] balanced-token-seq:<br>
-///         balanced-token<br>
-///         balanced-token-seq balanced-token<br>
-///<br>
-/// [C++11] balanced-token:<br>
-///         '(' balanced-token-seq ')'<br>
-///         '[' balanced-token-seq ']'<br>
-///         '{' balanced-token-seq '}'<br>
-///         any token but '(', ')', '[', ']', '{', or '}'<br>
 void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs,<br>
                                           SourceLocation *endLoc) {<br>
   if (Tok.is(tok::kw_alignas)) {<br>
@@ -3279,29 +3310,22 @@ void Parser::ParseCXX11AttributeSpecifie<br>
       }<br>
     }<br>
<br>
-    bool StandardAttr = IsBuiltInOrStandardCXX11Attribute(AttrName,ScopeName);<br>
+    bool StandardAttr = IsBuiltInOrStandardCXX11Attribute(AttrName, ScopeName);<br>
     bool AttrParsed = false;<br>
<br>
     if (StandardAttr &&<br>
         !SeenAttrs.insert(std::make_pair(AttrName, AttrLoc)).second)<br>
       Diag(AttrLoc, diag::err_cxx11_attribute_repeated)<br>
-        << AttrName << SourceRange(SeenAttrs[AttrName]);<br>
+          << AttrName << SourceRange(SeenAttrs[AttrName]);<br>
<br>
     // Parse attribute arguments<br>
     if (Tok.is(tok::l_paren)) {<br>
-      if (ScopeName && ScopeName->getName() == "gnu") {<br>
-        ParseGNUAttributeArgs(AttrName, AttrLoc, attrs, endLoc,<br>
-                              ScopeName, ScopeLoc, AttributeList::AS_CXX11, 0);<br>
-        AttrParsed = true;<br>
-      } else {<br>
-        if (StandardAttr)<br>
-          Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments)<br>
+      if (StandardAttr)<br>
+        Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments)<br>
             << AttrName->getName();<br>
<br>
-        // FIXME: handle other formats of c++11 attribute arguments<br>
-        ConsumeParen();<br>
-        SkipUntil(tok::r_paren);<br>
-      }<br>
+      AttrParsed = ParseCXX11AttributeArgs(AttrName, AttrLoc, attrs, endLoc,<br>
+                                           ScopeName, ScopeLoc);<br>
     }<br>
<br>
     if (!AttrParsed)<br>
<br>
Modified: cfe/trunk/test/Parser/cxx0x-attributes.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-attributes.cpp?rev=205226&r1=205225&r2=205226&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-attributes.cpp?rev=205226&r1=205225&r2=205226&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/Parser/cxx0x-attributes.cpp (original)<br>
+++ cfe/trunk/test/Parser/cxx0x-attributes.cpp Mon Mar 31 12:32:39 2014<br>
@@ -48,13 +48,13 @@ int array_attr [1] [[]];<br>
 alignas(8) int aligned_attr;<br>
 [[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}}<br>
 [[,,,static, class, namespace,, inline, constexpr, mutable,, bitand, bitor::compl(!.*_ Cx.!U^*R),,,]] int more_garbage_attr; // expected-warning {{unknown attribute 'static' ignored}} \<br>
-       // expected-warning {{unknown attribute 'class' ignored}} \<br>
-       // expected-warning {{unknown attribute 'namespace' ignored}} \<br>
-       // expected-warning {{unknown attribute 'inline' ignored}} \<br>
-       // expected-warning {{unknown attribute 'constexpr' ignored}} \<br>
-       // expected-warning {{unknown attribute 'mutable' ignored}} \<br>
-       // expected-warning {{unknown attribute 'bitand' ignored}} \<br>
-        // expected-warning {{unknown attribute 'compl' ignored}}<br>
+    // expected-warning {{unknown attribute 'class' ignored}} \<br>
+    // expected-warning {{unknown attribute 'namespace' ignored}} \<br>
+    // expected-warning {{unknown attribute 'inline' ignored}} \<br>
+    // expected-warning {{unknown attribute 'constexpr' ignored}} \<br>
+    // expected-warning {{unknown attribute 'mutable' ignored}} \<br>
+    // expected-warning {{unknown attribute 'bitand' ignored}} \<br>
+    // expected-warning {{unknown attribute 'compl' ignored}}<br>
 [[u8"invalid!"]] int invalid_string_attr; // expected-error {{expected ']'}}<br>
 void fn_attr () [[]];<br>
 void noexcept_fn_attr () noexcept [[]];<br>
@@ -281,7 +281,8 @@ enum class [[]] EvenMoreSecrets {};<br>
<br>
 namespace arguments {<br>
   void f[[gnu::format(printf, 1, 2)]](const char*, ...);<br>
-  void g() [[unknown::foo(arguments of attributes from unknown namespace other than 'gnu' namespace are ignored... blah...)]]; // expected-warning {{unknown attribute 'foo' ignored}}<br>
+  void g() [[unknown::foo(ignore arguments for unknown attributes, even with symbols!)]]; // expected-warning {{unknown attribute 'foo' ignored}}<br>
+  [[deprecated("with argument")]] int i;<br>
 }<br>
<br>
 // Forbid attributes on decl specifiers.<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>