[PATCH] D17893: Sema: Add semantic analysis for the C++ ABI stability attributes and whitelist.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 8 10:31:28 PST 2016


aaron.ballman added a comment.

Missing some attribute-related tests like attaching the attribute to something other than a record, or passing arguments to the attribute.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8332
@@ +8331,3 @@
+  "base %0 uses the stable ABI">;
+def note_unstable_abi_base : Note<
+  "base %0 uses the unstable ABI">;
----------------
It would be good to combine these into one Note using %select.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8336
@@ +8335,3 @@
+  "stable ABI specified by attribute">;
+def note_unstable_abi_attr : Note<
+  "unstable ABI specified by attribute">;
----------------
Same for these.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8345
@@ +8344,3 @@
+def note_add_unstable_abi_attr : Note<
+  "add attribute clang::unstable_abi or clang::stable_abi to silence this warning">;
+
----------------
Should this also be a %select, or can you really add either attribute to silence the warning?

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4896
@@ +4895,3 @@
+
+  // No need to do this for non-dynamic classes.
+  if (!Record->isDynamicClass())
----------------
Since the attributes do nothing for a non-dynamic class, should the user get a warning if the place the attribute on one (so they know it does nothing)?


http://reviews.llvm.org/D17893





More information about the cfe-commits mailing list