[cfe-dev] c++ 11 attribute support in clang

[Michael Han]

Hi Richard,

Thank you for the feedback! I'll rework the patch based on your feedback. Minor comments inline.

Michael

From: metafoo at gmail.com [mailto:metafoo at gmail.com] On Behalf Of Richard Smith
Sent: Monday, October 01, 2012 6:16 PM
To: Michael Han
Cc: cfe-dev at cs.uiuc.edu
Subject: Re: [cfe-dev] c++ 11 attribute support in clang

On Mon, Oct 1, 2012 at 3:56 PM, Michael Han <Michael.Han at autodesk.com<mailto:Michael.Han at autodesk.com>> wrote:
Hi,

Attached patch is trying to improve the C++ 11 generalized attributes support in Clang:

- Parse generic C++ 11 attributes into AST instead of parsing and ignoring them.
- Parse attributes arguments if the attributes are in gnu scope.
- Update several tests since with the unknown attributes introduced to AST some expected diagnostics shall be emitted.

This would allow one to extend Clang attributes using C++ 11 attribute syntax and migrate existing GNU style attributes with new syntax.

We had been deliberately holding back on that, waiting for the GCC guys to commit to a particular approach. It looks like they're just putting all their existing attributes into a gnu:: namespace, though, so this is fine:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53528
http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01348.html

[hanm] Good to know!

Are there any ideas and plans on adding generalized C++ 11 attribute support to Clang? I feel migrating existing GNU attributes to new syntax might be a good start. Any feedback are appreciated.

C++11 attributes certainly need more love, and such work would be very much appreciated. In terms of the standard C++11 pieces, we have a few deficiencies: We don't accept attributes (and alignas) in quite the right set of places, nor do we apply them to the right set of entities, we get the semantics of [[noreturn]] wrong (we use the GNU __attribute__((noreturn)) semantics, which don't quite match), and we don't issue an error for an attribute which is applied to an entity to which it can't appertain.

[hanm] I'll take a look at these after this patch. Thanks for the pointers!

On to your patch:

+++ lib/Parse/ParseDeclCXX.cpp     (working copy)
@@ -2963,46 +2963,35 @@
+      if (ScopeName && ScopeName->getName() == "gnu") {
+        ParseCXX11AttributeGNUStyleArgs(AttrName, AttrLoc, ScopeName,
+                                        ScopeLoc, attrs);

Please add some tests for this.
[hanm]  will do.

+        // FIXME: handle other formats of c++11 attribute arguments
+        ConsumeParen();
+        SkipUntil(tok::r_paren, false);

It looks like this silently drops unknown attributes followed by parens. Can we add them to the list in this case, so we'll get a warning?
[hanm] sounds good.

[...]
+      if (StandardAttr)
         Diag(Tok, diag::err_cxx11_attribute_forbids_ellipsis)
           << AttrName->getName();

Please reject ellipses for GNU attributes too, for now (with an 'unsupported' error), rather than silently dropping them.
[hanm] ok.

@@ -3014,6 +3003,75 @@
     SkipUntil(tok::r_square, false);
 }

+bool Parser::IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
+                                               IdentifierInfo *ScopeName) {

Make this a static, non-member function.
[hanm] ok.

+void Parser::ParseCXX11AttributeGNUStyleArgs(IdentifierInfo *AttrName,
+                                             SourceLocation AttrNameLoc,
+                                             IdentifierInfo *ScopeName,
+                                             SourceLocation ScopeLoc,
+                                             ParsedAttributes &Attrs) {

Can you share this code between C++11 and GNU attribute parsing?
[hanm] ok.

+++ test/Parser/cxx11-stmt-attributes.cpp    (working copy)
@@ -2,53 +2,55 @@

 void foo(int i) {
+  [[unknown_attribute]] ; // expected-warning {{attribute unknown_attribute cannot be specified on a statement}}

Yuck. Would you be interested in teaching this warning the difference between 'attribute cannot be specified on a statement' and 'unknown attribute'?
[hanm] I'll improve the diagnostic.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121002/73b09495/attachment.html>