r206186 - Properly diagnose standard C++ attributes which have optional argument lists when the arguments are elided. eg)
Aaron Ballman
aaron at aaronballman.com
Mon Apr 14 17:43:51 PDT 2014
All good comments -- I've applied everything but the fixit in r206229
(I put in a fixme for the fixit).
Thanks!
~Aaron
On Mon, Apr 14, 2014 at 8:20 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Mon, Apr 14, 2014 at 9:03 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Author: aaronballman
>> Date: Mon Apr 14 11:03:22 2014
>> New Revision: 206186
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206186&view=rev
>> Log:
>> Properly diagnose standard C++ attributes which have optional argument
>> lists when the arguments are elided. eg)
>>
>> [[deprecated()]] // error
>> [[deprecated]] // OK
>> [[deprecated("")]] // OK
>> [[gnu::deprecated()]] // OK
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> cfe/trunk/include/clang/Parse/Parser.h
>> cfe/trunk/lib/Parse/ParseDecl.cpp
>> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> cfe/trunk/test/Parser/attributes.c
>> cfe/trunk/test/Parser/cxx0x-attributes.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Apr 14
>> 11:03:22 2014
>> @@ -526,7 +526,9 @@ def warn_cxx98_compat_attribute : Warnin
>> "C++11 attribute syntax is incompatible with C++98">,
>> InGroup<CXX98Compat>, DefaultIgnore;
>> def err_cxx11_attribute_forbids_arguments : Error<
>> - "attribute '%0' cannot have an argument list">;
>> + "attribute %0 cannot have an argument list">;
>> +def err_attribute_requires_arguements : Error<
>
>
> Typo 'arguements'
>
>>
>> + "attribute %0 requires a nonempty argument list">;
>
>
> 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.
>
>>
>> def err_cxx11_attribute_forbids_ellipsis : Error<
>> "attribute '%0' cannot be used as an attribute pack">;
>> def err_cxx11_attribute_repeated : Error<
>>
>> Modified: cfe/trunk/include/clang/Parse/Parser.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Parse/Parser.h (original)
>> +++ cfe/trunk/include/clang/Parse/Parser.h Mon Apr 14 11:03:22 2014
>> @@ -2012,13 +2012,13 @@ private:
>>
>> /// \brief Parses syntax-generic attribute arguments for attributes
>> which are
>> /// known to the implementation, and adds them to the given
>> ParsedAttributes
>> - /// list with the given attribute syntax.
>> - void ParseAttributeArgsCommon(IdentifierInfo *AttrName,
>> - SourceLocation AttrNameLoc,
>> - ParsedAttributes &Attrs, SourceLocation
>> *EndLoc,
>> - IdentifierInfo *ScopeName,
>> - SourceLocation ScopeLoc,
>> - AttributeList::Syntax Syntax);
>> + /// list with the given attribute syntax. Returns the number of
>> arguments
>> + /// parsed for the attribute.
>> + unsigned
>> + ParseAttributeArgsCommon(IdentifierInfo *AttrName, SourceLocation
>> AttrNameLoc,
>> + ParsedAttributes &Attrs, SourceLocation
>> *EndLoc,
>> + IdentifierInfo *ScopeName, SourceLocation
>> ScopeLoc,
>> + AttributeList::Syntax Syntax);
>>
>> void MaybeParseGNUAttributes(Declarator &D,
>> LateParsedAttrList *LateAttrs = 0) {
>>
>> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Apr 14 11:03:22 2014
>> @@ -261,7 +261,7 @@ void Parser::ParseAttributeWithTypeArg(I
>> 0, AttrNameLoc, 0, 0, AttributeList::AS_GNU);
>> }
>>
>> -void Parser::ParseAttributeArgsCommon(
>> +unsigned Parser::ParseAttributeArgsCommon(
>> IdentifierInfo *AttrName, SourceLocation AttrNameLoc,
>> ParsedAttributes &Attrs, SourceLocation *EndLoc, IdentifierInfo
>> *ScopeName,
>> SourceLocation ScopeLoc, AttributeList::Syntax Syntax) {
>> @@ -302,7 +302,7 @@ void Parser::ParseAttributeArgsCommon(
>> ExprResult ArgExpr(ParseAssignmentExpression());
>> if (ArgExpr.isInvalid()) {
>> SkipUntil(tok::r_paren, StopAtSemi);
>> - return;
>> + return 0;
>> }
>> ArgExprs.push_back(ArgExpr.release());
>> // Eat the comma, move to the next argument
>> @@ -318,6 +318,8 @@ void Parser::ParseAttributeArgsCommon(
>>
>> if (EndLoc)
>> *EndLoc = RParen;
>> +
>> + return static_cast<unsigned>(ArgExprs.size());
>> }
>>
>> /// Parse the arguments to a parameterized GNU attribute or
>>
>> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Apr 14 11:03:22 2014
>> @@ -3194,6 +3194,7 @@ static bool IsBuiltInOrStandardCXX11Attr
>> switch (AttributeList::getKind(AttrName, ScopeName,
>> AttributeList::AS_CXX11)) {
>> case AttributeList::AT_CarriesDependency:
>> + case AttributeList::AT_Deprecated:
>> case AttributeList::AT_FallThrough:
>> case AttributeList::AT_CXX11NoReturn: {
>> return true;
>> @@ -3225,6 +3226,7 @@ bool Parser::ParseCXX11AttributeArgs(Ide
>> IdentifierInfo *ScopeName,
>> SourceLocation ScopeLoc) {
>> assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");
>> + SourceLocation LParenLoc = Tok.getLocation();
>>
>> // If the attribute isn't known, we will not attempt to parse any
>> // arguments.
>> @@ -3241,9 +3243,32 @@ bool Parser::ParseCXX11AttributeArgs(Ide
>> // behaviors.
>> ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc,
>> ScopeName,
>> ScopeLoc, AttributeList::AS_CXX11, 0);
>> - else
>> - ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,
>> ScopeName,
>> - ScopeLoc, AttributeList::AS_CXX11);
>> + else {
>> + unsigned NumArgs =
>> + ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,
>> + ScopeName, ScopeLoc,
>> AttributeList::AS_CXX11);
>> +
>> + const AttributeList *Attr = Attrs.getList();
>> + if (Attr && IsBuiltInOrStandardCXX11Attribute(AttrName, ScopeName)) {
>> + // If the attribute is a standard or built-in attribute and we are
>> + // parsing an argument list, we need to determine whether this
>> attribute
>> + // was allowed to have an argument list (such as [[deprecated]]),
>> and how
>> + // many arguments were parsed (so we can diagnose on
>> [[deprecated()]]).
>> + if (Attr->getMaxArgs() && !NumArgs) {
>> + // The attribute was allowed to have arguments, but none were
>> provided
>> + // even though the attribute parsed successfully. This is an
>> error.
>> + Diag(LParenLoc, diag::err_attribute_requires_arguements) <<
>> AttrName;
>> + return false;
>> + } else if (!Attr->getMaxArgs()) {
>> + // The attribute parsed successfully, but was not allowed to have
>> any
>> + // arguments. It doesn't matter whether any were provided -- the
>> + // presence of the argument list (even if empty) is diagnosed.
>> + Diag(LParenLoc, diag::err_cxx11_attribute_forbids_arguments)
>> + << AttrName;
>> + return false;
>> + }
>
>
> 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.
>
>> + }
>> + }
>> return true;
>> }
>>
>> @@ -3324,14 +3349,9 @@ void Parser::ParseCXX11AttributeSpecifie
>> << AttrName << SourceRange(SeenAttrs[AttrName]);
>>
>> // Parse attribute arguments
>> - if (Tok.is(tok::l_paren)) {
>> - if (StandardAttr)
>> - Diag(Tok.getLocation(),
>> diag::err_cxx11_attribute_forbids_arguments)
>> - << AttrName->getName();
>> -
>> + if (Tok.is(tok::l_paren))
>> AttrParsed = ParseCXX11AttributeArgs(AttrName, AttrLoc, attrs,
>> endLoc,
>> ScopeName, ScopeLoc);
>> - }
>>
>> if (!AttrParsed)
>> attrs.addNew(AttrName,
>>
>> Modified: cfe/trunk/test/Parser/attributes.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Parser/attributes.c (original)
>> +++ cfe/trunk/test/Parser/attributes.c Mon Apr 14 11:03:22 2014
>> @@ -94,5 +94,4 @@ void testFundef5() __attribute__(()) { }
>>
>> __attribute__((pure)) int testFundef6(int a) { return a; }
>>
>> -
>> -
>> +void deprecatedTestFun(void) __attribute__((deprecated()));
>>
>> Modified: cfe/trunk/test/Parser/cxx0x-attributes.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-attributes.cpp?rev=206186&r1=206185&r2=206186&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Parser/cxx0x-attributes.cpp (original)
>> +++ cfe/trunk/test/Parser/cxx0x-attributes.cpp Mon Apr 14 11:03:22 2014
>> @@ -322,3 +322,10 @@ namespace GccASan {
>> [[gnu::no_address_safety_analysis]] void f3();
>> [[gnu::no_sanitize_address]] void f4();
>> }
>> +
>> +namespace {
>> + [[deprecated]] void bar();
>> + [[deprecated("hello")]] void baz();
>> + [[deprecated()]] void foo(); // expected-error {{attribute 'deprecated'
>> requires a nonempty argument list}}
>> + [[gnu::deprecated()]] void quux();
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list