rsmith added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:875-876
   "C++ standards before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
+def err_anon_bitfield_member_init : Error<
+  "anonymous bit-field cannot have an in-class initializer">;
 def err_incomplete_array_member_init: Error<
----------------
Please retain the diagnostic wording using proper standard terminology; the 
other diagnostics say "in-class initializer" because they predate the existence 
of the standard terminology and haven't been fixed yet. (Fixing them -- and 
renaming the corresponding functions throughout Clang -- would be great if you 
feel so inclined.)


================
Comment at: clang/lib/Parse/ParseDecl.cpp:4127-4129
+      // Anonymous bit-fields cannot specify attributes; the attributes
+      // appertain to the type specifier for the bit-field instead. Provide a
+      // kinder parsing error than if we just let parsing happen organically.
----------------
I think this will regress our diagnostics for this (probably more common) case:

```
struct X { 
  int a, [[attr]] b;
};
```

Instead, how about we unconditionally `DiagnoseAndSkipCXX11Attributes()` before 
and after we parse GNU attributes in the `if (!FirstDeclarator)` check up 
above? (Aside: we should probably be better about handling mixed sequences of 
GNU and C++11 attributes in general.)


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2311-2313
+  //   identifier attribute-specifier-seq[opt] ':' constant-expression
+  //       brace-or-equal-initializer[opt]
+  //   ':' constant-expression
----------------
Please mention that this is a proposed bugfix, not the standard grammar.


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2318-2320
+    // Anonymous bit-fields cannot specify attributes; the attributes appertain
+    // to the type specifier for the bit-field instead. Provide a kinder
+    // parsing error than if we just let parsing happen organically.
----------------
(Same comment as for the C side of things.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88333/new/

https://reviews.llvm.org/D88333

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D88333: B... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] D883... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D883... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] D883... Aaron Ballman via Phabricator via cfe-commits
    • [PATCH] D883... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D883... Aaron Ballman via Phabricator via cfe-commits

Reply via email to