[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953
+  "have bases of non-trivial class types|have virtual bases|"
+  "have __weak fields under ARC|have fields of non-trivial class types}0">;
 
----------------
nit: "of non-trivial class types" should be "of non-trivial class type" in both places.

And I would write "are not move-constructible" rather than "don't have non-deleted copy/move constructors". Double negations aren't non-bad.

Actually I would rephrase this as `'trivial_abi' is disallowed on this class because it %select{is not move-constructible|is polymorphic|has a base of non-trivial class type|has a virtual base|has a __weak field|has a field of non-trivial class type}`, i.e., we're not just giving information about "classes" in general, we're talking about "this class" specifically. We could even name the class if we're feeling generous.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:7886
+    return false;
+  };
+
----------------
How confident are we that this logic is correct?  I ask because I need something similar for my own diagnostic in D50119. If this logic is rock-solid (no lurking corner-case bugs), I should copy it — and/or it should be moved into a helper member function on `CXXRecordDecl` so that other people can call it too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626