[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM :)



================
Comment at: ../llvm/tools/clang/include/clang/Driver/Options.td:609-616
+def fdouble_square_bracket_attributes
+    : Flag<[ "-" ], "fdouble-square-bracket-attributes">,
+      Group<f_Group>, Flags<[ DriverOption, CC1Option ]>,
+      HelpText<"Enable '[[]]' attributes in all C and C++ language modes">;
+def fno_double_square_bracket_attributes
+    : Flag<[ "-" ], "fno-fdouble-square-bracket-attributes">,
+      Group<f_Group>, Flags<[ DriverOption ]>,
----------------
This is not formatted how we normally format tablegen files.


================
Comment at: ../llvm/tools/clang/lib/Parse/ParseDecl.cpp:4422
+    if (standardAttributesAllowed() && isCXX11AttributeSpecifier()) {
       if (!getLangOpts().CPlusPlus1z)
         Diag(Tok.getLocation(), diag::warn_cxx14_compat_attribute)
----------------
Is this warning appropriate in C? I don't recall whether your proposal permits attributes on enumerators or not.

... in fact, this warning is completely wrong. Fixed in r315784. This should presumably be guarded by an `if (getLangOpts().CPlusPlus)` with your change, though.


================
Comment at: ../llvm/tools/clang/lib/Parse/ParseDeclCXX.cpp:3856-3857
+  const LangOptions &LO = getLangOpts();
+  bool AsCXX =
+      LO.CPlusPlus11 || (LO.DoubleSquareBracketAttributes && LO.CPlusPlus);
+  AttributeList::Syntax Syntax =
----------------
I think you can simplify this to `LO.CPlusPlus`, because `LO.DoubleSquareBracketAttributes` should always be `true` if we get here. (Right?)


================
Comment at: clang/lib/Parse/ParseTentative.cpp:592
                                   bool OuterMightBeMessageSend) {
-  if (Tok.is(tok::kw_alignas))
+  if (Tok.is(tok::kw_alignas) && getLangOpts().CPlusPlus11)
     return CAK_AttributeSpecifier;
----------------
aaron.ballman wrote:
> rsmith wrote:
> > This change is redundant. We wouldn't lex a `kw_alignas` token outside C++11.
> Ah, I thought that we would hit this case for `_Alignas` as well, but I see that's a different keyword. Just to double-check -- we don't expect `-fdouble-square-bracket-attributes` to enable `alignas` in C++98, correct?
Right, I would not expect it to affect the set of available keywords.


https://reviews.llvm.org/D37436