rsmith added inline comments. ================ Comment at: lib/Sema/SemaExprCXX.cpp:5636-5639 @@ -5634,4 +5635,6 @@ - Expr *E = ExprWithCleanups::Create(Context, SubExpr, Cleanups); + auto *E = ExprWithCleanups::Create(Context, SubExpr, Cleanups); + if (Cleanup.cleanupsHaveSideEffects()) + E->setCleanupsHaveSideEffects(); DiscardCleanupsInEvaluationContext(); ---------------- Please pass these flags into `::Create` rather than using a setter. Generally, we aim for the AST to be immutable once created.
================ Comment at: lib/Sema/SemaInit.cpp:6191 @@ +6190,3 @@ + if (getLangOpts().CPlusPlus) + Cleanup.setExprNeedsCleanups(MTE->getType().isDestructedType()); + return MTE; ---------------- 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. http://reviews.llvm.org/D20498 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
