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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

<details>
<summary>Changes</summary>

This fixes a regression introduced by bee78b88f.

When we form a deduction guide for a constructor, basically, we do the following work:
- Collect template parameters from the constructor's surrounding class template, if present.
- Collect template parameters from the constructor.
- Splice these template parameters together into a new template parameter list.
- Turn all the references (e.g. the function parameter list) to the invented parameter list by applying a `TreeTransform` to the function type.

In the previous fix, we handled cases of nested class templates by substituting the "outer" template parameters (i.e. those not declared at the surrounding class template or the constructor) with the instantiating template arguments. The approach per se makes sense, but there was a flaw in the following case:

```cpp
template <typename U, typename... Us> struct X {
  template <typename V> struct Y {
    template <typename T> Y(T) {}
  };

  template <typename T> Y(T) -> Y<T>;
};

X<int>::Y y(42);
```

While we're transforming the parameters for `Y(T)`, we first attempt to transform all references to `V` and `T`; then, we handle the references to outer parameters `U` and `Us` using the template arguments from `X<int>` by transforming the same `ParamDecl`. However, the first step results in the reference `T` being `<template-param-0-1>` because the invented `T` is the last of the parameter list of the deduction guide, and what we're substituting with is a corresponding parameter pack (which is `Us`, though empty). Hence we're messing up the substitution.

I think we can resolve it by reversing the substitution order, which means handling outer template parameters first and then the inner parameters.

There's no release note because this is a regression in 18, and I hope we can catch up with the last release.

Fixes https://github.com/llvm/llvm-project/issues/88142

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaTemplate.cpp (+19-6) 
- (modified) clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp (+14) 


``````````diff
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 7e57fa0696725..63c16b3fbc279 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2491,9 +2491,6 @@ struct ConvertConstructorToDeductionGuideTransform {
       Args.addOuterRetainedLevel();
     }
 
-    if (NestedPattern)
-      Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
-
     FunctionProtoTypeLoc FPTL = CD->getTypeSourceInfo()->getTypeLoc()
                                    .getAsAdjusted<FunctionProtoTypeLoc>();
     assert(FPTL && "no prototype for constructor declaration");
@@ -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);
       if (!NewParam)
         return QualType();
       ParamTypes.push_back(NewParam->getType());
diff --git a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
index 38b6706595a11..5428a4eab01cf 100644
--- a/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
+++ b/clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp
@@ -84,3 +84,17 @@ nested_init_list<int>::concept_fail nil_invalid{1, ""};
 // expected-note@#INIT_LIST_INNER_INVALID {{candidate template ignored: substitution failure [with F = const char *]: constraints not satisfied for class template 'concept_fail' [with F = const char *]}}
 // expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not viable: requires 1 argument, but 2 were provided}}
 // expected-note@#INIT_LIST_INNER_INVALID {{candidate function template not viable: requires 0 arguments, but 2 were provided}}
+
+namespace PR88142 {
+
+template <typename, typename...> struct X {
+  template <typename> struct Y {
+    template <typename T> Y(T) {}
+  };
+
+  template <typename T> Y(T) -> Y<T>;
+};
+
+X<int>::Y y(42);
+
+} // namespace PR88142

``````````

</details>


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