-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[Clang][Sema] Reject undeduced static data members without initializers (#173349) #174281
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][Sema] Reject undeduced static data members without initializers (#173349) #174281
Conversation
|
@llvm/pr-subscribers-clang Author: Hamza Hassanain (HamzaHassanain) ChangesThe ProblemClang crashes when encountering a Because of C++ CTAD rules, Clang's parser assigns this a placeholder The FixThe fix is implemented in
Changes
Notes on formattingWhen I Full diff: https://github.com/llvm/llvm-project/pull/174281.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 11323803e1910..50a6a4176d1ec 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14337,8 +14337,18 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
DeduceVariableDeclarationType(Var, false, nullptr))
return;
+ Type = Var->getType();
this->CheckAttributesOnDeducedType(RealDecl);
+ if (auto *Deduced = Type->getContainedDeducedType()) {
+ if (Var->isStaticDataMember() && Deduced->getDeducedType().isNull()) {
+ Diag(Var->getLocation(), diag::err_auto_var_requires_init)
+ << Var->getDeclName() << Type;
+ Var->setInvalidDecl();
+ return;
+ }
+ }
+
// C++11 [class.static.data]p3: A static data member can be declared with
// the constexpr specifier; if so, its declaration shall specify
// a brace-or-equal-initializer.
diff --git a/clang/test/SemaCXX/alias-template-static-member.cpp b/clang/test/SemaCXX/alias-template-static-member.cpp
new file mode 100644
index 0000000000000..0089aea33cc80
--- /dev/null
+++ b/clang/test/SemaCXX/alias-template-static-member.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+template <class T>
+struct A {
+ template <class U>
+ using E = U;
+
+ static E u; // expected-error {{declaration of variable 'u' with deduced type 'E' requires an initializer}}
+};
+
+decltype(A<int>::u) a;
\ No newline at end of file
|
|
Please add a release note entry to "clang/docs/ReleaseNotes.rst". We already diagnose correctly if we move |
|
Switched from checking As far as I understand:
So, should I leave it as |
| static E u; // expected-error {{declaration of variable 'u' with deduced type 'E' requires an initializer}} | ||
| }; | ||
|
|
||
| decltype(A<int>::u) a; No newline at end of file |
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.
Missing EOF - also, can we put that test in an existing file ? test/SemaCXX/alias-template.cpp would be a good candidate
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.
Ok, I added that!
zwuis
left a comment
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.
While we are here, could you handle the case that u has initializer?
So, should I leave it as
Type->isUndeducedType()and find another condition to force callingSema::DeduceVariableDeclarationType(so as to calldeduceVarTypeFromInitializerto hit the place where we handleE,U)
I'm not familiar enough with this part of code. Please wait for other reviewers.
| template <class U> | ||
| using E = U; | ||
|
|
||
| static E u; // expected-error {{declaration of variable 'u' with deduced type 'E' requires an initializer}} |
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.
It would be nice to emit same diagnostic messages as the case that E and u are out of the definition of A.
template <class U>
using E = U;
static E u;
It is already handled |
|
About the current diagnostic,
I believe the first one is fine. If not, could you please tell me, should I try to handle it inside SemaDecl/Sema::deduceVarTypeFromInitializer Or would it be inside SemaInit.cpp/Sema::DeduceTemplateSpecializationFromInitializer Also, about moving to a separate file, the file you mentioned uses I am very confused also with @zwuis comments, could you please explain a bit more? Or at least give me a little clue? |
We can refer to how it is handled. template <typename> struct A {
template <typename U>
using E = U;
// both of them should be rejected
static E u1;
static E u2 = 0;
};
We can write multiple |
zwuis
left a comment
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.
While we are here, could you handle the case that
uhas an initializer?It is already handled
Oh, I see. We already emit diagnostics for static E u2 = 0; during template instantiation. With this patch, we emit diagnostics for static E u; before template instantiation. It would be better to handle the former case before template instantiation.
| // RUN: %clang_cc1 -verify -std=c++14 -fcxx-exceptions %s | ||
| // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s | ||
|
|
||
| #if __cplusplus < 202002L |
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 we remove this #if?
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.
How then can I make some of the file be tested only with c++ 14, and my tests only run with c++ 20 ?
#173349
The Problem
Clang crashes when encountering a
staticdata member declared with an alias template but lacking template arguments (e.g.,static E u;).Because of C++ CTAD rules, Clang's parser assigns this a placeholder
AutoType. If this member is later referenced, the backend (CodeGen) attempts to emit the variable. Since the type remains undeduced and no initializer exists, the compiler hits anllvm_unreachableinCodeGenTypes.cpp.The Fix
The fix is implemented in
Sema::ActOnUninitializedDecl. We now explicitly check if astaticdata member still possesses an undeduced placeholder type after all deduction attempts have failed.err_auto_var_requires_init.Changes
ActOnUninitializedDeclto catch undeduced static members.Notes on formatting
When I
clang-formattheSemaDecl.cppafter applying my changes, I found the whole file got changed, so I undid the formatting.