Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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:
Expand Down
16 changes: 7 additions & 9 deletions clang/lib/Parse/ParseExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to make an equivalent change to TreeTransform?

I came up with the following testcase:

struct S {
    char* c = new char;
    consteval S(){}
    constexpr ~S() {
        delete c;
    }
};
template<typename T> 
int f() {
    if constexpr((T{}, true)) {
        return 1;
    }
    return 0;
}
auto ff = f<S>;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, thanks for thinking about that


ExprResult Expr = ParseExpression();

if (Expr.isInvalid())
return Sema::ConditionError();
Expand Down
11 changes: 6 additions & 5 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20628,6 +20628,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;

Expand All @@ -20640,13 +20641,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);
}

Expand Down
9 changes: 8 additions & 1 deletion clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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(
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
64 changes: 64 additions & 0 deletions clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
}
}