-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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] Fix immediate escalation of template function specializations. #124404
Conversation
We record whether an expression is immediate escalating in the FunctionScope. However, that only happen when parsing or transforming an expression. This might not happen when transforming a non dependent expression. This patch fixes that by considering a function immediate when instantiated from an immediate function.
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesWe record whether an expression is immediate escalating in the FunctionScope. This patch fixes that by considering a function immediate when instantiated from an immediate function. Fixes #123405 Full diff: https://github.com/llvm/llvm-project/pull/124404.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e9fffddd507c66..1209ba3ec923d9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -991,6 +991,7 @@ Bug Fixes to C++ Support
- Fixed assertions or false compiler diagnostics in the case of C++ modules for
lambda functions or inline friend functions defined inside templates (#GH122493).
- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423)
+- Fixed immediate escalation of non-dependent expressions. (#GH123405)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5ce03ce20d2841..4753b1727f0dd4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3314,6 +3314,10 @@ bool FunctionDecl::isImmediateFunction() const {
.getConstructor()
->isImmediateFunction();
+ if (FunctionDecl *P = getTemplateInstantiationPattern();
+ P && P->isImmediateFunction())
+ return true;
+
if (const auto *MD = dyn_cast<CXXMethodDecl>(this);
MD && MD->isLambdaStaticInvoker())
return MD->getParent()->getLambdaCallOperator()->isImmediateFunction();
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index 3f3123eaee76b6..222d482f40aa5d 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -528,3 +528,21 @@ D d(0); // expected-note {{in implicit initialization for inherited constructor
// expected-error@-1 {{call to immediate function 'GH112677::D::SimpleCtor' is not a constant expression}}
}
+
+namespace GH123405 {
+
+consteval void fn() {}
+
+template <typename>
+constexpr int tfn(int) {
+ auto p = &fn; // expected-note {{'tfn<int>' is an immediate function because its body evaluates the address of a consteval function 'fn'}}
+ return int(p); // expected-error {{cast from pointer to smaller type 'int' loses information}}
+}
+
+int g() {
+ int a; // expected-note {{declared here}}
+ return tfn<int>(a); // expected-error {{call to immediate function 'GH123405::tfn<int>' is not a constant expression}}\
+ // expected-note {{read of non-const variable 'a' is not allowed in a constant expression}}
+}
+
+}
|
I'll push a fix for the build failure on arm shortly |
template <typename> | ||
constexpr int tfn(int) { | ||
auto p = &fn; // expected-note {{'tfn<int>' is an immediate function because its body evaluates the address of a consteval function 'fn'}} | ||
return int(p); // expected-error {{cast from pointer to smaller type 'int' loses information}} |
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.
Should we also have a success case as well for this particular scenario or is that covered by existing tests?
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.
i completely removed the conversion to it, it was not salient to the test. this is just testing that tfn becomes immediate
FYI this change has some measurable compile-time overhead (about 0.2% for debug builds): https://llvm-compile-time-tracker.com/compare.php?from=eaa5897534cbd263d0cdbf780f72133c2fe8d8d4&to=561132e71b29d9b747dfda1509f715847852f77b&stat=instructions:u Not sure whether that's expected or not. |
We record whether an expression is immediate escalating in the FunctionScope.
However, that only happen when parsing or transforming an expression. This might not happen when transforming a non dependent expression.
This patch fixes that by considering a function immediate when instantiated from an immediate function.
Fixes #123405