rjmccall added a comment.

I accidentally hit 'send' too early on my review, so here's part two.  I still 
haven't fully reviewed the new IRGen file.



================
Comment at: lib/CodeGen/CodeGenCStruct.cpp:27
+  FK_Array   // array that has non-trivial elements.
+};
+}
----------------
There's no need to put this in an anonymous namespace.

This enum makes me wonder if it would make more sense to create equivalents to 
QualType::DestructionKind for each of these operations and all of the different 
primitive types.  That would let you e.g. implement your only-strong-members 
check above much more easily, and it would also make it simpler to guarantee 
that all the places that need to exhaustively enumerate the reasons why 
something might need special initialization/copying logic don't miss an 
important new case.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:1144
+    // indirectly, the callee is responsible for destructing the argument.
+    if (Ty.hasNonTrivialToDestroyStruct()) {
+      AutoVarEmission Emission(*VD);
----------------
You're not actually checking the "is not passed indirectly" part of this, but I 
think that's okay, because I don't think we actually want the ownership aspects 
of the ABI to vary based on how the argument is passed.  So you should just 
strike that part from the comment.

Also, this should really be done in EmitParmDecl.


================
Comment at: lib/CodeGen/CodeGenFunction.h:3347
+                                  QualType QT, bool IsVolatile);
+  void callDestructor(Address DstPtr, QualType QT, bool IsVolatile);
+
----------------
These methods should *definitely* be somehow namespaced for your new feature.


https://reviews.llvm.org/D41228



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

Reply via email to