ahatanak added inline comments.
================
Comment at: include/clang/AST/Type.h:1137
+ return isNonTrivialToPrimitiveDestroy() == PCK_Struct;
+ }
+
----------------
rjmccall wrote:
> Are these four queries really important enough to provide API for them on
> QualType?
I removed these functions.
================
Comment at: lib/CodeGen/CGDecl.cpp:1299
+ return;
+ }
+
----------------
rjmccall wrote:
> Again, it would be nice to restructure this code to use the result of
> isNonTrivialToPrimitiveDefaultInitialize() exhaustively.
It looks like this code needs restructuring, but I didn't understand how I can
use isNonTrivialToPrimitiveDefaultInitialize() here. The only change I made
here is that drillIntoBlockVariable is called if the variable is byref. This
fixes a bug where a non-trivial struct variable declared as __block wasn't
initialized correctly.
Also, in the process, I found that when a c++ struct declared as a __block is
captured by its initializer, an assertion fails (see test case in
test/CodeGenObjCXX/arc-blocks.mm). I fixed the bug in
CodeGenFunction::EmitExprAsInit, but I'm not sure that's the right way to fix
it. I'm not sure what the comment "how can we delay here if D is captured by
its initializer" means.
================
Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:38
+ template <class Visitor, class... Ts>
+ static void visit(Visitor &V, QualType FT, bool IsVolatile, unsigned Offset,
+ Ts... Args) {
----------------
rjmccall wrote:
> These visitors would be re-usable components outside of IRGen if they just
> took a QualType and then forwarded an arbitrary set of other arguments. I
> would suggest structuring them like this:
>
> template <class Derived, class RetTy> struct DestructedTypeVisitor {
> Derived &asDerived() { return static_cast<Derived&>(*this); }
>
> template <class... Ts>
> RetTy visit(QualType Ty, Ts &&...Args) {
> return asDerived().visit(Ty.isDestructedType(), Ty,
> std::forward<Ts>(Args)...);
> }
>
> template <class... Ts>
> RetTy visit(QualType::DestructionKind DK, QualType Ty, Ts &&...Args) {
> switch (DK) {
> case QualType::DK_none: return asDerived().visitTrivial(Ty,
> std::forward<Ts>(Args)...);
> case QualType::DK_cxx_destructor: return
> asDerived().visitCXXDestructor(Ty, std::forward<Ts>(Args)...);
> ...
> }
> }
> };
>
> You can then pretty easily add the special behavior for IsVolatile and arrays
> (when DK is not DK_none) in an IRGen-specific subclass. visitArray would
> take a DK and then recurse by calling back into visit with that same DK.
I created three visitor classes that can be used outside of IRGen,
================
Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:55
+ break;
+ default:
+ break;
----------------
rjmccall wrote:
> Better not to have a default case here. You can add an assert for the C++
> case that it should never get here.
I added llvm_unreachable in StructVisitor's methods visitWeak and
visitCXXDestructor.
https://reviews.llvm.org/D41228
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
