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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
+
----------------
rjmccall wrote:
> 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.
The subclasses in CGNonTrivialStruct.cpp need the size and the element type of the array to be passed to visitArray, so I think we have to pass the array type to visitArray. I guess it's possible to move this to the subclasses, but then the visit methods in the subclasses have to check whether the type is an array or not. I think we had a discussion on how arrays should be handled in this review: https://reviews.llvm.org/D41228.

But perhaps you have a better idea in mind?


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:619
+  "%select{primitive-default-initialize|primitive-copy}3">,
+  InGroup<DiagGroup<"cstruct-memaccess">>;
+def note_nontrivial_field : Note<
----------------
rjmccall wrote:
> I think this warning group should be -Wnontrivial-memaccess, and maybe -Wclass-memaccess should just be a subgroup of it.
Yes, we can create different DiagGroups when support for -Wclass-memaccess (warning for C++ classes) is added.


Repository:
  rC Clang

https://reviews.llvm.org/D45310