llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Corentin Jabot (cor3ntin)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/146890.diff
6 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+1)
- (modified) clang/include/clang/Sema/Sema.h (+3-3)
- (modified) clang/lib/Parse/ParseExprCXX.cpp (+7-9)
- (modified) clang/lib/Sema/SemaExpr.cpp (+6-5)
- (modified) clang/lib/Sema/SemaExprCXX.cpp (+8-1)
- (modified) clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp (+33)
``````````diff
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) {}
+}
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/146890
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
