[clang] CTAD alias: Respect explicit deduction guides defined after the first use of the alias template. #125478

hokein · 2025-02-03T11:02:42Z

This is the last missing piece for the C++20 CTAD alias feature. No release note being added in this PR yet, I will send out a follow-up patch to mark this feature done.

(Since the release 20 branch is cut, I think we should target on clang21).

llvmbot · 2025-02-03T11:04:33Z

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Fixes #103016

This is the last missing piece for the C++20 CTAD alias feature. No release note being added in this PR yet, I will send out a follow-up patch to mark this feature done.

(Since the release 20 branch is cut, I think we should target on clang21).

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

2 Files Affected:

(modified) clang/lib/Sema/SemaTemplateDeductionGuide.cpp (+25-8)
(modified) clang/test/SemaCXX/cxx20-ctad-type-alias.cpp (+11-2)

diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 0d079677eecc568..f5884f964d6fbd5 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -740,6 +740,24 @@ bool hasDeclaredDeductionGuides(DeclarationName Name, DeclContext *DC) {
   return false;
 }
 
+// Returns all source deduction guides associated with the declared
+// deduction guides that have the specified deduction guide name.
+llvm::DenseSet<const NamedDecl *> getSourceDeductionGuides(DeclarationName Name,
+                                                           DeclContext *DC) {
+  assert(Name.getNameKind() ==
+             DeclarationName::NameKind::CXXDeductionGuideName &&
+         "name must be a deduction guide name");
+  llvm::DenseSet<const NamedDecl *> Result;
+  for (auto *D : DC->lookup(Name)) {
+    if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+      D = FTD->getTemplatedDecl();
+
+    if (const auto *GD = dyn_cast<CXXDeductionGuideDecl>(D))
+      Result.insert(GD->getSourceDeductionGuide());
+  }
+  return Result;
+}
+
 // Build the associated constraints for the alias deduction guides.
 // C++ [over.match.class.deduct]p3.3:
 //   The associated constraints ([temp.constr.decl]) are the conjunction of the
@@ -1191,13 +1209,10 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
   if (AliasTemplate->isInvalidDecl())
     return;
   auto &Context = SemaRef.Context;
-  // FIXME: if there is an explicit deduction guide after the first use of the
-  // type alias usage, we will not cover this explicit deduction guide. fix this
-  // case.
-  if (hasDeclaredDeductionGuides(
-          Context.DeclarationNames.getCXXDeductionGuideName(AliasTemplate),
-          AliasTemplate->getDeclContext()))
-    return;
+  auto SourceDeductionGuides = getSourceDeductionGuides(
+      Context.DeclarationNames.getCXXDeductionGuideName(AliasTemplate),
+      AliasTemplate->getDeclContext());
+
   auto [Template, AliasRhsTemplateArgs] =
       getRHSTemplateDeclAndArgs(SemaRef, AliasTemplate);
   if (!Template)
