-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang] Fixed Constant Evaluation don't Call Destructor #140278
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
Conversation
@llvm/pr-subscribers-clang Author: Vincent (Mr-Anyone) ChangesWithin the condition statement of the for block, the destructor doesn't get when evaluating compile time constants. resolves #139818 Full diff: https://github.com/llvm/llvm-project/pull/140278.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c12091a90add..76c139632b31c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -623,6 +623,7 @@ Bug Fixes in This Version
- Fix crash due to unknown references and pointer implementation and handling of
base classes. (GH139452)
- Fixed an assertion failure in serialization of constexpr structs containing unions. (#GH140130)
+- Fix constant evaluation in for loop not calling destructor (#GH139818)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index ca1fbdf7e652f..d8364977258a1 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -5742,8 +5742,12 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info,
if (FS->getCond() && !EvaluateCond(Info, FS->getConditionVariable(),
FS->getCond(), Continue))
return ESR_Failed;
- if (!Continue)
+
+ if (!Continue) {
+ if (!IterScope.destroy())
+ return ESR_Failed;
break;
+ }
EvalStmtResult ESR = EvaluateLoopBody(Result, Info, FS->getBody());
if (ESR != ESR_Continue) {
diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp
index b53c67ee67932..7a47f2784ceb3 100644
--- a/clang/test/SemaCXX/static-assert-cxx26.cpp
+++ b/clang/test/SemaCXX/static-assert-cxx26.cpp
@@ -416,3 +416,38 @@ static_assert(
// expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
);
}
+
+// taken from: https://github.com/llvm/llvm-project/issues/139818
+namespace GH139818{
+ struct A {
+ constexpr ~A() { ref = false; }
+ constexpr operator bool() {
+ return b;
+ }
+ bool b;
+ bool& ref;
+ };
+
+ constexpr bool f1() {
+ bool ret = true;
+ for (bool b = false; A x{b, ret}; b = true) {}
+ return ret;
+ }
+
+ static_assert(!f1());
+
+ struct Y {
+ constexpr ~Y() noexcept(false) { throw "oops"; } // expected-error {{cannot use 'throw' with exceptions disabled}}
+ // expected-note@-1 {{subexpression not valid in a constant expression}}
+ constexpr operator bool() {
+ return b;
+ }
+ bool b;
+ };
+ constexpr bool f2() {
+ for (bool b = false; Y x = {b}; b = true) {} // expected-note {{in call to 'x.~Y()'}}
+ return true;
+ }
+ static_assert(f2()); // expected-error {{static assertion expression is not an integral constant expression}}
+ // expected-note@-1 {{in call to 'f2()'}}
+};
|
258ba7f
to
7635d4d
Compare
Within the condition statement of the for block, the destructor doesn't get called during compile time evaluated constants. resolves llvm#139818
Co-authored-by: Sirraide <aeternalmail@gmail.com>
7635d4d
to
123f6b5
Compare
ping |
Documentation fail seems to be fixed by #142387 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment but otherwise this LGTM.
Yeah, the changes to the release notes look fine, so if CI is unhappy about them then that’s for some other reason. |
@Sirraide I don't have merge access. Could you please merge? Thanks. |
Within the condition statement of the for block, the destructor doesn't get when evaluating compile time constants.
resolves #139818