[PATCH] D45310: Warn about memcpy'ing non-trivial C structs

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:12
+//
+//===----------------------------------------------------------------------===//
+
----------------
The header comment here was clearly just copied from another file.

I would name this header "NonTrivialTypeVisitor.h"; I don't think the CStruct adds anything, especially because the visitors actually start from types.


================
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
+
----------------
Should you have this pass the array type down?  And is it really important to do this in the generic visitor?  It seems like something you could do in an IRGen subclass.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:619
+  "%select{primitive-default-initialize|primitive-copy}3">,
+  InGroup<DiagGroup<"cstruct-memaccess">>;
+def note_nontrivial_field : Note<
----------------
I think this warning group should be -Wnontrivial-memaccess, and maybe -Wclass-memaccess should just be a subgroup of it.


================
Comment at: lib/Sema/SemaChecking.cpp:7343
+                  SourceLocation SL) {
+    const auto *AT = getContext().getAsConstantArrayType(FT);
+    visit(PDIK, getContext().getBaseElementType(AT), SL);
----------------
It would be better to just getAsArrayType, for generality purposes.


Repository:
  rC Clang

https://reviews.llvm.org/D45310