[cfe-commits] [Patch] Improve diagnostics on gnu attributes

[Michael Han]

Hi Ted,

Thanks for the comments. I have attached the revised patch which adds a hasParameterOrArguments to AttributeList and use that in Sema checking.

Cheers
~Michael

From: Ted Kremenek [mailto:kremenek at apple.com]
Sent: Thursday, April 14, 2011 12:03 PM
To: Michael Han
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [cfe-commits] [Patch] Improve diagnostics on gnu attributes

Hi Michael,

>From looking at ParseDecl.cpp, ParseGNUAttributes, I think the difference between the parameter and the arguments is that the parameter is an identifier and the arguments are expressions.  Essentially, the parameter is the first argument that is guaranteed to be an identifier.

Instead of doing:

+  if (Attr.getParameterName() || Attr.getNumArgs() != 0) {

how about something like:

  if (Attr.hasParameterOrArguments())

which would just an inline method in Attribute that expands to the logic you wrote.  Right now you're basically copy-pasting a "complex" predicate in multiple places, and that is the perfect use of an inline method that summarizes your intent and makes the client code simpler.

Ted

On Apr 10, 2011, at 9:17 PM, Michael Han wrote:


Consider this code:

int a;
__attribute__((noreturn(a))) void foo();

Clang does not emit any warning or errors on this code; gcc would report wrong number of arguments for the attribute specified.
The attached patch fixed this by emit a "takes no argument" error for clang.

Alternatively parser can be updated so the first identifier immediately following the attribute name could be treated as argument, instead of "parameter" (what is the difference between them?). This would require a lot more work to update the parser and the AttributeList so I am not sure if that is the right thing to do for now. Please review the patch thanks

Cheers
~Michael

<attributes.fix.patch>_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto: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/20110414/3d6f8baa/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attr.arg.fix.revised.patch
Type: application/octet-stream
Size: 5480 bytes
Desc: attr.arg.fix.revised.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110414/3d6f8baa/attachment.obj>