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) {} +} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
