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



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

Reply via email to