Skip to content
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 CTAD for aggregates for nested template classes #78387

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

antangelo
Copy link
Contributor

Use the template pattern in determining whether to synthesize the aggregate deduction guide, and update DeclareImplicitDeductionGuideFromInitList to substitute outer template arguments.

Fixes #77599

Use the template pattern in determining whether to synthesize the
aggregate deduction guide, and update DeclareImplicitDeductionGuideFromInitList
to substitute outer template arguments.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 17, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 17, 2024

@llvm/pr-subscribers-clang

Author: None (antangelo)

Changes

Use the template pattern in determining whether to synthesize the aggregate deduction guide, and update DeclareImplicitDeductionGuideFromInitList to substitute outer template arguments.

Fixes #77599


Full diff: https://github.com/llvm/llvm-project/pull/78387.diff

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaInit.cpp (+8-1)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+17-4)
  • (modified) clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp (+18-1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1eba8ab5590c52..db589beb46aadb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -928,6 +928,9 @@ Bug Fixes to C++ Support
   (`#57410 <https://github.com/llvm/llvm-project/issues/57410>`_) and
   (`#76604 <https://github.com/llvm/llvm-project/issues/57410>`_)
 
+- Fixes CTAD for aggregates on nested template classes. Fixes:
+  (`#77599 <https://github.com/llvm/llvm-project/issues/77599>`_)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 408ee5f775804b..48235941f62aa2 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -10718,7 +10718,14 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
     bool HasAnyDeductionGuide = false;
 
     auto SynthesizeAggrGuide = [&](InitListExpr *ListInit) {
-      auto *RD = cast<CXXRecordDecl>(Template->getTemplatedDecl());
+      auto *Pattern = Template;
+      while (Pattern->getInstantiatedFromMemberTemplate()) {
+        if (Pattern->isMemberSpecialization())
+          break;
+        Pattern = Pattern->getInstantiatedFromMemberTemplate();
+      }
+
+      auto *RD = cast<CXXRecordDecl>(Pattern->getTemplatedDecl());
       if (!(RD->getDefinition() && RD->isAggregate()))
         return;
       QualType Ty = Context.getRecordType(RD);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 80a48c268a648b..8c2199c36f0fe3 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2418,6 +2418,9 @@ struct ConvertConstructorToDeductionGuideTransform {
     QualType Result = SemaRef.BuildFunctionType(DeducedType, ParamTypes, Loc,
                                                 DeductionGuideName, EPI);
     TypeSourceInfo *TSI = SemaRef.Context.getTrivialTypeSourceInfo(Result, Loc);
+    if (NestedPattern)
+      TSI = SemaRef.SubstType(TSI, OuterInstantiationArgs, Loc,
+                              DeductionGuideName);
 
     FunctionProtoTypeLoc FPTL =
         TSI->getTypeLoc().castAs<FunctionProtoTypeLoc>();
@@ -2425,9 +2428,13 @@ struct ConvertConstructorToDeductionGuideTransform {
     // Build the parameters, needed during deduction / substitution.
     SmallVector<ParmVarDecl*, 4> Params;
     for (auto T : ParamTypes) {
-      ParmVarDecl *NewParam = ParmVarDecl::Create(
-          SemaRef.Context, DC, Loc, Loc, nullptr, T,
-          SemaRef.Context.getTrivialTypeSourceInfo(T, Loc), SC_None, nullptr);
+      auto *TSI = SemaRef.Context.getTrivialTypeSourceInfo(T, Loc);
+      if (NestedPattern)
+        TSI = SemaRef.SubstType(TSI, OuterInstantiationArgs, Loc,
+                                DeclarationName());
+      ParmVarDecl *NewParam =
+          ParmVarDecl::Create(SemaRef.Context, DC, Loc, Loc, nullptr,
+                              TSI->getType(), TSI, SC_None, nullptr);
       NewParam->setScopeInfo(0, Params.size());
       FPTL.setParam(Params.size(), NewParam);
       Params.push_back(NewParam);
@@ -2670,8 +2677,14 @@ FunctionTemplateDecl *Sema::DeclareImplicitDeductionGuideFromInitList(
   if (BuildingDeductionGuides.isInvalid())
     return nullptr;
 
-  return cast<FunctionTemplateDecl>(
+  ClassTemplateDecl *Pattern =
+      Transform.NestedPattern ? Transform.NestedPattern : Transform.Template;
+  ContextRAII SavedContext(*this, Pattern->getTemplatedDecl());
+
+  auto *DG = cast<FunctionTemplateDecl>(
       Transform.buildSimpleDeductionGuide(ParamTypes));
+  SavedContext.pop();
+  return DG;
 }
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index c44ec6918c7afb..f3af6e8d6c17da 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++20 -verify %s
-// expected-no-diagnostics
 
 template<class T> struct S {
     template<class U> struct N {
@@ -58,3 +57,21 @@ template<class X> struct requires_clause {
 requires_clause<int>::B req(1, 2);
 using RC = decltype(req);
 using RC = requires_clause<int>::B<int>;
+
+template<typename X> struct nested_init_list {
+    template<C<X> Y>
+    struct B { // #INIT_LIST_INNER
+        X x;
+        Y y;
+    };
+};
+
+nested_init_list<int>::B nil {1, 2};
+using NIL = decltype(nil);
+using NIL = nested_init_list<int>::B<int>;
+
+// expected-error@+1 {{no viable constructor or deduction guide for deduction of template arguments of 'B'}}
+nested_init_list<int>::B nil_invalid {1, ""};
+// expected-note@#INIT_LIST_INNER {{candidate template ignored: substitution failure [with Y = const char *]: constraints not satisfied for class template 'B' [with Y = const char *]}}
+// expected-note@#INIT_LIST_INNER {{candidate function template not viable: requires 1 argument, but 2 were provided}}
+// expected-note@#INIT_LIST_INNER {{candidate function template not viable: requires 0 arguments, but 2 were provided}}

@cor3ntin
Copy link
Contributor

@erichkeane it looks reasonable to me. WDYT?

@hokein
Copy link
Collaborator

hokein commented Jan 17, 2024

Thanks, this change looks good to me.

@antangelo antangelo merged commit 04a69a1 into llvm:main Jan 17, 2024
7 checks passed
@antangelo antangelo deleted the nested-ctad-aggregate-init branch January 17, 2024 23:03
antangelo added a commit that referenced this pull request Jan 18, 2024
…78541)

Reverts #78387

The added tests are failing on several build bots.
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
Use the template pattern in determining whether to synthesize the
aggregate deduction guide, and update
DeclareImplicitDeductionGuideFromInitList to substitute outer template
arguments.

Fixes llvm#77599
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
antangelo added a commit that referenced this pull request Jan 20, 2024
…78670)

Reland of #78387

Use the template pattern in determining whether to synthesize the
aggregate deduction guide, and update
DeclareImplicitDeductionGuideFromInitList to substitute outer template
arguments.

The tests in the original patch made an assumption about the size of a
pointer type, and this led to them failing on targets with 32-bit
pointers. The tests have been updated to not depend on the size of any
type. This only requires updates to the test file, no functionality has
otherwise changed between this and the original patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CTAD for aggregates fails for inner template class
5 participants