<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 14, 2014 at 9:03 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 Apr 14 11:03:22 2014<br>
New Revision: 206186<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206186&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206186&view=rev</a><br>
Log:<br>
Properly diagnose standard C++ attributes which have optional argument lists when the arguments are elided. eg)<br>
<br>
[[deprecated()]] // error<br>
[[deprecated]] // OK<br>
[[deprecated("")]] // OK<br>
[[gnu::deprecated()]] // OK<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<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/attributes.c<br>
cfe/trunk/test/Parser/cxx0x-attributes.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Apr 14 11:03:22 2014<br>
@@ -526,7 +526,9 @@ def warn_cxx98_compat_attribute : Warnin<br>
"C++11 attribute syntax is incompatible with C++98">,<br>
InGroup<CXX98Compat>, DefaultIgnore;<br>
def err_cxx11_attribute_forbids_arguments : Error<<br>
- "attribute '%0' cannot have an argument list">;<br>
+ "attribute %0 cannot have an argument list">;<br>
+def err_attribute_requires_arguements : Error<<br></blockquote><div><br></div><div>Typo 'arguements'</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ "attribute %0 requires a nonempty argument list">;<br></blockquote><div><br></div><div>It'd be nicer to say something more like "parentheses must be omitted if %0 attribute's argument list is empty". The user probably didn't want to give an argument here, and we shouldn't suggest that one is necessary.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
def err_cxx11_attribute_forbids_ellipsis : Error<<br>
"attribute '%0' cannot be used as an attribute pack">;<br>
def err_cxx11_attribute_repeated : Error<<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=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Parse/Parser.h (original)<br>
+++ cfe/trunk/include/clang/Parse/Parser.h Mon Apr 14 11:03:22 2014<br>
@@ -2012,13 +2012,13 @@ private:<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>
+ /// list with the given attribute syntax. Returns the number of arguments<br>
+ /// parsed for the attribute.<br>
+ unsigned<br>
+ ParseAttributeArgsCommon(IdentifierInfo *AttrName, SourceLocation AttrNameLoc,<br>
+ ParsedAttributes &Attrs, SourceLocation *EndLoc,<br>
+ IdentifierInfo *ScopeName, SourceLocation ScopeLoc,<br>
+ AttributeList::Syntax Syntax);<br>
<br>
void MaybeParseGNUAttributes(Declarator &D,<br>
LateParsedAttrList *LateAttrs = 0) {<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=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Apr 14 11:03:22 2014<br>
@@ -261,7 +261,7 @@ void Parser::ParseAttributeWithTypeArg(I<br>
0, AttrNameLoc, 0, 0, AttributeList::AS_GNU);<br>
}<br>
<br>
-void Parser::ParseAttributeArgsCommon(<br>
+unsigned Parser::ParseAttributeArgsCommon(<br>
IdentifierInfo *AttrName, SourceLocation AttrNameLoc,<br>
ParsedAttributes &Attrs, SourceLocation *EndLoc, IdentifierInfo *ScopeName,<br>
SourceLocation ScopeLoc, AttributeList::Syntax Syntax) {<br>
@@ -302,7 +302,7 @@ void Parser::ParseAttributeArgsCommon(<br>
ExprResult ArgExpr(ParseAssignmentExpression());<br>
if (ArgExpr.isInvalid()) {<br>
SkipUntil(tok::r_paren, StopAtSemi);<br>
- return;<br>
+ return 0;<br>
}<br>
ArgExprs.push_back(ArgExpr.release());<br>
// Eat the comma, move to the next argument<br>
@@ -318,6 +318,8 @@ void Parser::ParseAttributeArgsCommon(<br>
<br>
if (EndLoc)<br>
*EndLoc = RParen;<br>
+<br>
+ return static_cast<unsigned>(ArgExprs.size());<br>
}<br>
<br>
/// Parse the arguments to a parameterized GNU attribute or<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=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Apr 14 11:03:22 2014<br>
@@ -3194,6 +3194,7 @@ static bool IsBuiltInOrStandardCXX11Attr<br>
switch (AttributeList::getKind(AttrName, ScopeName,<br>
AttributeList::AS_CXX11)) {<br>
case AttributeList::AT_CarriesDependency:<br>
+ case AttributeList::AT_Deprecated:<br>
case AttributeList::AT_FallThrough:<br>
case AttributeList::AT_CXX11NoReturn: {<br>
return true;<br>
@@ -3225,6 +3226,7 @@ bool Parser::ParseCXX11AttributeArgs(Ide<br>
IdentifierInfo *ScopeName,<br>
SourceLocation ScopeLoc) {<br>
assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");<br>
+ SourceLocation LParenLoc = Tok.getLocation();<br>
<br>
// If the attribute isn't known, we will not attempt to parse any<br>
// arguments.<br>
@@ -3241,9 +3243,32 @@ bool Parser::ParseCXX11AttributeArgs(Ide<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>
+ else {<br>
+ unsigned NumArgs =<br>
+ ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,<br>
+ ScopeName, ScopeLoc, AttributeList::AS_CXX11);<br>
+<br>
+ const AttributeList *Attr = Attrs.getList();<br>
+ if (Attr && IsBuiltInOrStandardCXX11Attribute(AttrName, ScopeName)) {<br>
+ // If the attribute is a standard or built-in attribute and we are<br>
+ // parsing an argument list, we need to determine whether this attribute<br>
+ // was allowed to have an argument list (such as [[deprecated]]), and how<br>
+ // many arguments were parsed (so we can diagnose on [[deprecated()]]).<br>
+ if (Attr->getMaxArgs() && !NumArgs) {<br>
+ // The attribute was allowed to have arguments, but none were provided<br>
+ // even though the attribute parsed successfully. This is an error.<br>
+ Diag(LParenLoc, diag::err_attribute_requires_arguements) << AttrName;<br>
+ return false;<br>
+ } else if (!Attr->getMaxArgs()) {<br>
+ // The attribute parsed successfully, but was not allowed to have any<br>
+ // arguments. It doesn't matter whether any were provided -- the<br>
+ // presence of the argument list (even if empty) is diagnosed.<br>
+ Diag(LParenLoc, diag::err_cxx11_attribute_forbids_arguments)<br>
+ << AttrName;<br>
+ return false;<br>
+ }<br></blockquote><div><br></div><div>I think it'd be better to use the "remove the ()" diagnostic whenever NumArgs is 0, and use the "forbids arguments" diagnostic whenever NumArgs is nonzero and getMaxArgs() is 0. The "remove the ()" diagnostic should probably also get a fixit removing the parens.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br>
+ }<br>
return true;<br>
}<br>
<br>
@@ -3324,14 +3349,9 @@ void Parser::ParseCXX11AttributeSpecifie<br>
<< AttrName << SourceRange(SeenAttrs[AttrName]);<br>
<br>
// Parse attribute arguments<br>
- if (Tok.is(tok::l_paren)) {<br>
- if (StandardAttr)<br>
- Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments)<br>
- << AttrName->getName();<br>
-<br>
+ if (Tok.is(tok::l_paren))<br>
AttrParsed = ParseCXX11AttributeArgs(AttrName, AttrLoc, attrs, endLoc,<br>
ScopeName, ScopeLoc);<br>
- }<br>
<br>
if (!AttrParsed)<br>
attrs.addNew(AttrName,<br>
<br>
Modified: cfe/trunk/test/Parser/attributes.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Parser/attributes.c (original)<br>
+++ cfe/trunk/test/Parser/attributes.c Mon Apr 14 11:03:22 2014<br>
@@ -94,5 +94,4 @@ void testFundef5() __attribute__(()) { }<br>
<br>
__attribute__((pure)) int testFundef6(int a) { return a; }<br>
<br>
-<br>
-<br>
+void deprecatedTestFun(void) __attribute__((deprecated()));<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=206186&r1=206185&r2=206186&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-attributes.cpp?rev=206186&r1=206185&r2=206186&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Parser/cxx0x-attributes.cpp (original)<br>
+++ cfe/trunk/test/Parser/cxx0x-attributes.cpp Mon Apr 14 11:03:22 2014<br>
@@ -322,3 +322,10 @@ namespace GccASan {<br>
[[gnu::no_address_safety_analysis]] void f3();<br>
[[gnu::no_sanitize_address]] void f4();<br>
}<br>
+<br>
+namespace {<br>
+ [[deprecated]] void bar();<br>
+ [[deprecated("hello")]] void baz();<br>
+ [[deprecated()]] void foo(); // expected-error {{attribute 'deprecated' requires a nonempty argument list}}<br>
+ [[gnu::deprecated()]] void quux();<br>
+}<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></div>