Clone constructor template parameters when creating deduction guide

[rsmith]

clang/lib/Sema/SemaTemplate.cpp

2174

This outer retained level would correspond to the parameters you're building -- we shouldn't add this. Instead, we should have two different SubstArgs lists, one for the outer parameters and one for the inner parameters; we should add just the outer parameters here, and add the inner and outer parameters below. (And at the end of each iteration of this loop we should accumulate arguments onto the outer arguments list like we accumulate arguments onto the inner arguments list in the next loop.)

[aeubanks]

update

[aeubanks]

clang/lib/Sema/SemaTemplate.cpp

2174

I think this is what you're referring to? I still don't understand exactly what Args.addOuterRetainedLevel() does, and the current draft doesn't fix the modules crash.

[rsmith]

clang/lib/Sema/SemaTemplate.cpp

2174

I found a few more places that need to be adjusted. Here's where I got to: https://github.com/zygoloid/llvm-project/commit/fffb4b688eb5a91bc2aca673773bc2da018f3dba (this is on top of the earlier version of this patch, though).

There are still some test failures there, so I think some more updates are still needed. We also need to do the same cloning process in buildSimpleDeductionGuide, which I think is why the modules unit test is still failing.

clang/lib/Sema/SemaTemplate.cpp

struct ConvertConstructorToDeductionGuideTransform {
 
   // Index adjustment to apply to convert depth-1 template parameters into
   // depth-0 template parameters.
   unsigned Depth1IndexAdjustment = Template->getTemplateParameters()->size();
 
   /// Transform a constructor declaration into a deduction guide.
   NamedDecl *transformConstructor(FunctionTemplateDecl *FTD,
                                   CXXConstructorDecl *CD) {
-    SmallVector<TemplateArgument, 16> SubstArgs;
+    SmallVector<TemplateArgument, 16> OuterSubstArgs;
+    SmallVector<TemplateArgument, 16> InnerSubstArgs;
 
     LocalInstantiationScope Scope(SemaRef);
 
     // C++ [over.match.class.deduct]p1:
     // -- For each constructor of the class template designated by the
     //    template-name, a function template with the following properties:
 
     //    -- The template parameters are the template parameters of the class
     //       template followed by the template parameters (including default
     //       template arguments) of the constructor, if any.
     TemplateParameterList *TemplateParams = Template->getTemplateParameters();
-    if (FTD) {
-      TemplateParameterList *InnerParams = FTD->getTemplateParameters();
-      SmallVector<NamedDecl *, 16> AllParams;
-      AllParams.reserve(TemplateParams->size() + InnerParams->size());
-      AllParams.insert(AllParams.begin(),
-                       TemplateParams->begin(), TemplateParams->end());
-      SubstArgs.reserve(InnerParams->size());
+    TemplateParameterList *InnerParams =
+        FTD ? FTD->getTemplateParameters() : nullptr;
+    AllParams.reserve(TemplateParams->size() +
+                      (InnerParams ? InnerParams->size() : 0));
+    OuterSubstArgs.reserve(TemplateParams->size());
+    for (NamedDecl *Param : *TemplateParams) {
+      MultiLevelTemplateArgumentList Args;
+      Args.setKind(TemplateSubstitutionKind::Rewrite);
+      Args.addOuterTemplateArguments(OuterSubstArgs);
+      NamedDecl *NewParam = transformTemplateParameter(Param, Args);
+      if (!NewParam)
+        return nullptr;
+      AllParams.push_back(NewParam);
+      OuterSubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
+          SemaRef.Context.getInjectedTemplateArg(NewParam)));
+    }
+    if (InnerParams) {
+      InnerSubstArgs.reserve(InnerParams->size());
 
       // Later template parameters could refer to earlier ones, so build up
       // a list of substituted template arguments as we go.
       for (NamedDecl *Param : *InnerParams) {
         MultiLevelTemplateArgumentList Args;
         Args.setKind(TemplateSubstitutionKind::Rewrite);
-        Args.addOuterTemplateArguments(SubstArgs);
+        Args.addOuterTemplateArguments(InnerSubstArgs);
+        Args.addOuterTemplateArguments(OuterSubstArgs);
         Args.addOuterRetainedLevel();
         NamedDecl *NewParam = transformTemplateParameter(Param, Args);
         if (!NewParam)
           return nullptr;
         AllParams.push_back(NewParam);
-        SubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
+        InnerSubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
             SemaRef.Context.getInjectedTemplateArg(NewParam)));
       }
-      TemplateParams = TemplateParameterList::Create(
-          SemaRef.Context, InnerParams->getTemplateLoc(),
-          InnerParams->getLAngleLoc(), AllParams, InnerParams->getRAngleLoc(),
-          /*FIXME: RequiresClause*/ nullptr);
-    }
+    }
+    TemplateParameterList *InheritParameters =
+        InnerParams ? InnerParams : TemplateParams;
+        SemaRef.Context, InheritParameters->getTemplateLoc(),
+        InheritParameters->getLAngleLoc(), AllParams,
+        InheritParameters->getRAngleLoc(),
 
     // If we built a new template-parameter-list, track that we need to
     // substitute references to the old parameters into references to the
     // new ones.
     MultiLevelTemplateArgumentList Args;
     Args.setKind(TemplateSubstitutionKind::Rewrite);
-    if (FTD) {
-      Args.addOuterTemplateArguments(SubstArgs);
-      Args.addOuterRetainedLevel();
-    }
+    Args.addOuterTemplateArguments(InnerSubstArgs);
+    Args.addOuterTemplateArguments(OuterSubstArgs);
 
     FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
                                    .getAsAdjusted<FunctionProtoTypeLoc>();
     assert(FPTL && "no prototype for constructor declaration");
 
     // Transform the type of the function, adjusting the return type and
     // replacing references to the old parameters with references to the
     // new ones.

clang/test/Modules/Inputs/ctad/a.h

+#pragma once
+
+template <typename T = int>
+struct A {
+  template <typename U = float>
+  A(T, U) {}
+};
+
+template <typename T = int>
+struct B {
+  B(T) {}
+};
+
+A _a(2, 2);
+B _b(2);

clang/test/Modules/Inputs/ctad/b.h

+#pragma once
+
+template<typename T=int> struct A {
+  template <typename U = float>
+  A(T, U) {}
+};
+
+template <typename T = int>
+struct B {
+  B(T) {}
+};
+
+A _x(5, 1.2);
+B _y(5);

clang/test/Modules/Inputs/ctad/module.modulemap

+module a { header "a.h" export * }
+module b { header "b.h" export * }

clang/test/Modules/ctad.cpp

+// RUN: rm -rf %t
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fno-implicit-modules -x c++ -emit-module %S/Inputs/ctad/module.modulemap -fmodule-name=a -o %t/a.pcm -std=c++17
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fno-implicit-modules -x c++ -emit-module %S/Inputs/ctad/module.modulemap -fmodule-name=b -o %t/b.pcm -std=c++17
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fno-implicit-modules -x c++ -emit-llvm -I%S/Inputs/ctad -o - %s -fmodule-file=%t/a.pcm -fmodule-file=%t/b.pcm -std=c++17 | FileCheck %s
+
+// CHECK: @_a = global
+// CHECK: @_b = global
+// CHECK: @_x = global
+// CHECK: @_y = global
+// CHECK: call void @__cxx_global_var_init()
+// CHECK: call void @__cxx_global_var_init.1()
+
+#include "a.h"
+#include "b.h"