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