https://github.com/cor3ntin updated
https://github.com/llvm/llvm-project/pull/146890
>From c138801c212f69f4dee83c7516e0e77eb876a9f0 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Thu, 3 Jul 2025 14:26:59 +0200
Subject: [PATCH 1/2] [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
---
clang/docs/ReleaseNotes.rst | 1 +
clang/include/clang/Sema/Sema.h | 6 ++--
clang/lib/Parse/ParseExprCXX.cpp | 16 ++++-----
clang/lib/Sema/SemaExpr.cpp | 11 ++++---
clang/lib/Sema/SemaExprCXX.cpp | 9 ++++-
.../test/SemaCXX/cxx2a-constexpr-dynalloc.cpp | 33 +++++++++++++++++++
6 files changed, 58 insertions(+), 18 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3d893e0aa8e2c..9f9fc84b53104 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
^^^^^^^^^^^^^^^^^^^^^^^^^
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 437df742d572b..7c65489cc6234 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/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);
}
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/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
index ed8cbbbfe7067..edffe177e08c7 100644
--- a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ b/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) {}
+}
+}
>From 7d6b68a4a6d1cc6356d34dd60ad513de9b2a78bc Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Thu, 3 Jul 2025 17:00:39 +0200
Subject: [PATCH 2/2] add tests and remove bogus assert
---
clang/lib/Sema/SemaExpr.cpp | 10 +++++-----
clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp | 8 +++++++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7c65489cc6234..1a352fbcb3cbb 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20628,10 +20628,8 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S,
SourceLocation Loc,
break;
case ConditionKind::ConstexprIf:
+ // Note: this might produce a FullExpr
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:
@@ -20643,10 +20641,12 @@ Sema::ConditionResult Sema::ActOnCondition(Scope *S,
SourceLocation Loc,
{SubExpr}, PreferredConditionType(CK));
if (!Cond.get())
return ConditionError();
- }
- if (!isa<FullExpr>(Cond.get()))
+ } else if (Cond.isUsable() && !isa<FullExpr>(Cond.get()))
Cond = ActOnFinishFullExpr(Cond.get(), Loc, /*DiscardedValue*/ false);
+ if (Cond.isInvalid())
+ return ConditionError();
+
return ConditionResult(*this, nullptr, Cond,
CK == ConditionKind::ConstexprIf);
}
diff --git a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
index edffe177e08c7..606e06957c4ea 100644
--- a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -252,7 +252,13 @@ struct S {
};
int f() {
- if constexpr((S{}, true)) {
+ 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;
}
return 0;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
