[clang] [Clang] call HandleImmediateInvocation before checking for immediate escacalating expressions (reland) (PR #124708)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 27 22:55:46 PST 2025
https://github.com/cor3ntin created https://github.com/llvm/llvm-project/pull/124708
HandleImmediateInvocation can call MarkExpressionAsImmediateEscalating
and should always be called before CheckImmediateEscalatingFunctionDefinition.
However, we were not doing that in `ActFunctionBody`.
Fixes #119046
>From fb5d3dafcc72e815ed7122dc33b4622a26c16cd8 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Sat, 25 Jan 2025 16:54:18 +0100
Subject: [PATCH 1/3] [Clang] call HandleImmediateInvocation before checking
for immediate 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 #119046
---
clang/docs/ReleaseNotes.rst | 1 +
clang/include/clang/Sema/Sema.h | 6 +++---
clang/lib/Sema/SemaDecl.cpp | 1 -
clang/lib/Sema/SemaExpr.cpp | 3 +++
clang/test/CodeGenCXX/gh119046.cpp | 32 ++++++++++++++++++++++++++++++
5 files changed, 39 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/gh119046.cpp
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..f1d1e2e567193f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13139,10 +13139,10 @@ class Sema final : public SemaBase {
~SynthesizedFunctionScope() {
if (PushedCodeSynthesisContext)
S.popCodeSynthesisContext();
- if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext)) {
+
+ 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..accef4c5d5d916 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() &&
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 176627c3df37c6..e098b21045f01d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17881,6 +17881,9 @@ void Sema::PopExpressionEvaluationContext() {
WarnOnPendingNoDerefs(Rec);
HandleImmediateInvocations(*this, Rec);
+ if (auto *FD = dyn_cast<FunctionDecl>(CurContext); FD && getCurFunction())
+ CheckImmediateEscalatingFunctionDefinition(FD, getCurFunction());
+
// Warn on any volatile-qualified simple-assignments that are not discarded-
// value expressions nor unevaluated operands (those cases get removed from
// this list by CheckUnusedVolatileAssignment).
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
>From 6711aa9c0241f637464155ce0b506e30dd38a839 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Sun, 26 Jan 2025 13:58:49 +0100
Subject: [PATCH 2/3] Add additional test
---
clang/test/SemaCXX/cxx2b-consteval-propagate.cpp | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
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 at -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 at -1 {{call to immediate function 'GH119046::tfn<GH119046::S>' is not a constant expression}}
+ //expected-note at -2 {{read of non-const variable 'a' is not allowed in a constant expression}}
+}
+}
>From f58459d08ac3eae3b1c77fb4808da60b6e5bb7ec Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Tue, 28 Jan 2025 00:19:08 +0100
Subject: [PATCH 3/3] Address perf regression
---
clang/include/clang/Sema/Sema.h | 4 +++-
clang/lib/Sema/SemaDecl.cpp | 3 +++
clang/lib/Sema/SemaExpr.cpp | 3 ---
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f1d1e2e567193f..4f399536cc0ed3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13140,8 +13140,10 @@ class Sema final : public SemaBase {
if (PushedCodeSynthesisContext)
S.popCodeSynthesisContext();
- if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext))
+ 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 accef4c5d5d916..a5afafb750e81e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16396,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/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e098b21045f01d..176627c3df37c6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17881,9 +17881,6 @@ void Sema::PopExpressionEvaluationContext() {
WarnOnPendingNoDerefs(Rec);
HandleImmediateInvocations(*this, Rec);
- if (auto *FD = dyn_cast<FunctionDecl>(CurContext); FD && getCurFunction())
- CheckImmediateEscalatingFunctionDefinition(FD, getCurFunction());
-
// Warn on any volatile-qualified simple-assignments that are not discarded-
// value expressions nor unevaluated operands (those cases get removed from
// this list by CheckUnusedVolatileAssignment).
More information about the cfe-commits
mailing list