Skip to content

Conversation

@Mr-Anyone
Copy link
Contributor

Within the condition statement of the for block, the destructor doesn't get when evaluating compile time constants.

resolves #139818

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 16, 2025
@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-clang

Author: Vincent (Mr-Anyone)

Changes

Within 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:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/AST/ExprConstant.cpp (+5-1)
  • (modified) clang/test/SemaCXX/static-assert-cxx26.cpp (+35)
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()'}}
+};

@Mr-Anyone Mr-Anyone requested a review from Sirraide May 20, 2025 20:44
@Mr-Anyone Mr-Anyone force-pushed the constant_evaluation branch 2 times, most recently from 258ba7f to 7635d4d Compare May 21, 2025 19:05
Mr-Anyone and others added 5 commits June 2, 2025 15:43
Within the condition statement of the for block, the destructor
doesn't get called during compile time evaluated constants.

resolves llvm#139818
@Mr-Anyone Mr-Anyone force-pushed the constant_evaluation branch from 7635d4d to 123f6b5 Compare June 2, 2025 08:09
@Mr-Anyone
Copy link
Contributor Author

ping

@Mr-Anyone
Copy link
Contributor Author

Mr-Anyone commented Jun 3, 2025

Documentation fail seems to be fixed by #142387

Copy link
Member

@Sirraide Sirraide left a 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.

@Sirraide
Copy link
Member

Sirraide commented Jun 3, 2025

Documentation fail seems to be fixed by #142387

Yeah, the changes to the release notes look fine, so if CI is unhappy about them then that’s for some other reason.

@Mr-Anyone Mr-Anyone requested a review from Sirraide June 3, 2025 15:50
@Mr-Anyone Mr-Anyone requested a review from zyn0217 June 4, 2025 06:58
@Mr-Anyone
Copy link
Contributor Author

@Sirraide I don't have merge access. Could you please merge? Thanks.

@Sirraide Sirraide merged commit 49386f4 into llvm:main Jun 5, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[clang] constant evaluation of for loop does not run destructor for condition variable

4 participants