[PATCH] D51855: [constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Thank you!



================
Comment at: include/clang/Basic/DiagnosticASTKinds.td:172
   "non-trivially-copyable type %1">;
+def note_constexpr_memcpy_incompletetype : Note<
+  "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
----------------
Nit: add an underscore between `incomplete` and `type`.


================
Comment at: lib/AST/ExprConstant.cpp:6225-6228
+    if (T->isIncompleteType()) {
+      Info.FFDiag(E, diag::note_constexpr_memcpy_incompletetype) << Move << T;
+      return false;
+    }
----------------
Please reorder this before the trivial-copyability check. We don't know whether a type is trivially-copyable if it's not complete, so it makes more sense to check these things in the opposite order. Testcase:

`struct A; extern A x, y; constexpr void f() { __builtin_memcpy(&x, &y, 4); }`

... currently produces a bogus diagnostic about `A` not being trivially-copyable but instead should produce your new diagnostic that `A` is incomplete.


Repository:
  rC Clang

https://reviews.llvm.org/D51855