r206186 - Properly diagnose standard C++ attributes which have optional argument lists when the arguments are elided. eg)
Richard Smith
richard at metafoo.co.uk
Mon Apr 14 17:20:28 PDT 2014
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140414/6197ac3f/attachment.html>
More information about the cfe-commits
mailing list