Skip to content
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

[Clang] Fixes to immediate-escalating functions #82281

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

cor3ntin
Copy link
Contributor

  • Consider that immediate escalating function can appear at global scope, fixing a crash

  • Lambda conversion to function pointer was sometimes not performed in an immediate function context when it should be.

Fixes #82258

* Consider that immediate escalating function can appear at global
  scope, fixing a crash

* Lambda conversion to function pointer was sometimes
  not performed in an immediate function context when it should be.

Fixes llvm#82258
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 19, 2024

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes
  • Consider that immediate escalating function can appear at global scope, fixing a crash

  • Lambda conversion to function pointer was sometimes not performed in an immediate function context when it should be.

Fixes #82258


Full diff: https://github.com/llvm/llvm-project/pull/82281.diff

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+4)
  • (modified) clang/include/clang/Sema/Sema.h (+3-1)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+2-2)
  • (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+26)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 649ad655905af2..7524741e8ed0ab 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -267,6 +267,10 @@ Bug Fixes to C++ Support
   was only accepted at namespace scope but not at local function scope.
 - Clang no longer tries to call consteval constructors at runtime when they appear in a member initializer.
   (`#782154 <https://github.com/llvm/llvm-project/issues/82154>`_`)
+- Fix crash when using an immediate-escalated function at global scope.
+  (`#82258 <https://github.com/llvm/llvm-project/issues/82258>`_)
+- Correctly immediate-escalate lambda conversion functions.
+  (`#82258 <https://github.com/llvm/llvm-project/issues/82258>`_)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e9cd42ae777df5..6a71fc9a0bf87c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1158,7 +1158,9 @@ class Sema final {
       if (FD) {
         FD->setWillHaveBody(true);
         S.ExprEvalContexts.back().InImmediateFunctionContext =
-            FD->isImmediateFunction();
+            FD->isImmediateFunction() ||
+            S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
+                .isConstantEvaluated();
         S.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
             S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
       } else
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 37a7db889a6eac..816ee9e281359a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18311,7 +18311,6 @@ void Sema::CheckUnusedVolatileAssignment(Expr *E) {
 }
 
 void Sema::MarkExpressionAsImmediateEscalating(Expr *E) {
-  assert(!FunctionScopes.empty() && "Expected a function scope");
   assert(getLangOpts().CPlusPlus20 &&
          ExprEvalContexts.back().InImmediateEscalatingFunctionContext &&
          "Cannot mark an immediate escalating expression outside of an "
@@ -18328,7 +18327,8 @@ void Sema::MarkExpressionAsImmediateEscalating(Expr *E) {
   } else {
     assert(false && "expected an immediately escalating expression");
   }
-  getCurFunction()->FoundImmediateEscalatingExpression = true;
+  if (FunctionScopeInfo *FI = getCurFunction())
+    FI->FoundImmediateEscalatingExpression = true;
 }
 
 ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 531a6262287335..4a75392045d05a 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -368,3 +368,29 @@ vector<void> v{};
 // expected-note@-2 {{in call to 'vector()'}}
 
 }
+
+
+namespace GH82258 {
+
+template <class R, class Pred>
+constexpr auto none_of(R&& r, Pred pred) -> bool { return true; }
+
+struct info { int value; };
+consteval auto is_invalid(info i) -> bool { return false; }
+constexpr info types[] = { {1}, {3}, {5}};
+
+static_assert(none_of(
+    types,
+    +[](info i) consteval {
+        return is_invalid(i);
+    }
+));
+
+static_assert(none_of(
+    types,
+    []{
+        return is_invalid;
+    }()
+));
+
+}

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -1158,7 +1158,9 @@ class Sema final {
if (FD) {
FD->setWillHaveBody(true);
S.ExprEvalContexts.back().InImmediateFunctionContext =
FD->isImmediateFunction();
FD->isImmediateFunction() ||
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Someday... I would like to replace this with a function call so I can stop going "wait, is -2 okay??" each time I see it. ;-)

@cor3ntin cor3ntin merged commit baf6bd3 into llvm:main Feb 21, 2024
7 of 8 checks passed
@cor3ntin cor3ntin deleted the gh82258 branch February 21, 2024 19:53
steakhal pushed a commit to steakhal/llvm-project that referenced this pull request Feb 22, 2024
* Consider that immediate escalating function can appear at global
scope, fixing a crash

* Lambda conversion to function pointer was sometimes not performed in
an immediate function context when it should be.

Fixes llvm#82258

(cherry picked from commit baf6bd3)
@steakhal
Copy link
Contributor

I've submitted a PR #82609 to backport this commit to release/18.x as it fixes a crash and would be also important for us downstream.

steakhal pushed a commit to steakhal/llvm-project that referenced this pull request Feb 23, 2024
* Consider that immediate escalating function can appear at global
scope, fixing a crash

* Lambda conversion to function pointer was sometimes not performed in
an immediate function context when it should be.

Fixes llvm#82258

(cherry picked from commit baf6bd3)
tstellar pushed a commit that referenced this pull request Feb 27, 2024
* Consider that immediate escalating function can appear at global
scope, fixing a crash

* Lambda conversion to function pointer was sometimes not performed in
an immediate function context when it should be.

Fixes #82258

(cherry picked from commit baf6bd3)
@pointhex pointhex mentioned this pull request May 7, 2024
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.

Crash in frontend when trying examples of P2564R3 (propagating consteval)
4 participants