[Clang] Fix missed initializer instantiation bug for variable templates #138122
+30
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: 执着 (dty2) ChangesTry to fix missing initializer for inline static template member with auto caused by delayed template instantiation. Bug Fix: Bug 135032 Reproduction Steps: template <typename T> struct B {
template <typename G> inline static auto var = 5;
};
int main() {
int bb = B<int>::var<int>;
return 0;
}GDB Breakpoints for Debugging: #(1)
b Sema::BuildTemplateIdExpr
#(2)
b Sema::CheckVarTemplateId
#(3)
b Sema::CheckVarTemplateId
#(4)
b Sema::BuildVarTemplateInstantiation
#(5)
b TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl
#(6)
b VarTemplateSpecializationDecl::Create
#(7)
b VarTemplateSpecializationDecl::VarTemplateSpecializationDecl
#(8)
b VarDecl::VarDecl
#(6)
b Sema::BuildVariableInstantiation
#(7)
b Sema::InstantiateVariableInitializer
#(3)
b Sema::BuildDeclarationNameExpr
#(4)
b Sema::BuildDeclRefExprKey Observations: The AST shows VarTemplateSpecializationDecl as int (correct), but GDB reveals its type is actually auto during execution. Example from AST: DeclRefExpr 0x598f651f5a60 <col:9, col:27> 'auto' lvalue VarTemplateSpecialization 0x598f651f5980 'var' 'int' (VarTemplate 0x598f651d1938 'var') Solution: Full diff: https://github.com/llvm/llvm-project/pull/138122.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 76c055d28f091..eb66ec34a956f 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6027,8 +6027,14 @@ void Sema::BuildVariableInstantiation(
Context.setManglingNumber(NewVar, Context.getManglingNumber(OldVar));
Context.setStaticLocalNumber(NewVar, Context.getStaticLocalNumber(OldVar));
+ bool VarTemplateWithAutoType = false;
+ QualType VarSourceType = OldVar->getTypeSourceInfo()->getType();
+ if (VarSourceType->getAs<AutoType>()) {
+ VarTemplateWithAutoType = true;
+ }
+
// Figure out whether to eagerly instantiate the initializer.
- if (InstantiatingVarTemplate || InstantiatingVarTemplatePartialSpec) {
+ if ((InstantiatingVarTemplate && !VarTemplateWithAutoType) || InstantiatingVarTemplatePartialSpec) {
// We're producing a template. Don't instantiate the initializer yet.
} else if (NewVar->getType()->isUndeducedType()) {
// We need the type to complete the declaration of the variable.
diff --git a/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp b/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
index 812e438f30c9a..b1d8e376b826f 100644
--- a/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
+++ b/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
@@ -1,5 +1,13 @@
// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s
+template <typename T> struct B {
+ template <typename G> inline static auto var = 5;
+};
+
+int inlinevartemplate = B<int>::var<int>;
+// CHECK: @_ZN1BIiE3varIiEE = {{.*}}global i32 5{{.*}}comdat
+// CHECK-NOT: @_ZN1BIiE3varIfEE
+
struct Q {
// CHECK: @_ZN1Q1kE = linkonce_odr constant i32 5, comdat
static constexpr int k = 5;
diff --git a/clang/test/SemaTemplate/cxx17-inline-variables.cpp b/clang/test/SemaTemplate/cxx17-inline-variables.cpp
index 7fc0aa8eeeb0c..27067c8e1b5e4 100644
--- a/clang/test/SemaTemplate/cxx17-inline-variables.cpp
+++ b/clang/test/SemaTemplate/cxx17-inline-variables.cpp
@@ -27,3 +27,9 @@ template <typename T> constexpr int A<T>::n = sizeof(A) + sizeof(T);
template <typename T> inline constexpr int A<T>::m = sizeof(A) + sizeof(T);
static_assert(A<int>().f() == 5);
static_assert(A<int>().g() == 5);
+
+template <typename T> struct B {
+ template <typename G> inline static auto var = 5;
+};
+
+int b = B<int>::var<int>;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
zyn0217
reviewed
May 1, 2025
Comment on lines
6030
to
6032
| bool VarTemplateWithAutoType = false; | ||
| QualType VarSourceType = OldVar->getTypeSourceInfo()->getType(); | ||
| if (VarSourceType->getAs<AutoType>()) { | ||
| VarTemplateWithAutoType = true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Suggested change
| bool VarTemplateWithAutoType = false; | |
| QualType VarSourceType = OldVar->getTypeSourceInfo()->getType(); | |
| if (VarSourceType->getAs<AutoType>()) { | |
| VarTemplateWithAutoType = true; | |
| } | |
| bool VarTemplateWithAutoType = OldVar->getTypeSourceInfo()->getType()->getAs<AutoType>(); |
Can we avoid getTypeSourceInfo?
There was a problem hiding this comment.
Your suggestion is correct, I will adopt it.
|
Also, can you help me test if this fixes #134526? Thanks |
sure, no problem. |
|
@zyn0217 #include <iostream>
template <class T>
struct S {
template <class U> static const auto var = T();
template <class U> static const auto foo = var<T>;
};
int main() {
const int p = S<int>::var<int> + S<int>::foo<int> + 1;
std::cout << p;
return 0;
} |
zwuis
reviewed
May 1, 2025
| @@ -6027,8 +6027,15 @@ void Sema::BuildVariableInstantiation( | ||
| Context.setManglingNumber(NewVar, Context.getManglingNumber(OldVar)); | ||
| Context.setStaticLocalNumber(NewVar, Context.getStaticLocalNumber(OldVar)); | ||
|
|
||
| bool VarTemplateWithAutoType = false; | ||
| QualType VarSourceType = OldVar->getTypeSourceInfo()->getType(); | ||
| if (VarSourceType->getAs<AutoType>()) { |
There was a problem hiding this comment.
Can we use ->isUndeducedType() to handle this case
template <typename T> struct S { S(T); };
template <typename T> struct B {
template <typename G> inline static S var = 5;
};?
Update: And we need to change OldVar-> to NewVar->.
There was a problem hiding this comment.
This test case makes me thinking that we should swap that two if statements. @dty2
There was a problem hiding this comment.
Oh my goodness, I realize my mistake, thank you for your patience in correcting me. You are right.
zwuis
reviewed
May 2, 2025
Comment on lines
6037
to
6032
| if ((InstantiatingVarTemplate && !VarTemplateWithAutoType) || | ||
| InstantiatingVarTemplatePartialSpec) { | ||
| // We're producing a template. Don't instantiate the initializer yet. | ||
| } else if (NewVar->getType()->isUndeducedType()) { | ||
| // We need the type to complete the declaration of the variable. |
There was a problem hiding this comment.
It seems that we can just swap these two if statements. We need to handle partial specializations as well. E.g.
template <typename> struct B {
template <typename, typrname> inline static auto var = 6;
template <typename T> inline static auto var<int, T> = 7;
};
int b = B<int>::var<int, int>;There was a problem hiding this comment.
Thank you for your review. I think the two if statements should not be swapped, because clang adopts the strategy of delaying template instantiation. If the two if statements are swapped, it will completely violate this strategy. Therefore, I do not plan to swap the two if statements, but only pre-instantiate the template variables of auto type. Of course, if there are other reasons or my understanding is wrong, please let me know and I will consider it again. In addition, thank you again for reminding me to handle some specialized templates, so I modified the if statement to handle this situation.
There was a problem hiding this comment.
I don't understand what "strategy" is going to be violated. So you have an example?
c41f6ff to
75e6e3c
Compare
mizvekov
reviewed
May 9, 2025
Comment on lines
6030
to
6035
| bool VarTemplateWithAutoType = | ||
| OldVar->getTypeSourceInfo()->getType()->getAs<AutoType>(); | ||
|
|
||
| // Figure out whether to eagerly instantiate the initializer. | ||
| if (InstantiatingVarTemplate || InstantiatingVarTemplatePartialSpec) { | ||
| if (!VarTemplateWithAutoType && | ||
| (InstantiatingVarTemplate || InstantiatingVarTemplatePartialSpec)) { |
There was a problem hiding this comment.
This is special casing AutoType, but this should have the same problem for any DeducedType, like deduced template specializations as well, right?
Wouldn't just swapping the first two branches of the if-else chain here work?
There was a problem hiding this comment.
Oh, I realized my mistake, thank you for correcting me.
zyn0217
reviewed
May 14, 2025
| template <typename T> struct InlineAuto { | ||
| template <typename G> inline static auto var = 5; | ||
| }; | ||
| int inlineauot = InlineAuto<int>::var<int>; |
zyn0217
reviewed
May 14, 2025
| template <typename T> inline static auto var<int, T> = 7; | ||
| }; | ||
|
|
||
| int partialinlineauot = PartialInlineAuto<int>::var<int, int>; |
zyn0217
reviewed
May 14, 2025
Comment on lines
40
to
41
| int inlineauot = InlineAuto<int>::var<int>; | ||
| int partialinlineauot = PartialInlineAuto<int>::var<int, int>; |
30b3516 to
673a13b
Compare
[Clang] Fix missed initializer instantiation bug for variable templates [Clang] Fix missed initializer instantiation bug for variable templates
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/17946 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/202/builds/1299 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Try to fix missing initializer for inline static template member with auto caused by delayed template instantiation.
Bug Fix: Bug 135032
Problem Description:
Due to nested templates, when instantiating the outer layer (the template class), the inner layer (the template variable) uses delayed instantiation.
This causes the declaration (VarDecl) of the template variable to retain the type from the original template declaration (i.e., auto), and it loses the initializer.
Later, when instantiating the template variable, its VarTemplateSpecializationDecl type depends on the VarDecl type.
Thus, the VarTemplateSpecializationDecl also has no initializer, and its type remains auto.
Ultimately, when building the reference expression in Sema::BuildDeclarationNameExpr, the expression's type is auto and stays as auto until code generation, triggering llvm_unreachable in CodeGenTypes::ConvertType.
Reproduction Steps:
Test Code:
GDB Breakpoints for Debugging:
Annotations like #(1) mark the call order in a tree structure. For example, two #(3) entries mean Sema::CheckVarTemplateId calls Sema::BuildDeclarationNameExpr after its own execution.
Key Observations:
While debugging, I generated the AST for the test code and noticed a contradiction:
The AST shows VarTemplateSpecializationDecl as int (correct), but GDB reveals its type is actually auto during execution.
Example from AST:
DeclRefExpr 0x598f651f5a60 <col:9, col:27> 'auto' lvalue VarTemplateSpecialization 0x598f651f5980 'var' 'int' (VarTemplate 0x598f651d1938 'var')
The DeclRefExpr has type auto, while its referenced VarTemplateSpecialization claims to be int—a clear inconsistency.
Solution:
Since I noticed that the deduction of auto type is caused by the initializer
I plan to do special processing for template variables of type auto, that is, to prevent their delayed instantiation
so that their initializers will not be lost when the outer template class is instantiated