[PATCH] D20498: [Temporary] Add an ExprWithCleanups for each C++ MaterializeTemporaryExpr

[Tim Shen via cfe-commits]

timshen marked 7 inline comments as done.

================
Comment at: lib/Sema/SemaInit.cpp:6190-6191
@@ +6189,4 @@
+      MaterializeTemporaryExpr(T, Temporary, BoundToLvalueReference);
+
+  // Order an ExprWithCleanups for lifetime marks.
+  //
----------------
> Please also sink the calls to mark the destructor referenced and check its access into here too. We should keep the checks for a usable destructor and the triggering of an ExprWithCleanups for the destructor together.

As discussed, MaybeBindToTemporary more or less overlaps with this in the access checking perspective. Added a TODO for a potential merge of MaterializeTemporaryExpr and BindToCXXTemporary in the future.

> I think this may set ExprNeedsCleanups even when unnecessary, if the MTE is lifetime-extended (I think you only actually want an ExprWithCleanups if the storage duration is SD_Automatic). We can't really deal with that here, because we don't know the storage duration of the MTE yet, but please add a FIXME here to not set ExprNeedsCleanups if the temporary is lifetime-extended.

Due to the use of pushCleanupAfterFullExpr in CodeGen::EmitMaterializeTemporaryExpr, there has to have a RunCleanupsScope at FullExpr level to delay and forward the cleanups to its upper level. So even for lifetime-extended variables, the cleanup scope is necessary.


http://reviews.llvm.org/D20498