@@ -1210,6 +1225,8 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
 
   for (auto *G : Guides) {
     if (auto *DG = dyn_cast<CXXDeductionGuideDecl>(G)) {
+      if (SourceDeductionGuides.contains(DG))
+        continue;
       // The deduction guide is a non-template function decl, we just clone it.
       auto *FunctionType =
           SemaRef.Context.getTrivialTypeSourceInfo(DG->getType());
@@ -1252,7 +1269,7 @@ void DeclareImplicitDeductionGuidesForTypeAlias(
       continue;
     }
     FunctionTemplateDecl *F = dyn_cast<FunctionTemplateDecl>(G);
-    if (!F)
+    if (!F || SourceDeductionGuides.contains(F->getTemplatedDecl()))
       continue;
     // The **aggregate** deduction guides are handled in a different code path
     // (DeclareAggregateDeductionGuideFromInitList), which involves the tricky
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index 2d43e46b9e3d76b..6a004769a6d4d61 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -237,8 +237,17 @@ static_assert(__is_same(decltype(s.t), int));
 // explicit deduction guide.
 Foo(int) -> Foo<X>;
 AFoo s2{i};
-// FIXME: the type should be X because of the above explicit deduction guide.
-static_assert(__is_same(decltype(s2.t), int));
+static_assert(__is_same(decltype(s2.t), X));
+
+
+template<class T>
+using BFoo = AFoo<T>;
+static_assert(__is_same(decltype(BFoo(i).t), X));
+
+
+Foo(double) -> Foo<int>;
+static_assert(__is_same(decltype(AFoo(1.0).t), int));
+static_assert(__is_same(decltype(BFoo(1.0).t), int));
 } // namespace test16
 
 namespace test17 {

cor3ntin · 2025-02-03T11:57:37Z

clang/lib/Sema/SemaTemplateDeductionGuide.cpp

+      D = FTD->getTemplatedDecl();
+
+    if (const auto *GD = dyn_cast<CXXDeductionGuideDecl>(D))
+      Result.insert(GD->getSourceDeductionGuide());


This can be null right? So we'd be inserting a bunch of nullptr into the map which is harmless but also not useful

Added an assertion -- this cannot be nullptr in theory. The function is only called for the alias templates, if a deduction guide exists for a type alias, then it must be generated from a source deduction guide by definition.

cor3ntin · 2025-02-03T12:00:01Z

clang/lib/Sema/SemaTemplateDeductionGuide.cpp

+  auto SourceDeductionGuides = getSourceDeductionGuides(
+      Context.DeclarationNames.getCXXDeductionGuideName(AliasTemplate),
+      AliasTemplate->getDeclContext());
+


Would it be worth it to create the map only if it's needed?

Moved to right after the if (!Template) bailout.

cor3ntin

Thanks

antangelo

LGTM

antangelo · 2025-02-03T23:19:07Z

clang/test/SemaCXX/cxx20-ctad-type-alias.cpp

Do we also need a test case for the templated DG case?

Done, added one.

shafik · 2025-02-04T01:48:01Z

clang/test/SemaCXX/cxx20-ctad-type-alias.cpp

Can we get more complicated test cases, w/ multiple template params and multiple constructor arguments etc e.g.: https://compiler-explorer.com/z/x8PhEqWeo

We should really be trying harder to stress test our implementation.

Does it fix this case: https://godbolt.org/z/b84rzhjjK if not we need to open a bug.

Does it fix this case: https://godbolt.org/z/b84rzhjjK if not we need to open a bug.

That test case doesn't look related to these changes to me, I think it's a different issue.

https://compiler-explorer.com/z/x8PhEqWeo

A simpler example: https://compiler-explorer.com/z/Tdx1jd1bM

I think Clang (and MSVC) are correct in rejecting this example. The candidate deduction guide (AFoo(T t, U) -> Foo<T, U>) is not selected because the is_deducible type trait is not satisfied, as indicated in the diagnostic message.

It looks like a bug in gcc.

Does it fix this case: https://godbolt.org/z/b84rzhjjK if not we need to open a bug.

That test case doesn't look related to these changes to me, I think it's a different issue.

This is not related to this patch. Filed #125821 for tracking (thanks for spotting it!)

first use of the alias template.

…he first use of the alias template. (llvm#125478) Fixes llvm#103016 This is the last missing piece for the C++20 CTAD alias feature. No release note being added in this PR yet, I will send out a follow-up patch to mark this feature done. (Since the release 20 branch is cut, I think we should target on clang21).

vogelsgesang · 2025-07-08T15:52:37Z

@hokein is there still anything missing to set the feature test macro and mark P1814 as done on https://clang.llvm.org/cxx_status.html#cxx26?

This is the last missing piece for the C++20 CTAD alias feature. No release note being added in this PR yet, I will send out a follow-up patch to mark this feature done.

(Since the release 20 branch is cut, I think we should target on clang21).

Are we still targetting to finish this for clang-21 before the branch cut?

hokein · 2025-07-08T21:13:22Z

Thanks for the reminder.

Are we still targetting to finish this for clang-21 before the branch cut?

A quick search in the issue tracker shows that there are some opening issues, but I think we can consider this feature "done," as those issues don’t appear to be hard blockers. @cor3ntin @shafik thoughts?

hokein requested review from antangelo and cor3ntin February 3, 2025 11:02

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 3, 2025

cor3ntin reviewed Feb 3, 2025

View reviewed changes

hokein changed the title ~~[clang] CTAD alias: Respecte explicit deduction guides defined after the first use of the alias template.~~ [clang] CTAD alias: Respect explicit deduction guides defined after the first use of the alias template. Feb 3, 2025

cor3ntin approved these changes Feb 3, 2025

View reviewed changes

antangelo approved these changes Feb 3, 2025

View reviewed changes

shafik reviewed Feb 4, 2025

View reviewed changes

hokein mentioned this pull request Feb 5, 2025

CTAD for alias template crashes. #125821

Closed

hokein added 3 commits February 10, 2025 14:20

[clang] CTAD alias: Respecte explicit deduction guides defined after the

4af8e19

first use of the alias template.

Address review comments.

abc8882

Add a test case for explicit template deduction guides.

1750551

hokein force-pushed the ctad-after-first-use branch from 49be6d4 to 1750551 Compare February 10, 2025 13:47

hokein merged commit 4d2a1bf into llvm:main Feb 10, 2025
6 of 8 checks passed

hokein deleted the ctad-after-first-use branch February 10, 2025 14:05

[clang] CTAD alias: Respect explicit deduction guides defined after the first use of the alias template. #125478

[clang] CTAD alias: Respect explicit deduction guides defined after the first use of the alias template. #125478

Uh oh!

Conversation

hokein commented Feb 3, 2025

Uh oh!

llvmbot commented Feb 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

antangelo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vogelsgesang commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hokein commented Jul 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vogelsgesang commented Jul 8, 2025 •

edited

Loading