-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[clang] Consistently handle consteval constructors for variables. #144970
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-coroutines @llvm/pr-subscribers-clang Author: Eli Friedman (efriedma-quic) ChangesThis is a simple extension of 443377a to also handle the implicit expressions created by default initialization. This usually doesn't matter, but it's relevant if the constructor stores "this" as a member. Fixes #135281 . Full diff: https://github.com/llvm/llvm-project/pull/144970.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bbd63372c168b..e10dc65897b8a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14423,6 +14423,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
Var->getType().getAddressSpace() == LangAS::hlsl_input)
return;
+ if (getLangOpts().CPlusPlus)
+ ActOnCXXEnterDeclInitializer(nullptr, Var);
+
// C++03 [dcl.init]p9:
// If no initializer is specified for an object, and the
// object is of (possibly cv-qualified) non-POD class type (or
@@ -14458,6 +14461,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
}
CheckCompleteVariableDeclaration(Var);
+
+ if (getLangOpts().CPlusPlus)
+ ActOnCXXExitDeclInitializer(nullptr, Var);
}
}
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index dd5063cb29c5b..aa5d79af589ca 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -576,3 +576,21 @@ int f() {
//expected-note@-2 {{read of non-const variable 'a' is not allowed in a constant expression}}
}
}
+
+#if __cplusplus >= 202302L
+namespace GH135281 {
+ struct B {
+ const void* p;
+ consteval B() : p{this} {}
+ };
+ B b;
+ B b2{};
+ B &&b3{};
+ void f() {
+ static B b4;
+ B b5; // expected-error {{call to consteval function 'GH135281::B::B' is not a constant expression}} \
+ // expected-note {{pointer to temporary is not a constant expression}} \
+ // expected-note {{temporary created here}}
+ }
+}
+#endif
|
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.
LGTM!
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 am a little suspicious of this change because right now we are only calling ActOnCXXExitDeclInitializer
in ~InitializerScopeRAII()
and now it seems like we need to sprinkle it in other places.
ActOnCXXExitDeclInitializer does three things:
The special handling for variables is specifically relevant for variables... I don't see why we'd need it anywhere else? Except, hmm... I guess we need the same handling if we're instantiating a template, like:
But I can't think of anything else. Any suggestions for how to refactor? |
b508252
to
75ae080
Compare
75ae080
to
b423545
Compare
Came up with a different approach: I moved the check to PopExpressionEvaluationContext(). This required annotating the ExpressionEvaluationContextRecord to indicate we're handling a variable initializer. |
443377a handled simple variables definitions, but it didn't handle uninitialized variables with a constexpr constructor, and it didn't handle template instantiation. Fixes llvm#135281 .
b423545
to
b1f7402
Compare
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.
Can you add a changelog entry? LGTM otherwiswe
443377a handled simple variable definitions, but it didn't handle uninitialized variables with a consteval constructor, and it didn't handle template instantiation.
Fixes #135281 .