[Clang] Fix the assertion condition after b8d1f3d6 #132669
Merged
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThanks to the example provided by @MagentaTreehouse, I realized the assertion I added didn't cover all valid cases like, when inheriting from a class template specialization, the source of a synthesized template parameter might be an implicit specialization, whose inner function template is thus living at depth 0, for which we don’t want it to overflow too. Full diff: https://github.com/llvm/llvm-project/pull/132669.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 9cfdb7596b660..e20130a33f368 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -376,12 +376,14 @@ struct ConvertConstructorToDeductionGuideTransform {
if (NestedPattern)
Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
auto [Depth, Index] = getDepthAndIndex(Param);
- // Depth can still be 0 if FTD belongs to an explicit class template
- // specialization with an empty template parameter list. In that case,
- // we don't want the NewDepth to overflow, and it should remain 0.
+ // Depth can be 0 if FTD belongs to a class template specialization with
+ // an empty template parameter list (i.e. either an explicit full
+ // specialization or an implicit specialization) In that case, we don't
+ // want the NewDepth to overflow, and it should remain 0.
assert(Depth ||
- cast<ClassTemplateSpecializationDecl>(FTD->getDeclContext())
- ->isExplicitSpecialization());
+ isa<ClassTemplateDecl *>(
+ cast<ClassTemplateSpecializationDecl>(FTD->getDeclContext())
+ ->getSpecializedTemplateOrPartial()));
NamedDecl *NewParam = transformTemplateParameter(
SemaRef, DC, Param, Args, Index + Depth1IndexAdjustment,
Depth ? Depth - 1 : 0);
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index ecd152abebd74..95e740a026550 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -723,3 +723,29 @@ void test() { NewDeleteAllocator abc(42); } // expected-error {{no viable constr
// CHECK-NEXT: `-ParmVarDecl {{.+}} 'T'
} // namespace GH128691
+
+namespace GH132616_DeductionGuide {
+
+template <class T> struct A {
+ template <class U>
+ A(U);
+};
+
+template <typename>
+struct B: A<int> {
+ using A::A;
+};
+
+template <class T>
+B(T) -> B<T>;
+
+B b(24);
+
+// CHECK-LABEL: Dumping GH132616_DeductionGuide::<deduction guide for B>:
+// CHECK-NEXT: FunctionTemplateDecl {{.+}} implicit <deduction guide for B>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} typename depth 0 index 0
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} class depth 0 index 1 U
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} implicit <deduction guide for B> 'auto (U) -> B<type-parameter-0-0>'
+// CHECK-NEXT: `-ParmVarDecl {{.+}} 'U'
+
+} // namespace GH132616_DeductionGuide
|
zyn0217
force-pushed
the
GH132616-deduction-guide-depth
branch
from
8562b2b to
49f2a7e
Compare
Thanks to the example provided by @MagentaTreehouse, I realized the assertion I added didn't cover all valid cases like, when inheriting from a class template specialization, the source of a synthesized template parameter might be an implicit specialization, whose inner function template is thus living at depth 0, for which we don’t want it to overflow too.
zyn0217
force-pushed
the
GH132616-deduction-guide-depth
branch
from
49f2a7e to
b7bef60
Compare
cor3ntin
approved these changes
Mar 24, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14818 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.
Thanks to the example provided by @MagentaTreehouse, I realized the assertion I added in b8d1f3d didn't cover all valid cases like, when inheriting from a class template specialization, the source of a synthesized template parameter might be an implicit specialization, whose inner function template is thus living at depth 0, for which we don’t want it to overflow too.
I've decided to remove that assertion because I don't think it's particularly useful: we're checking whether Depth = 0 parameters come from function templates whose parents contribute no template parameters to the depth, which is redundant given what the template depth already means.
This also incorporates a drive-by fix for #132061 (comment), which I somehow missed.