[clang] [Clang][Sema] Revise the transformation of CTAD parameters of nested class templates (PR #91628)

[Younan Zhang via cfe-commits]

================
@@ -2583,11 +2580,27 @@ struct ConvertConstructorToDeductionGuideTransform {
 
     //    -- The types of the function parameters are those of the constructor.
     for (auto *OldParam : TL.getParams()) {
-      ParmVarDecl *NewParam =
-          transformFunctionTypeParam(OldParam, Args, MaterializedTypedefs);
-      if (NestedPattern && NewParam)
+      ParmVarDecl *NewParam = OldParam;
+      // Given
+      //   template <class T> struct C {
+      //     template <class U> struct D {
+      //       template <class V> D(U, V);
+      //     };
+      //   };
+      // First, transform all the references to template parameters that are
+      // defined outside of the surrounding class template. That is T in the
+      // above example.
+      if (NestedPattern) {
         NewParam = transformFunctionTypeParam(NewParam, OuterInstantiationArgs,
                                               MaterializedTypedefs);
+        if (!NewParam)
+          return QualType();
+      }
+      // Then, transform all the references to template parameters that are
+      // defined at the class template and the constructor. In this example,
+      // they're U and V, respectively.
+      NewParam =
+          transformFunctionTypeParam(NewParam, Args, MaterializedTypedefs);
----------------
zyn0217 wrote:

I think that's just fine, although it looks weird: those lines are merely "creating" new template parameters rather than doing type substitution. (it's applying a `TemplateDeclInstantiator` with the instantiating arguments - the only valuable part from these arguments is the depth we're supposed to deduct, and the type isn't something we're concerned about at that time.)

Frankly, I have to say I didn't fully understand why we have such discrepancies (use the `transformTemplateParameter` first and then a `TemplateDeclInstantiator` for nested cases). I was expecting that perhaps we could unify them in one call. I think it warrants a separate refactoring PR, then.

https://github.com/llvm/llvm-project/pull/91628