[Clang] Correctly handle allocations in the condition of a if constexpr

Deal with the following scenario

```cpp
struct S {
    char* c = new char;
    constexpr ~S() {
        delete c;
    }
};
if constexpr((S{}, true)){};
```

There were two issues
 - We need to produce a full expressions _before_ evaluating the condition
   (otherwise automatic variables are neber destroyed)
 - We need to preserve the evaluation context of the condition while doing
   the semantics analysis for it (lest it is evaluated in a non constant
   evaluated context)

Fixes #120197
Fixes #134820

Author: cor3ntin

clang/docs/ReleaseNotes.rst

@@ -896,6 +896,7 @@ Bug Fixes to C++ Support
 - Fixed a crash when constant evaluating some explicit object member assignment operators. (#GH142835)
 - Fixed an access checking bug when substituting into concepts (#GH115838)
 - Fix a bug where private access specifier of overloaded function not respected. (#GH107629)
+- Correctly handle allocations in the condition of a ``if constexpr``.(#GH120197) (#GH134820)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Sema/Sema.h

@@ -7723,12 +7723,12 @@ class Sema final : public SemaBase {
 
   class ConditionResult {
     Decl *ConditionVar;
-    FullExprArg Condition;
+    ExprResult Condition;
     bool Invalid;
     std::optional<bool> KnownValue;
 
     friend class Sema;
-    ConditionResult(Sema &S, Decl *ConditionVar, FullExprArg Condition,
+    ConditionResult(Sema &S, Decl *ConditionVar, ExprResult Condition,
                     bool IsConstexpr)
         : ConditionVar(ConditionVar), Condition(Condition), Invalid(false) {
       if (IsConstexpr && Condition.get()) {
@@ -7739,7 +7739,7 @@ class Sema final : public SemaBase {
       }
     }
     explicit ConditionResult(bool Invalid)
-        : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
+        : ConditionVar(nullptr), Condition(Invalid), Invalid(Invalid),
           KnownValue(std::nullopt) {}
 
   public:

clang/lib/Parse/ParseExprCXX.cpp

@@ -1931,15 +1931,13 @@ Parser::ParseCXXCondition(StmtResult *InitStmt, SourceLocation Loc,
       return ParseCXXCondition(nullptr, Loc, CK, MissingOK);
     }
 
-    ExprResult Expr = [&] {
-      EnterExpressionEvaluationContext Eval(
-          Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated,
-          /*LambdaContextDecl=*/nullptr,
-          /*ExprContext=*/Sema::ExpressionEvaluationContextRecord::EK_Other,
-          /*ShouldEnter=*/CK == Sema::ConditionKind::ConstexprIf);
-      // Parse the expression.
-      return ParseExpression(); // expression
-    }();
+    EnterExpressionEvaluationContext Eval(
+        Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated,
+        /*LambdaContextDecl=*/nullptr,
+        /*ExprContext=*/Sema::ExpressionEvaluationContextRecord::EK_Other,
+        /*ShouldEnter=*/CK == Sema::ConditionKind::ConstexprIf);
+
+    ExprResult Expr = ParseExpression();
 
     if (Expr.isInvalid())
       return Sema::ConditionError();

clang/lib/Sema/SemaExpr.cpp

@@ -20629,6 +20629,9 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
 
   case ConditionKind::ConstexprIf:
     Cond = CheckBooleanCondition(Loc, SubExpr, true);
+    assert(isa<FullExpr>(Cond.get()) &&
+           "we should have converted this expression to a FullExpr before "
+           "evaluating it");
     break;
 
   case ConditionKind::Switch:
@@ -20641,12 +20644,10 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
     if (!Cond.get())
       return ConditionError();
   }
-  // FIXME: FullExprArg doesn't have an invalid bit, so check nullness instead.
-  FullExprArg FullExpr = MakeFullExpr(Cond.get(), Loc);
-  if (!FullExpr.get())
-    return ConditionError();
+  if (!isa<FullExpr>(Cond.get()))
+    Cond = ActOnFinishFullExpr(Cond.get(), Loc, /*DiscardedValue*/ false);
 
-  return ConditionResult(*this, nullptr, FullExpr,
+  return ConditionResult(*this, nullptr, Cond,
                          CK == ConditionKind::ConstexprIf);
 }
 

clang/lib/Sema/SemaExprCXX.cpp

@@ -4357,7 +4357,8 @@ Sema::ConditionResult Sema::ActOnConditionVariable(Decl *ConditionVar,
       CheckConditionVariable(cast<VarDecl>(ConditionVar), StmtLoc, CK);
   if (E.isInvalid())
     return ConditionError();
-  return ConditionResult(*this, ConditionVar, MakeFullExpr(E.get(), StmtLoc),
+  E = ActOnFinishFullExpr(E.get(), /*DiscardedValue*/ false);
+  return ConditionResult(*this, ConditionVar, E,
                          CK == ConditionKind::ConstexprIf);
 }
 
@@ -4417,6 +4418,12 @@ ExprResult Sema::CheckCXXBooleanCondition(Expr *CondExpr, bool IsConstexpr) {
   if (!IsConstexpr || E.isInvalid() || E.get()->isValueDependent())
     return E;
 
+  E = ActOnFinishFullExpr(E.get(), E.get()->getExprLoc(),
+                          /*DiscardedValue*/ false,
+                          /*IsConstexpr*/ true);
+  if (E.isInvalid())
+    return E;
+
   // FIXME: Return this value to the caller so they don't need to recompute it.
   llvm::APSInt Cond;
   E = VerifyIntegerConstantExpression(

clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp

@@ -242,3 +242,36 @@ void f() {
 }
 
 }
+
+namespace GH134820 {
+struct S {
+    char* c = new char;
+    constexpr ~S() {
+        delete c;
+    }
+};
+
+int f() {
+    if constexpr((S{}, true)) {
+        return 1;
+    }
+    return 0;
+}
+}
+
+namespace GH120197{
+struct NonTrivialDtor {
+  NonTrivialDtor() = default;
+  NonTrivialDtor(const NonTrivialDtor&) = default;
+  NonTrivialDtor(NonTrivialDtor&&) = default;
+  NonTrivialDtor& operator=(const NonTrivialDtor&)  = default;
+  NonTrivialDtor& operator=(NonTrivialDtor&&) = default;
+  constexpr ~NonTrivialDtor() noexcept {}
+};
+
+static_assert(((void)NonTrivialDtor{}, true)); // passes
+
+void f() {
+  if constexpr ((void)NonTrivialDtor{}, true) {}
+}
+}