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



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

Reply via email to