[cfe-commits] [Patch] C++11 attribute parsing

Richard Smith richard at metafoo.co.uk
Tue Oct 2 17:39:50 PDT 2012


On Tue, Oct 2, 2012 at 4:23 PM, Michael Han <Michael.Han at autodesk.com>wrote:

> Hi,
>
> Following the feedback from Richard [1], here is the updated patch that's
> trying to improve clang support of C++11 attribute. Thanks!
>

The patch looks good, just a few minor nits then feel free to commit.

> +++ lib/Parse/ParseDeclCXX.cpp (working copy)
> @@ -2963,46 +2978,38 @@
> [...]
> +    // parse attribute arguments

Please capitalize comments, per
http://llvm.org/docs/CodingStandards.html#commenting

> +++ lib/Parse/ParseDecl.cpp (working copy)
> @@ -154,7 +154,8 @@
>            Eof.setLocation(Tok.getLocation());
>            LA->Toks.push_back(Eof);
>          } else {
> -          ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc);
> +          ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc,
> +                                0, AttrNameLoc, AttributeList::AS_GNU);

This should be SourceLocation(), rather than AttrNameLoc, since there isn't
a scope token (I see that you were preserving the existing behavior, but we
may as well fix this). Likewise in the other two calls without a scope
token. If something is relying on the source location being valid here,
we'll need to fix it to not do so (feel free to defer this tweak in that
case).

> @@ -173,11 +174,15 @@
>  }
>
>
> -/// Parse the arguments to a parameterized GNU attribute
> +/// Parse the arguments to a parameterized GNU attribute or
> +/// a C++ 11 attribute in "gnu" namespace.

C++11 is usually written without the space.

> +++ test/Parser/cxx0x-attributes.cpp (working copy)
> @@ -4,7 +4,7 @@
>  namespace std {
>    typedef decltype(sizeof(int)) size_t;
>
> -  // libc++'s implementation
> +  // libc++'s implementation

An extra tab has appeared at the end of this line.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121002/bb3a5085/attachment.html>


More information about the cfe-commits mailing list