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] call HandleImmediateInvocation before checking for immediate escacalating expressions (reland) #124708

Merged
merged 4 commits into from
Jan 28, 2025

Conversation

cor3ntin
Copy link
Contributor

HandleImmediateInvocation can call MarkExpressionAsImmediateEscalating
and should always be called before CheckImmediateEscalatingFunctionDefinition.

However, we were not doing that in ActFunctionBody.

Fixes #119046

…escacalating expressions

HandleImmediateInvocation can call MarkExpressionAsImmediateEscalating
and should always be called before CheckImmediateEscalatingFunctionDefinition.

However, we were not doing that in `ActFunctionBody`.

We simply move CheckImmediateEscalatingFunctionDefinition to
PopExpressionEvaluationContext.

Fixes llvm#119046
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 28, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

HandleImmediateInvocation can call MarkExpressionAsImmediateEscalating
and should always be called before CheckImmediateEscalatingFunctionDefinition.

However, we were not doing that in ActFunctionBody.

Fixes #119046


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/include/clang/Sema/Sema.h (+2)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+3-1)
  • (added) clang/test/CodeGenCXX/gh119046.cpp (+32)
  • (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+16)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index af3632322b8453..29d549efc7382c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1008,6 +1008,7 @@ Bug Fixes to C++ Support
 - Fix type of expression when calling a template which returns an ``__array_rank`` querying a type depending on a
   template parameter. Now, such expression can be used with ``static_assert`` and ``constexpr``. (#GH123498)
 - Correctly determine the implicit constexprness of lambdas in dependent contexts. (#GH97958) (#GH114234)
+- Fix that some dependent immediate expressions did not cause immediate escalation (#GH119046)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 528304409b8092..4f399536cc0ed3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13139,10 +13139,12 @@ class Sema final : public SemaBase {
     ~SynthesizedFunctionScope() {
       if (PushedCodeSynthesisContext)
         S.popCodeSynthesisContext();
+
       if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext)) {
         FD->setWillHaveBody(false);
         S.CheckImmediateEscalatingFunctionDefinition(FD, S.getCurFunction());
       }
+
       S.PopExpressionEvaluationContext();
       S.PopFunctionScopeInfo();
     }
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fe68eadc951b5f..a5afafb750e81e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16019,7 +16019,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
       if (!FD->isDeletedAsWritten())
         FD->setBody(Body);
       FD->setWillHaveBody(false);
-      CheckImmediateEscalatingFunctionDefinition(FD, FSI);
 
       if (getLangOpts().CPlusPlus14) {
         if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
@@ -16397,6 +16396,9 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
     // the declaration context below. Otherwise, we're unable to transform
     // 'this' expressions when transforming immediate context functions.
 
+  if(FD)
+    CheckImmediateEscalatingFunctionDefinition(FD, getCurFunction());
+
   if (!IsInstantiation)
     PopDeclContext();
 
diff --git a/clang/test/CodeGenCXX/gh119046.cpp b/clang/test/CodeGenCXX/gh119046.cpp
new file mode 100644
index 00000000000000..cad76879f08624
--- /dev/null
+++ b/clang/test/CodeGenCXX/gh119046.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++2a -triple x86_64-elf-gnu %s -emit-llvm -o - | FileCheck %s
+
+struct S {
+    consteval void operator()() {}
+};
+
+template <class Fn>
+constexpr void dispatch(Fn fn) {
+    fn();
+}
+
+template <class Visitor>
+struct value_visitor {
+    constexpr void operator()() { visitor(); }
+    Visitor&& visitor;
+};
+
+template <class Visitor>
+constexpr auto make_dispatch() {
+    return dispatch<value_visitor<S>>;
+}
+
+template <class Visitor>
+constexpr void visit(Visitor&&) {
+    make_dispatch<Visitor>();
+}
+
+void f() { visit(S{}); }
+
+// CHECK: define {{.*}} @_Z1fv
+// CHECK-NOT: define {{.*}} @_Z5visitI1SEvOT_
+// CHECK-NOT: define {{.*}} @_Z13make_dispatchI1SEDav
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 05904d9ade067f..dd5063cb29c5b7 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -560,3 +560,19 @@ void foo() {
     S s;
 }
 } // namespace GH118000
+
+namespace GH119046 {
+
+template <typename Cls> constexpr auto tfn(int) {
+  return (unsigned long long)(&Cls::sfn);
+  //expected-note@-1 {{'tfn<GH119046::S>' is an immediate function because its body evaluates the address of a consteval function 'sfn'}}
+};
+struct S { static consteval void sfn() {} };
+
+int f() {
+  int a = 0; // expected-note{{declared here}}
+  return tfn<S>(a);
+  //expected-error@-1 {{call to immediate function 'GH119046::tfn<GH119046::S>' is not a constant expression}}
+  //expected-note@-2 {{read of non-const variable 'a' is not allowed in a constant expression}}
+}
+}

Copy link

github-actions bot commented Jan 28, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@cor3ntin cor3ntin merged commit 820c6ac into llvm:main Jan 28, 2025
9 checks passed
@cor3ntin cor3ntin deleted the perf/cor3ntin/check_escalation branch January 28, 2025 14:18
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] Linker error after immediate escalation
3 participants