rsmith added a comment.

If this is just supposed to be an experiment to get feedback on the feature, 
then I don't think we should be treating it as a different attribute syntax at 
all. Rather, I think we just want to permit C++11 attributes to be parsed in 
other language modes. If/when this becomes part of some future C working draft, 
I think that's the time to have a separate attribute syntax with a distinct set 
of valid unqualified attribute names.



================
Comment at: include/clang/Basic/LangOptions.def:140
 
+LANGOPT(CAttributes       , 1, 0, "'[[]]' attributes extension to C")
+
----------------
Can we give this a more general name, then enable it under `CPlusPlus` too? 
That's what we do for other similar features.


================
Comment at: include/clang/Driver/Options.td:607
 
+def fc_attributes : Flag<["-"], "fc-attributes">, Group<f_Group>,
+  Flags<[DriverOption, CC1Option]>, HelpText<"Enable '[[]]' attributes in C">;
----------------
Hmm. On reflection, if we're going to have a flag to enable C++11 attributes in 
other language modes, it should probably work in C++98 mode too, so calling 
this `-fc-attributes` is probably not the best idea. `-fc++11-attributes` might 
make sense, though.


================
Comment at: lib/Parse/ParseDecl.cpp:4219
+
+    // Attributes are prohibited in this location in C2x (and forward
+    // declarations are prohibited in C++).
----------------
I don't think we can reasonably say what C2x will do. Also, doesn't this reject 
valid code such as:
```
struct __attribute__((deprecated)) Foo;
```
?


================
Comment at: lib/Parse/ParseDeclCXX.cpp:3939-3940
                                           SourceLocation *endLoc) {
-  if (Tok.is(tok::kw_alignas)) {
+  bool CXXAttr = getLangOpts().CPlusPlus11;
+  if (CXXAttr && Tok.is(tok::kw_alignas)) {
     Diag(Tok.getLocation(), diag::warn_cxx98_compat_alignas);
----------------
This change is unnecessary. `kw_alignas` is not produced by the lexer in modes 
where we should not parse it.


================
Comment at: lib/Parse/ParseDeclCXX.cpp:3947
+  assert(Tok.is(tok::l_square) && NextToken().is(tok::l_square) &&
+         "Not a standards-based attribute list");
 
----------------
`[[`... in C is not a standards-based attribute list. I'd revert this change 
too.


================
Comment at: lib/Parse/ParseDeclCXX.cpp:3956
   IdentifierInfo *CommonScopeName = nullptr;
-  if (Tok.is(tok::kw_using)) {
+  if (CXXAttr && Tok.is(tok::kw_using)) {
     Diag(Tok.getLocation(), getLangOpts().CPlusPlus1z
----------------
Likewise.


https://reviews.llvm.org/D37436



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to