release/20.x: [Clang] Fix various bugs in alias CTAD transform

zyn0217 · tstellar · commit c86df914dee1 · 2025-03-25T16:37:15.000-07:00
This patch cherry-picks 032ad59, 868c89f and 38d71c9 onto the 20 release branch. The first patch addresses recently surfaced CTAD problems, which we believe it would be nice to roll out the fix quickly, given the release window is not closed yet. The second patch is a follow-up to the first and fixed a test failure on the arm32 platform. The third patch follows-up on the previous patch that I cherry-picked to the 20 release branch, which removes a unnecessary assertion.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -1060,6 +1060,8 @@ Bug Fixes to C++ Support
 - Clang is now better at keeping track of friend function template instance contexts. (#GH55509)
 - Fixed an integer overflow bug in computing template parameter depths when synthesizing CTAD guides. (#GH128691)
 - Fixed an incorrect pointer access when checking access-control on concepts. (#GH131530)
+- Fixed various alias CTAD bugs involving variadic template arguments. (#GH123591), (#GH127539), (#GH129077),
+  (#GH129620), and (#GH129998).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -377,12 +377,10 @@ struct ConvertConstructorToDeductionGuideTransform {
         if (NestedPattern)
           Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
         auto [Depth, Index] = getDepthAndIndex(Param);
-        // Depth can still be 0 if FTD belongs to an explicit class template
-        // specialization with an empty template parameter list. In that case,
-        // we don't want the NewDepth to overflow, and it should remain 0.
-        assert(Depth ||
-               cast<ClassTemplateSpecializationDecl>(FTD->getDeclContext())
-                   ->isExplicitSpecialization());
+        // Depth can be 0 if FTD belongs to a non-template class/a class
+        // template specialization with an empty template parameter list. In
+        // that case, we don't want the NewDepth to overflow, and it should
+        // remain 0.
         NamedDecl *NewParam = transformTemplateParameter(
             SemaRef, DC, Param, Args, Index + Depth1IndexAdjustment,
             Depth ? Depth - 1 : 0);
@@ -989,6 +987,19 @@ getRHSTemplateDeclAndArgs(Sema &SemaRef, TypeAliasTemplateDecl *AliasTemplate) {
   return {Template, AliasRhsTemplateArgs};
 }
 
+bool IsNonDeducedArgument(const TemplateArgument &TA) {
+  // The following cases indicate the template argument is non-deducible:
+  //   1. The result is null. E.g. When it comes from a default template
+  //   argument that doesn't appear in the alias declaration.
+  //   2. The template parameter is a pack and that cannot be deduced from
+  //   the arguments within the alias declaration.
+  // Non-deducible template parameters will persist in the transformed
+  // deduction guide.
+  return TA.isNull() ||
+         (TA.getKind() == TemplateArgument::Pack &&
+          llvm::any_of(TA.pack_elements(), IsNonDeducedArgument));
+}
+
 // Build deduction guides for a type alias template from the given underlying
 // deduction guide F.
 FunctionTemplateDecl *
@@ -1057,7 +1068,8 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
   // !!NOTE: DeduceResults respects the sequence of template parameters of
   // the deduction guide f.
   for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
-    if (const auto &D = DeduceResults[Index]; !D.isNull()) // Deduced
+    const auto &D = DeduceResults[Index];
+    if (!IsNonDeducedArgument(D))
       DeducedArgs.push_back(D);
     else
       NonDeducedTemplateParamsInFIndex.push_back(Index);
@@ -1121,7 +1133,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
   Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
   for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
     const auto &D = DeduceResults[Index];
-    if (D.isNull()) {
+    if (IsNonDeducedArgument(D)) {
       // 2): Non-deduced template parameters would be substituted later.
       continue;
     }
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1347,6 +1347,16 @@ std::optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const {
   return std::nullopt;
 }
 
+static TemplateArgument
+getPackSubstitutedTemplateArgument(Sema &S, TemplateArgument Arg) {
+  assert(S.ArgumentPackSubstitutionIndex >= 0);
+  assert(S.ArgumentPackSubstitutionIndex < (int)Arg.pack_size());
+  Arg = Arg.pack_begin()[S.ArgumentPackSubstitutionIndex];
+  if (Arg.isPackExpansion())
+    Arg = Arg.getPackExpansionPattern();
+  return Arg;
+}
+
 //===----------------------------------------------------------------------===/
 // Template Instantiation for Types
 //===----------------------------------------------------------------------===/
@@ -1466,11 +1476,13 @@ namespace {
       }
     }
 
-    static TemplateArgument
+    TemplateArgument
     getTemplateArgumentPackPatternForRewrite(const TemplateArgument &TA) {
       if (TA.getKind() != TemplateArgument::Pack)
         return TA;
-      assert(TA.pack_size() == 1 &&
+      if (SemaRef.ArgumentPackSubstitutionIndex != -1)
+        return getPackSubstitutedTemplateArgument(SemaRef, TA);
+      assert(TA.pack_size() == 1 && TA.pack_begin()->isPackExpansion() &&
              "unexpected pack arguments in template rewrite");
       TemplateArgument Arg = *TA.pack_begin();
       if (Arg.isPackExpansion())
@@ -1629,6 +1641,9 @@ namespace {
       std::vector<TemplateArgument> TArgs;
       switch (Arg.getKind()) {
       case TemplateArgument::Pack:
+        assert(SemaRef.CodeSynthesisContexts.empty() ||
+               SemaRef.CodeSynthesisContexts.back().Kind ==
+                   Sema::CodeSynthesisContext::BuildingDeductionGuides);
         // Literally rewrite the template argument pack, instead of unpacking
         // it.
         for (auto &pack : Arg.getPackAsArray()) {
@@ -1649,6 +1664,23 @@ namespace {
       return inherited::TransformTemplateArgument(Input, Output, Uneval);
     }
 
+    std::optional<unsigned> ComputeSizeOfPackExprWithoutSubstitution(
+        ArrayRef<TemplateArgument> PackArgs) {
+      // Don't do this when rewriting template parameters for CTAD:
+      //   1) The heuristic needs the unpacked Subst* nodes to figure out the
+      //   expanded size, but this never applies since Subst* nodes are not
+      //   created in rewrite scenarios.
+      //
+      //   2) The heuristic substitutes into the pattern with pack expansion
+      //   suppressed, which does not meet the requirements for argument
+      //   rewriting when template arguments include a non-pack matching against
+      //   a pack, particularly when rewriting an alias CTAD.
+      if (TemplateArgs.isRewrite())
+        return std::nullopt;
+
+      return inherited::ComputeSizeOfPackExprWithoutSubstitution(PackArgs);
+    }
+
     template<typename Fn>
     QualType TransformFunctionProtoType(TypeLocBuilder &TLB,
                                         FunctionProtoTypeLoc TL,
@@ -1867,16 +1899,6 @@ bool TemplateInstantiator::AlreadyTransformed(QualType T) {
   return true;
 }
 
-static TemplateArgument
-getPackSubstitutedTemplateArgument(Sema &S, TemplateArgument Arg) {
-  assert(S.ArgumentPackSubstitutionIndex >= 0);
-  assert(S.ArgumentPackSubstitutionIndex < (int)Arg.pack_size());
-  Arg = Arg.pack_begin()[S.ArgumentPackSubstitutionIndex];
-  if (Arg.isPackExpansion())
-    Arg = Arg.getPackExpansionPattern();
-  return Arg;
-}
-
 Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) {
   if (!D)
     return nullptr;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
@@ -3660,6 +3660,9 @@ class TreeTransform {
     return SemaRef.BuildCXXNoexceptExpr(Range.getBegin(), Arg, Range.getEnd());
   }
 
+  std::optional<unsigned>
+  ComputeSizeOfPackExprWithoutSubstitution(ArrayRef<TemplateArgument> PackArgs);
+
   /// Build a new expression to compute the length of a parameter pack.
   ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc, NamedDecl *Pack,
                                    SourceLocation PackLoc,
@@ -15877,6 +15880,49 @@ TreeTransform<Derived>::TransformPackExpansionExpr(PackExpansionExpr *E) {
                                            E->getNumExpansions());
 }
 
+template <typename Derived>
+std::optional<unsigned>
+TreeTransform<Derived>::ComputeSizeOfPackExprWithoutSubstitution(
+    ArrayRef<TemplateArgument> PackArgs) {
+  std::optional<unsigned> Result = 0;
+  for (const TemplateArgument &Arg : PackArgs) {
+    if (!Arg.isPackExpansion()) {
+      Result = *Result + 1;
+      continue;
+    }
+
+    TemplateArgumentLoc ArgLoc;
+    InventTemplateArgumentLoc(Arg, ArgLoc);
+
+    // Find the pattern of the pack expansion.
+    SourceLocation Ellipsis;
+    std::optional<unsigned> OrigNumExpansions;
+    TemplateArgumentLoc Pattern =
+        getSema().getTemplateArgumentPackExpansionPattern(ArgLoc, Ellipsis,
+                                                          OrigNumExpansions);
+
+    // Substitute under the pack expansion. Do not expand the pack (yet).
+    TemplateArgumentLoc OutPattern;
+    Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
+    if (getDerived().TransformTemplateArgument(Pattern, OutPattern,
+                                               /*Uneval*/ true))
+      return true;
+
+    // See if we can determine the number of arguments from the result.
+    std::optional<unsigned> NumExpansions =
+        getSema().getFullyPackExpandedSize(OutPattern.getArgument());
+    if (!NumExpansions) {
+      // No: we must be in an alias template expansion, and we're going to
+      // need to actually expand the packs.
+      Result = std::nullopt;
+      break;
+    }
+
+    Result = *Result + *NumExpansions;
+  }
+  return Result;
+}
+
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
@@ -15942,42 +15988,8 @@ TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
   }
 
   // Try to compute the result without performing a partial substitution.
-  std::optional<unsigned> Result = 0;
-  for (const TemplateArgument &Arg : PackArgs) {
-    if (!Arg.isPackExpansion()) {
-      Result = *Result + 1;
-      continue;
-    }
-
-    TemplateArgumentLoc ArgLoc;
-    InventTemplateArgumentLoc(Arg, ArgLoc);
-
-    // Find the pattern of the pack expansion.
-    SourceLocation Ellipsis;
-    std::optional<unsigned> OrigNumExpansions;
-    TemplateArgumentLoc Pattern =
-        getSema().getTemplateArgumentPackExpansionPattern(ArgLoc, Ellipsis,
-                                                          OrigNumExpansions);
-
-    // Substitute under the pack expansion. Do not expand the pack (yet).
-    TemplateArgumentLoc OutPattern;
-    Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
-    if (getDerived().TransformTemplateArgument(Pattern, OutPattern,
-                                               /*Uneval*/ true))
-      return true;
-
-    // See if we can determine the number of arguments from the result.
-    std::optional<unsigned> NumExpansions =
-        getSema().getFullyPackExpandedSize(OutPattern.getArgument());
-    if (!NumExpansions) {
-      // No: we must be in an alias template expansion, and we're going to need
-      // to actually expand the packs.
-      Result = std::nullopt;
-      break;
-    }
-
-    Result = *Result + *NumExpansions;
-  }
+  std::optional<unsigned> Result =
+      getDerived().ComputeSizeOfPackExprWithoutSubstitution(PackArgs);
 
   // Common case: we could determine the number of expansions without
   // substituting.
diff --git a/clang/test/SemaCXX/ctad.cpp b/clang/test/SemaCXX/ctad.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -std=c++20 %s
-// expected-no-diagnostics
 
 namespace GH64347 {
 
@@ -17,3 +16,134 @@ void k() {
 }
 
 } // namespace GH64347
+
+namespace GH123591 {
+
+
+template < typename... _Types >
+struct variant {
+  template <int N = sizeof...(_Types)>
+  variant(_Types...);
+};
+
+template <class T>
+using AstNode = variant<T, T, T>;
+
+AstNode tree(42, 43, 44);
+
+}
+
+namespace GH123591_2 {
+
+template <int>
+using enable_if_t = char;
+
+template < typename... Types >
+struct variant {
+  template < enable_if_t<sizeof...(Types)>>
+  variant();
+};
+
+template <int>
+using AstNode = variant<>;
+// expected-note@-1 {{couldn't infer template argument ''}} \
+// expected-note@-1 2{{implicit deduction guide declared as}} \
+// expected-note@-1 {{candidate function template not viable}}
+
+
+AstNode tree; // expected-error {{no viable constructor or deduction guide}}
+
+}
+
+namespace GH127539 {
+
+template <class...>
+struct A {
+    template <class... ArgTs>
+    A(ArgTs...) {}
+};
+
+template <class... ArgTs>
+A(ArgTs...) -> A<typename ArgTs::value_type...>;
+
+template <class... Ts>
+using AA = A<Ts..., Ts...>;
+
+AA a{};
+
+}
+
+namespace GH129077 {
+
+using size_t = decltype(sizeof(0));
+
+struct index_type
+{
+  size_t value = 0;
+  index_type() = default;
+  constexpr index_type(size_t i) noexcept : value(i) {}
+};
+
+template <index_type... Extents>
+struct extents
+{
+  constexpr extents(decltype(Extents)...) noexcept {}
+};
+
+template <class... Extents>
+extents(Extents...) -> extents<(requires { Extents::value; } ? Extents{} : ~0ull)...>;
+
+template <index_type... Index>
+using index = extents<Index...>;
+
+int main()
+{
+  extents i{0,0};
+  auto j = extents<64,{}>({}, 42);
+
+  index k{0,0};
+  auto l = index<64,{}>({}, 42);
+
+  return 0;
+}
+
+}
+
+namespace GH129620 {
+
+template <class... Ts>
+struct A {
+    constexpr A(Ts...) {}
+};
+
+template <class... Ts>
+using Foo = A<Ts...>;
+
+template <class T>
+using Bar = Foo<T, T>;
+
+Bar a{0, 0};
+
+}
+
+namespace GH129998 {
+
+struct converible_to_one {
+    constexpr operator int() const noexcept { return 1; }
+};
+
+template <int... Extents>
+struct class_template {
+    class_template() = default;
+    constexpr class_template(auto&&...) noexcept {}
+};
+
+template <class... Extents>
+class_template(Extents...) -> class_template<(true ? 0 : +Extents{})...>;
+
+template <int... Extents>
+using alias_template = class_template<Extents...>;
+
+alias_template var2{converible_to_one{}, 2};
+
+}
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
