[clang] af2bb8f - [Clang] Correctly handle allocations in the condition of a `if constexpr` (#146890)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 4 06:40:32 PDT 2025
Author: Corentin Jabot
Date: 2025-07-04T16:40:29+03:00
New Revision: af2bb8f826050402fa9c6a6be8475808dcb63df0
URL: https://github.com/llvm/llvm-project/commit/af2bb8f826050402fa9c6a6be8475808dcb63df0
DIFF: https://github.com/llvm/llvm-project/commit/af2bb8f826050402fa9c6a6be8475808dcb63df0.diff
LOG: [Clang] Correctly handle allocations in the condition of a `if constexpr` (#146890)
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 expression _before_ evaluating the condition
(otherwise, automatic variables are never 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
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 73a80b6272d84..3cd4508db9202 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -899,6 +899,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
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3fe26f950ad51..b281b1cfef96a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/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:
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 1ea0cf52933f6..9f36c3ade5e74 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/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();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index aedaf5d1f6b1a..3c3d29c415249 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20636,6 +20636,7 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
break;
case ConditionKind::ConstexprIf:
+ // Note: this might produce a FullExpr
Cond = CheckBooleanCondition(Loc, SubExpr, true);
break;
@@ -20648,13 +20649,13 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
{SubExpr}, PreferredConditionType(CK));
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())
+ } else if (Cond.isUsable() && !isa<FullExpr>(Cond.get()))
+ Cond = ActOnFinishFullExpr(Cond.get(), Loc, /*DiscardedValue*/ false);
+
+ if (!Cond.isUsable())
return ConditionError();
- return ConditionResult(*this, nullptr, FullExpr,
+ return ConditionResult(*this, nullptr, Cond,
CK == ConditionKind::ConstexprIf);
}
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4a86cbd0633b6..f17a338825423 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/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(
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2f57672375a01..758012f894a41 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4561,6 +4561,13 @@ bool TreeTransform<Derived>::TransformExprs(Expr *const *Inputs,
template <typename Derived>
Sema::ConditionResult TreeTransform<Derived>::TransformCondition(
SourceLocation Loc, VarDecl *Var, Expr *Expr, Sema::ConditionKind Kind) {
+
+ EnterExpressionEvaluationContext Eval(
+ SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated,
+ /*LambdaContextDecl=*/nullptr,
+ /*ExprContext=*/Sema::ExpressionEvaluationContextRecord::EK_Other,
+ /*ShouldEnter=*/Kind == Sema::ConditionKind::ConstexprIf);
+
if (Var) {
VarDecl *ConditionVar = cast_or_null<VarDecl>(
getDerived().TransformDefinition(Var->getLocation(), Var));
diff --git a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
index ed8cbbbfe7067..c5d5427170394 100644
--- a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -242,3 +242,67 @@ void f() {
}
}
+
+namespace GH134820 {
+struct S {
+ char* c = new char;
+ constexpr ~S() {
+ delete c;
+ }
+ int i = 0;
+};
+
+int f() {
+ if constexpr((S{}, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(S s; (S{}, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(S s; (s, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(constexpr int _ = S{}.i; true) {
+ return 1;
+ }
+ return 0;
+}
+
+template <typename T>
+int f2() {
+ if constexpr((T{}, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(T s; (T{}, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(T s; (s, true)) { // expected-warning{{left operand of comma operator has no effect}}
+ return 1;
+ }
+ if constexpr(constexpr int _ = T{}.i; true) {
+ return 1;
+ }
+ return 0;
+}
+
+void test() {
+ f2<S>(); // expected-note {{in instantiation}}
+}
+
+}
+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) {}
+}
+}
More information about the cfe-commits
mailing list