[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 &lt;typename U, typename... Us&gt; struct X {
  template &lt;typename V&gt; struct Y {
    template &lt;typename T&gt; Y(T) {}
  };

  template &lt;typename T&gt; Y(T) -&gt; Y&lt;T&gt;;
};

X&lt;int&gt;::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&lt;int&gt;` 
by transforming the same `ParamDecl`. However, the first step results in the 
reference `T` being `&lt;template-param-0-1&gt;` 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits