Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] CTAD: Fix require-clause is not transformed. #89378

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Apr 19, 2024

Fixes #89013

When building the deduction guide, we use the TemplateArgsForBuildingFPrime to transform the require-clause from the underlying class deduction guide. However, we do this at the wrong place where not all elements of TemplateArgsForBuildingFPrime are initialized. The fix involves rearranging the transformRequireClause call to the correct location.

As part of the fix, we extend the TemplateInstantiator to support more types in the template-rewrite mode. Otherwise, we will encounter an assertion error when attempting to rewrite the template type parameter type like D with a complex type like Derived.

Fixes llvm#89013

When building the deduction guide, we use the TemplateArgsForBuildingFPrime to
transform the require-clause from the underlying class deduction guide. However,
we do this at the wrong place where not all elements of TemplateArgsForBuildingFPrime
are initialized. The fix involves rearranging the transformRequireClause call to
the correct location.

As part of the fix, we extend the TemplateInstantiator to support more types in
the template-rewrite mode. Otherwise, we will encounter an assertion error when
attempting to rewrite the template type parameter type like D with a complex type
like Derived<U>.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Fixes #89013

When building the deduction guide, we use the TemplateArgsForBuildingFPrime to transform the require-clause from the underlying class deduction guide. However, we do this at the wrong place where not all elements of TemplateArgsForBuildingFPrime are initialized. The fix involves rearranging the transformRequireClause call to the correct location.

As part of the fix, we extend the TemplateInstantiator to support more types in the template-rewrite mode. Otherwise, we will encounter an assertion error when attempting to rewrite the template type parameter type like D with a complex type like Derived<U>.


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

4 Files Affected:

  • (modified) clang/lib/Sema/SemaTemplate.cpp (+14-13)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+1-4)
  • (modified) clang/test/SemaCXX/cxx20-ctad-type-alias.cpp (+18)
  • (modified) clang/test/SemaTemplate/deduction-guide.cpp (+28)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index d4976f9d0d11d8..4bda31ba67c02d 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2962,19 +2962,6 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
           Context.getCanonicalTemplateArgument(
               Context.getInjectedTemplateArg(NewParam));
     }
-    // Substitute new template parameters into requires-clause if present.
-    Expr *RequiresClause =
-        transformRequireClause(SemaRef, F, TemplateArgsForBuildingFPrime);
-    // FIXME: implement the is_deducible constraint per C++
-    // [over.match.class.deduct]p3.3:
-    //    ... and a constraint that is satisfied if and only if the arguments
-    //    of A are deducible (see below) from the return type.
-    auto *FPrimeTemplateParamList = TemplateParameterList::Create(
-        Context, AliasTemplate->getTemplateParameters()->getTemplateLoc(),
-        AliasTemplate->getTemplateParameters()->getLAngleLoc(),
-        FPrimeTemplateParams,
-        AliasTemplate->getTemplateParameters()->getRAngleLoc(),
-        /*RequiresClause=*/RequiresClause);
 
     // To form a deduction guide f' from f, we leverage clang's instantiation
     // mechanism, we construct a template argument list where the template
@@ -3020,6 +3007,20 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
             F, TemplateArgListForBuildingFPrime, AliasTemplate->getLocation(),
             Sema::CodeSynthesisContext::BuildingDeductionGuides)) {
       auto *GG = cast<CXXDeductionGuideDecl>(FPrime);
+      // Substitute new template parameters into requires-clause if present.
+      Expr *RequiresClause =
+          transformRequireClause(SemaRef, F, TemplateArgsForBuildingFPrime);
+      // FIXME: implement the is_deducible constraint per C++
+      // [over.match.class.deduct]p3.3:
+      //    ... and a constraint that is satisfied if and only if the arguments
+      //    of A are deducible (see below) from the return type.
+      auto *FPrimeTemplateParamList = TemplateParameterList::Create(
+          Context, AliasTemplate->getTemplateParameters()->getTemplateLoc(),
+          AliasTemplate->getTemplateParameters()->getLAngleLoc(),
+          FPrimeTemplateParams,
+          AliasTemplate->getTemplateParameters()->getRAngleLoc(),
+          /*RequiresClause=*/RequiresClause);
+
       buildDeductionGuide(SemaRef, AliasTemplate, FPrimeTemplateParamList,
                           GG->getCorrespondingConstructor(),
                           GG->getExplicitSpecifier(), GG->getTypeSourceInfo(),
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 7cd428de0bb32d..63894cc3cdd538 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2501,10 +2501,7 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB,
       assert(Arg.getKind() == TemplateArgument::Type &&
              "unexpected nontype template argument kind in template rewrite");
       QualType NewT = Arg.getAsType();
-      assert(isa<TemplateTypeParmType>(NewT) &&
-             "type parm not rewritten to type parm");
-      auto NewTL = TLB.push<TemplateTypeParmTypeLoc>(NewT);
-      NewTL.setNameLoc(TL.getNameLoc());
+      TLB.pushTrivial(SemaRef.Context, NewT, TL.getNameLoc());
       return NewT;
     }
 
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index 6f04264a655ad5..508a3a5da76a91 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -289,3 +289,21 @@ using String = Array<char, N>;
 // Verify no crash on constructing the aggregate deduction guides.
 String s("hello");
 } // namespace test21
