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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

I've commit in r315856, thank you for the reviews!



================
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 ]>,
----------------
rsmith wrote:
> This is not formatted how we normally format tablegen files.
I'll fix it up -- I think this was clang-formatted.


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

Yes, my proposal allows attributes on enumerators. I will fix up with your merge, thanks!


================
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 =
----------------
rsmith wrote:
> I think you can simplify this to `LO.CPlusPlus`, because `LO.DoubleSquareBracketAttributes` should always be `true` if we get here. (Right?)
Correct, I've simplified.


https://reviews.llvm.org/D37436