[clang] Propagate requires-clause from constructor template to implicit deduction guide

[nridge]

Summary

Fixes https://github.com/clangd/clangd/issues/890

[nridge]

Revise fix approach

[nridge]

Adding some potential reviewers.

I'm open to testing this some way other than through clangd, but note that, while the issue is not in AST serialization code, it only trips an assertion after a serialization round-trip.

[kadircet]

As for testing strategy, after this change the AST should look different. You can probably modify clang/test/SemaTemplate/deduction-guide.cpp to check for existence of requires clauses in the implicitly generated function templates for guides.

[nridge]

Now with clang test

[sammccall]

Fantastic, thank you!

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

794 ↗

(On Diff #416827)

Having solved and tested this in the AST, this test is arguably redundant (it's not like there's anything interesting going on in de/serialization).

Happy if you want to keep it as a regression test, maybe leave a comment that we used to crash here or linking to the bug?

clang/test/SemaTemplate/deduction-guide.cpp

219

Before your patch, clang requires this explicit deduction guide in order to accept the code.
Without it, it says the two implicit deduction guides from the constructor are ambiguous. But this is precisely because they lack the constraints!

After your patch, clang accepts this code without the explicit deduction guide.
So I think omitting the guide (and having no expected-error...) makes it a better test - in addition to verifying the guides can be dumped, it checks they are used.

(Kadir and I spent some time poking at variations of this example to understand how CTAD works in the AST, so thanks for this!)

[nridge]

Remove clangd test, remove explicit deduction guide from clang test

clang/lib/Sema/SemaTemplate.cpp

if (FTD) {
         Args.addOuterRetainedLevel();
         NamedDecl *NewParam = transformTemplateParameter(Param, Args);
         if (!NewParam)
           return nullptr;
         AllParams.push_back(NewParam);
         SubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
             SemaRef.Context.getInjectedTemplateArg(NewParam)));
       }
+
+      // Substitute new template parameters into requires-clause if present.
+      Expr *RequiresClause = nullptr;
+      if (Expr *InnerRC = InnerParams->getRequiresClause()) {
+        MultiLevelTemplateArgumentList Args;
+        Args.setKind(TemplateSubstitutionKind::Rewrite);
+        Args.addOuterTemplateArguments(SubstArgs);
+        Args.addOuterRetainedLevel();
+        ExprResult E = SemaRef.SubstExpr(InnerRC, Args);
+        if (E.isInvalid())
+          return nullptr;
+        RequiresClause = E.getAs<Expr>();
+      }
+
       TemplateParams = TemplateParameterList::Create(
           SemaRef.Context, InnerParams->getTemplateLoc(),
           InnerParams->getLAngleLoc(), AllParams, InnerParams->getRAngleLoc(),
-          /*FIXME: RequiresClause*/ nullptr);
+          RequiresClause);
     }
 
     // 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) {

clang/test/SemaTemplate/deduction-guide.cpp

 // CHECK:               |-NonTypeTemplateParmDecl {{.*}} referenced 'int' depth 1 index 0 ... M
 // CHECK:               `-TemplateArgument pack
 // CHECK:                 |-TemplateArgument expr
 // CHECK-NOT: Subst
 // CHECK:                 | `-DeclRefExpr {{.*}} 'int' NonTypeTemplateParm [[M1]] 'M1' 'int'
 // CHECK:                 `-TemplateArgument expr
 // CHECK-NOT: Subst
 // CHECK:                   `-DeclRefExpr {{.*}} 'int' NonTypeTemplateParm [[M2]] 'M2' 'int'
+
+template <char = 'x'> struct F;
+
+template <char> struct F {
+  template <typename U>
+  requires(false) F(U);
+  template <typename U>
+  requires(true) F(U);
+};
+
+F s(0);
+
+// CHECK-LABEL: Dumping <deduction guide for F>:
+// CHECK: FunctionTemplateDecl
+// CHECK: |-NonTypeTemplateParmDecl {{.*}} 'char' depth 0 index 0
+// CHECK:   `-TemplateArgument expr
+// CHECK: |   |-inherited from NonTypeTemplateParm {{.*}} '' 'char'
+// CHECK: |   `-ConstantExpr {{.*}} 'char'
+// CHECK: |     |-value: Int 120
+// CHECK: |     `-CharacterLiteral {{.*}} 'char' 120
+// CHECK: |-TemplateTypeParmDecl {{.*}} typename depth 0 index 1 U
+// CHECK: |-ParenExpr {{.*}} 'bool'
+// CHECK: | `-CXXBoolLiteralExpr {{.*}} 'bool' false
+// CHECK: |-CXXDeductionGuideDecl {{.*}} implicit <deduction guide for F> 'auto (type-parameter-0-1) -> F<>'
+// CHECK: | `-ParmVarDecl {{.*}} 'type-parameter-0-1'
+// CHECK: `-CXXDeductionGuideDecl {{.*}} implicit <deduction guide for F> 'auto (int) -> F<'x'>'
+// CHECK:   |-TemplateArgument integral 120
+// CHECK:   |-TemplateArgument type 'int'
+// CHECK:   | `-BuiltinType {{.*}} 'int'
+// CHECK:   `-ParmVarDecl {{.*}} 'int':'int'
+// CHECK: FunctionProtoType {{.*}} 'auto (type-parameter-0-1) -> F<>' dependent trailing_return cdecl
+// CHECK: |-InjectedClassNameType {{.*}} 'F<>' dependent
+// CHECK: | `-CXXRecord {{.*}} 'F'
+// CHECK: `-TemplateTypeParmType {{.*}} 'type-parameter-0-1' dependent depth 0 index 1