[PATCH] D12375: [PATCH] Relax parse ordering rules for attributes

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:3923
@@ +3922,3 @@
+
+void Parser::MaybeParseAttributes(unsigned WhichAttrKinds,
+                                  ParsedAttributesWithRange &Attrs,
----------------
Please provide an inlineable wrapper for this that checks if the current token is `[`, `__declspec`, or `__attribute__` before calling into this, as we do for the existing `MaybeParse*` functions.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:3942-3946
@@ +3941,7 @@
+      MoreToParse |= MaybeParseCXX11Attributes(Attrs, End);
+      // If C++11 attributes are not allowed because other attributes have
+      // already been parsed, we should diagnose if such an attribute was
+      // parsed.
+      if (MoreToParse && !CXX11AttrAllowed)
+        Diag(AttrStartLoc, diag::warn_attribute_ordering);
+    }
----------------
I'm not convinced this is right. It seems like there are two different cases:

1) Some context accepts a sequence of attributes before other things, such as:

  struct __attribute__((blah)) [[blah]] S;

There doesn't seem to be a principled reason to reject this.

2) Different attributes appear as different components of the grammar, such as:

  [[before_declaration]] __attribute__((im_a_decl_specifier)) int n;
  __attribute__((im_a_decl_specifier)) [[i_dont_go_here]] int n;

Only the second case should be rejected here. (And I think we should provide an error not just a warning if we don't like this case.)

I think the right thing to do is probably to always parse all attribute syntaxes (except for MS attributes, which are ambiguous with other constructs), and give a nicely-worded error if we see an unexpected syntax.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:3956-3959
@@ +3955,6 @@
+    }
+    if (WhichAttrKinds & PAKM_Microsoft) {
+      MoreToParse |= MaybeParseMicrosoftAttributes(Attrs, End);
+      CXX11AttrAllowed = false;
+    }
+  } while (MoreToParse);
----------------
This seems wrong: given `[[foo]] [[bar]]`, we'll now parse the second one as an MS attribute in a context where both are allowed.


http://reviews.llvm.org/D12375