[clang] [CIR] Fix cleanup of temporaries with null-checked new expr (PR #213376)

[Andy Kaylor via cfe-commits]

================
@@ -26,23 +26,36 @@ using namespace clang;
 using namespace clang::CIRGen;
 
 namespace {
-/// Return true if the expression tree contains an AbstractConditionalOperator
-/// (ternary ?:), which is the only construct whose CIR codegen calls
-/// ConditionalEvaluation::beginEvaluation() and thus causes cleanups to be
-/// deferred via pushFullExprCleanup.  Logical &&/|| do NOT call
-/// beginEvaluation(); their branch-local cleanups are handled by LexicalScope.
+/// Return true if the expression tree contains a construct that causes cleanups
+/// to be deferred via pushFullExprCleanup.
 class ConditionalEvaluationFinder
     : public RecursiveASTVisitor<ConditionalEvaluationFinder> {
+  const ASTContext &astContext;
   bool foundConditional = false;
 
 public:
+  ConditionalEvaluationFinder(const ASTContext &astContext)
+      : astContext(astContext) {}
+
   bool found() const { return foundConditional; }
 
   bool VisitAbstractConditionalOperator(AbstractConditionalOperator *) {
     foundConditional = true;
     return false;
   }
 
+  bool VisitCXXNewExpr(CXXNewExpr *e) {
+    // If the new expression requires a null check, its initializer may be
+    // skipped. In that case, the cleanup for any temporaries created in the
+    // initializer must be conditional.
+    if (e->shouldNullCheckAllocation() &&
----------------
andykaylor wrote:

Looking at classic codegen, I see that it also puts `|| sanitizerPerformTypeCheck()` in the second part of this condition. Thinking about what that means for this code, I think maybe I'm putting more than we need in the check here. This was duplicating the conditions under which we perform a null check, but the conditional case only happens if there is an initializer and even then only if the initializer requires cleanup. If the allocation itself requires cleanup (that is, if we need to delete the allocated pointer in the case where the constructor throws), the cleanup is emitted inside the null check true region).

So, I don't we do want to share this check with the other location, but I think I can simplify it here.

https://github.com/llvm/llvm-project/pull/213376