[clang] [Clang] Fix various bugs in alias CTAD transform (PR #132061)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 20 01:04:42 PDT 2025
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/132061
>From fb9fa67da10a7dbfb2db5520d2773085585f4c14 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 20 Mar 2025 00:54:54 +0800
Subject: [PATCH 1/2] [Clang] Fix various bugs in alias CTAD transform
---
clang/lib/Sema/SemaTemplateDeductionGuide.cpp | 11 +-
clang/lib/Sema/SemaTemplateInstantiate.cpp | 31 +++--
clang/lib/Sema/TreeTransform.h | 6 +
clang/test/SemaCXX/ctad.cpp | 115 +++++++++++++++++-
4 files changed, 149 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index ee89ee8594bc4..63a100545b5e7 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -1077,7 +1077,11 @@ 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];
+ bool NonDeduced =
+ D.isNull() || (D.getKind() == TemplateArgument::Pack &&
+ D.pack_size() == 1 && D.pack_begin()->isNull());
+ if (!NonDeduced)
DeducedArgs.push_back(D);
else
NonDeducedTemplateParamsInFIndex.push_back(Index);
@@ -1141,7 +1145,10 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
- if (D.isNull()) {
+ bool NonDeduced =
+ D.isNull() || (D.getKind() == TemplateArgument::Pack &&
+ D.pack_size() == 1 && D.pack_begin()->isNull());
+ if (NonDeduced) {
// 2): Non-deduced template parameters would be substituted later.
continue;
}
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 19c27a76b182c..9371b578c8558 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1348,6 +1348,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
//===----------------------------------------------------------------------===/
@@ -1467,10 +1477,16 @@ namespace {
}
}
- static TemplateArgument
+ bool HeuristicallyComputeSizeOfPackExpr() const {
+ return !TemplateArgs.isRewrite();
+ }
+
+ TemplateArgument
getTemplateArgumentPackPatternForRewrite(const TemplateArgument &TA) {
if (TA.getKind() != TemplateArgument::Pack)
return TA;
+ if (SemaRef.ArgumentPackSubstitutionIndex != -1)
+ return getPackSubstitutedTemplateArgument(SemaRef, TA);
assert(TA.pack_size() == 1 &&
"unexpected pack arguments in template rewrite");
TemplateArgument Arg = *TA.pack_begin();
@@ -1630,6 +1646,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()) {
@@ -1869,16 +1888,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
index b5de98e3989ea..5d96c3fcf92e3 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3660,6 +3660,8 @@ class TreeTransform {
return SemaRef.BuildCXXNoexceptExpr(Range.getBegin(), Arg, Range.getEnd());
}
+ bool HeuristicallyComputeSizeOfPackExpr() const { return true; }
+
/// Build a new expression to compute the length of a parameter pack.
ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc, NamedDecl *Pack,
SourceLocation PackLoc,
@@ -16095,6 +16097,10 @@ 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 (!getDerived().HeuristicallyComputeSizeOfPackExpr()) {
+ Result = std::nullopt;
+ break;
+ }
if (!Arg.isPackExpansion()) {
Result = *Result + 1;
continue;
diff --git a/clang/test/SemaCXX/ctad.cpp b/clang/test/SemaCXX/ctad.cpp
index 10806f107b4ee..813d5d7098663 100644
--- 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,117 @@ 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 at -1 {{couldn't infer template argument ''}} \
+// expected-note at -1 2{{implicit deduction guide declared as}} \
+// expected-note at -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...>;
+
+AA a{};
+
+}
+
+namespace GH129077 {
+
+using size_t = decltype(sizeof(0));
+
+struct index_type
+{
+ size_t value{~0ull};
+ 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 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};
+
+}
>From 9da379e238dbde9fd0d41c9586e99077dae6283a Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 20 Mar 2025 16:02:04 +0800
Subject: [PATCH 2/2] Address comments
---
clang/lib/Sema/SemaTemplateDeductionGuide.cpp | 22 +++--
clang/lib/Sema/SemaTemplateInstantiate.cpp | 19 +++-
clang/lib/Sema/TreeTransform.h | 88 ++++++++++---------
3 files changed, 79 insertions(+), 50 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index 63a100545b5e7..6e225c8caaa09 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -1072,16 +1072,25 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
AliasRhsTemplateArgs, TDeduceInfo, DeduceResults,
/*NumberOfArgumentsMustMatch=*/false);
+ static auto IsNonDeducedArgument = [&](const DeducedTemplateArgument &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 &&
+ TA.pack_size() == 1 && TA.pack_begin()->isNull());
+ };
+
SmallVector<TemplateArgument> DeducedArgs;
SmallVector<unsigned> NonDeducedTemplateParamsInFIndex;
// !!NOTE: DeduceResults respects the sequence of template parameters of
// the deduction guide f.
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
- bool NonDeduced =
- D.isNull() || (D.getKind() == TemplateArgument::Pack &&
- D.pack_size() == 1 && D.pack_begin()->isNull());
- if (!NonDeduced)
+ if (!IsNonDeducedArgument(D))
DeducedArgs.push_back(D);
else
NonDeducedTemplateParamsInFIndex.push_back(Index);
@@ -1145,10 +1154,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
const auto &D = DeduceResults[Index];
- bool NonDeduced =
- D.isNull() || (D.getKind() == TemplateArgument::Pack &&
- D.pack_size() == 1 && D.pack_begin()->isNull());
- if (NonDeduced) {
+ 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
index 9371b578c8558..358f337be2e64 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1487,7 +1487,7 @@ namespace {
return TA;
if (SemaRef.ArgumentPackSubstitutionIndex != -1)
return getPackSubstitutedTemplateArgument(SemaRef, TA);
- assert(TA.pack_size() == 1 &&
+ assert(TA.pack_size() == 1 && TA.pack_begin()->isPackExpansion() &&
"unexpected pack arguments in template rewrite");
TemplateArgument Arg = *TA.pack_begin();
if (Arg.isPackExpansion())
@@ -1669,6 +1669,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,
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 5d96c3fcf92e3..b101db3631f1f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3660,7 +3660,8 @@ class TreeTransform {
return SemaRef.BuildCXXNoexceptExpr(Range.getBegin(), Arg, Range.getEnd());
}
- bool HeuristicallyComputeSizeOfPackExpr() const { return true; }
+ 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,
@@ -16030,6 +16031,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) {
@@ -16095,46 +16139,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 (!getDerived().HeuristicallyComputeSizeOfPackExpr()) {
- Result = std::nullopt;
- break;
- }
- 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.
More information about the cfe-commits
mailing list