+
+// GH89013
+namespace test22 {
+class Base {};
+template <typename T>
+class Derived final : public Base {};
+
+template <typename T, typename D>
+requires __is_base_of(Base, D)
+struct Foo {
+  explicit Foo(D) {}
+};
+
+template <typename U>
+using AFoo = Foo<int, Derived<U>>;
+
+AFoo a(Derived<int>{});
+} // namespace test22
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index 58f08aa1eed650..29cc5a9b996f95 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -260,3 +260,31 @@ AG ag = {1};
 // CHECK:   |-TemplateArgument type 'int'
 // CHECK:   | `-BuiltinType {{.*}} 'int'
 // CHECK:   `-ParmVarDecl {{.*}} 'int'
+
+template <typename D>
+requires (sizeof(D) == 4)
+struct Foo {
+  Foo(D);
+};
+
+template <typename U>
+using AFoo = Foo<G<U>>;
+// Verify that the require-clause from the Foo deduction guide is transformed.
+// The D occurrence should be rewritten to G<type-parameter-0-0>.
+//
+// CHECK-LABEL: Dumping <deduction guide for AFoo>
+// CHECK: FunctionTemplateDecl {{.*}} implicit <deduction guide for AFoo>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} typename depth 0 index 0 U
+// CHECK-NEXT: |-ParenExpr {{.*}} 'bool'
+// CHECK-NEXT: | `-BinaryOperator {{.*}} 'bool' '=='
+// CHECK-NEXT: |   |-UnaryExprOrTypeTraitExpr {{.*}} 'unsigned long' sizeof 'G<type-parameter-0-0>'
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} 'unsigned long' <IntegralCast>
+// CHECK-NEXT: |     `-IntegerLiteral {{.*}} 'int' 4
+// CHECK-NEXT: |-CXXDeductionGuideDecl {{.*}} implicit <deduction guide for AFoo> 'auto (G<type-parameter-0-0>) -> Foo<G<type-parameter-0-0>>'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} 'G<type-parameter-0-0>'
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.*}} implicit used <deduction guide for AFoo> 'auto (G<int>) -> Foo<G<int>>' implicit_instantiation
+// CHECK-NEXT:   |-TemplateArgument type 'int'
+// CHECK-NEXT:   | `-BuiltinType {{.*}} 'int'
+// CHECK-NEXT:   `-ParmVarDecl {{.*}} 'G<int>'
+
+AFoo aa(G<int>{});

@hokein hokein merged commit c8e65e1 into llvm:main Apr 19, 2024
6 of 7 checks passed
@hokein hokein deleted the ctad-transform-require-clause branch April 19, 2024 20:07
joker-eph added a commit that referenced this pull request Apr 20, 2024
@joker-eph
Copy link
Collaborator

This seems to have broken the bot: #89476 (you should have had an email?)

I reverted in #89476

aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 21, 2024
Fixes llvm#89013

When building the deduction guide, we use the
TemplateArgsForBuildingFPrime to transform the require-clause from the
underlying class deduction guide. However, we do this at the wrong place
where not all elements of TemplateArgsForBuildingFPrime are initialized.
The fix involves rearranging the transformRequireClause call to the
correct location.

As part of the fix, we extend the TemplateInstantiator to support more
types in the template-rewrite mode. Otherwise, we will encounter an
assertion error when attempting to rewrite the template type parameter
type like D with a complex type like Derived<U>.
aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 21, 2024
@hokein
Copy link
Collaborator Author

hokein commented Apr 22, 2024

This seems to have broken the bot: #89476 (you should have had an email?)

I reverted in #89476

sorry, and thanks for the revert. I will take a further look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion !isValueDependent() && "Expression evaluator can't be called on a dependent expression."' failed.
4 participants