Skip to content

Commit c86df91

Browse files
zyn0217tstellar
zyn0217
authored and
tstellar
committed
release/20.x: [Clang] Fix various bugs in alias CTAD transform
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.
1 parent f7b6f23 commit c86df91

File tree

6 files changed

+283
-57
lines changed

6 files changed

+283
-57
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,8 @@ Bug Fixes to C++ Support
10601060
- Clang is now better at keeping track of friend function template instance contexts. (#GH55509)
10611061
- Fixed an integer overflow bug in computing template parameter depths when synthesizing CTAD guides. (#GH128691)
10621062
- Fixed an incorrect pointer access when checking access-control on concepts. (#GH131530)
1063+
- Fixed various alias CTAD bugs involving variadic template arguments. (#GH123591), (#GH127539), (#GH129077),
1064+
(#GH129620), and (#GH129998).
10631065

10641066
Bug Fixes to AST Handling
10651067
^^^^^^^^^^^^^^^^^^^^^^^^^

‎clang/lib/Sema/SemaTemplateDeductionGuide.cpp‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,12 +377,10 @@ struct ConvertConstructorToDeductionGuideTransform {
377377
if (NestedPattern)
378378
Args.addOuterRetainedLevels(NestedPattern->getTemplateDepth());
379379
auto [Depth, Index] = getDepthAndIndex(Param);
380-
// Depth can still be 0 if FTD belongs to an explicit class template
381-
// specialization with an empty template parameter list. In that case,
382-
// we don't want the NewDepth to overflow, and it should remain 0.
383-
assert(Depth ||
384-
cast<ClassTemplateSpecializationDecl>(FTD->getDeclContext())
385-
->isExplicitSpecialization());
380+
// Depth can be 0 if FTD belongs to a non-template class/a class
381+
// template specialization with an empty template parameter list. In
382+
// that case, we don't want the NewDepth to overflow, and it should
383+
// remain 0.
386384
NamedDecl *NewParam = transformTemplateParameter(
387385
SemaRef, DC, Param, Args, Index + Depth1IndexAdjustment,
388386
Depth ? Depth - 1 : 0);
@@ -989,6 +987,19 @@ getRHSTemplateDeclAndArgs(Sema &SemaRef, TypeAliasTemplateDecl *AliasTemplate) {
989987
return {Template, AliasRhsTemplateArgs};
990988
}
991989

990+
bool IsNonDeducedArgument(const TemplateArgument &TA) {
991+
// The following cases indicate the template argument is non-deducible:
992+
// 1. The result is null. E.g. When it comes from a default template
993+
// argument that doesn't appear in the alias declaration.
994+
// 2. The template parameter is a pack and that cannot be deduced from
995+
// the arguments within the alias declaration.
996+
// Non-deducible template parameters will persist in the transformed
997+
// deduction guide.
998+
return TA.isNull() ||
999+
(TA.getKind() == TemplateArgument::Pack &&
1000+
llvm::any_of(TA.pack_elements(), IsNonDeducedArgument));
1001+
}
1002+
9921003
// Build deduction guides for a type alias template from the given underlying
9931004
// deduction guide F.
9941005
FunctionTemplateDecl *
@@ -1057,7 +1068,8 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
10571068
// !!NOTE: DeduceResults respects the sequence of template parameters of
10581069
// the deduction guide f.
10591070
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
1060-
if (const auto &D = DeduceResults[Index]; !D.isNull()) // Deduced
1071+
const auto &D = DeduceResults[Index];
1072+
if (!IsNonDeducedArgument(D))
10611073
DeducedArgs.push_back(D);
10621074
else
10631075
NonDeducedTemplateParamsInFIndex.push_back(Index);
@@ -1121,7 +1133,7 @@ BuildDeductionGuideForTypeAlias(Sema &SemaRef,
11211133
Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
11221134
for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
11231135
const auto &D = DeduceResults[Index];
1124-
if (D.isNull()) {
1136+
if (IsNonDeducedArgument(D)) {
11251137
// 2): Non-deduced template parameters would be substituted later.
11261138
continue;
11271139
}

‎clang/lib/Sema/SemaTemplateInstantiate.cpp‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,16 @@ std::optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const {
13471347
return std::nullopt;
13481348
}
13491349

1350+
static TemplateArgument
1351+
getPackSubstitutedTemplateArgument(Sema &S, TemplateArgument Arg) {
1352+
assert(S.ArgumentPackSubstitutionIndex >= 0);
1353+
assert(S.ArgumentPackSubstitutionIndex < (int)Arg.pack_size());
1354+
Arg = Arg.pack_begin()[S.ArgumentPackSubstitutionIndex];
1355+
if (Arg.isPackExpansion())
1356+
Arg = Arg.getPackExpansionPattern();
1357+
return Arg;
1358+
}
1359+
13501360
//===----------------------------------------------------------------------===/
13511361
// Template Instantiation for Types
13521362
//===----------------------------------------------------------------------===/
@@ -1466,11 +1476,13 @@ namespace {
14661476
}
14671477
}
14681478

1469-
static TemplateArgument
1479+
TemplateArgument
14701480
getTemplateArgumentPackPatternForRewrite(const TemplateArgument &TA) {
14711481
if (TA.getKind() != TemplateArgument::Pack)
14721482
return TA;
1473-
assert(TA.pack_size() == 1 &&
1483+
if (SemaRef.ArgumentPackSubstitutionIndex != -1)
1484+
return getPackSubstitutedTemplateArgument(SemaRef, TA);
1485+
assert(TA.pack_size() == 1 && TA.pack_begin()->isPackExpansion() &&
14741486
"unexpected pack arguments in template rewrite");
14751487
TemplateArgument Arg = *TA.pack_begin();
14761488
if (Arg.isPackExpansion())
@@ -1629,6 +1641,9 @@ namespace {
16291641
std::vector<TemplateArgument> TArgs;
16301642
switch (Arg.getKind()) {
16311643
case TemplateArgument::Pack:
1644+
assert(SemaRef.CodeSynthesisContexts.empty() ||
1645+
SemaRef.CodeSynthesisContexts.back().Kind ==
1646+
Sema::CodeSynthesisContext::BuildingDeductionGuides);
16321647
// Literally rewrite the template argument pack, instead of unpacking
16331648
// it.
16341649
for (auto &pack : Arg.getPackAsArray()) {
@@ -1649,6 +1664,23 @@ namespace {
16491664
return inherited::TransformTemplateArgument(Input, Output, Uneval);
16501665
}
16511666

1667+
std::optional<unsigned> ComputeSizeOfPackExprWithoutSubstitution(
1668+
ArrayRef<TemplateArgument> PackArgs) {
1669+
// Don't do this when rewriting template parameters for CTAD:
1670+
// 1) The heuristic needs the unpacked Subst* nodes to figure out the
1671+
// expanded size, but this never applies since Subst* nodes are not
1672+
// created in rewrite scenarios.
1673+
//
1674+
// 2) The heuristic substitutes into the pattern with pack expansion
1675+
// suppressed, which does not meet the requirements for argument
1676+
// rewriting when template arguments include a non-pack matching against
1677+
// a pack, particularly when rewriting an alias CTAD.
1678+
if (TemplateArgs.isRewrite())
1679+
return std::nullopt;
1680+
1681+
return inherited::ComputeSizeOfPackExprWithoutSubstitution(PackArgs);
1682+
}
1683+
16521684
template<typename Fn>
16531685
QualType TransformFunctionProtoType(TypeLocBuilder &TLB,
16541686
FunctionProtoTypeLoc TL,
@@ -1867,16 +1899,6 @@ bool TemplateInstantiator::AlreadyTransformed(QualType T) {
18671899
return true;
18681900
}
18691901

1870-
static TemplateArgument
1871-
getPackSubstitutedTemplateArgument(Sema &S, TemplateArgument Arg) {
1872-
assert(S.ArgumentPackSubstitutionIndex >= 0);
1873-
assert(S.ArgumentPackSubstitutionIndex < (int)Arg.pack_size());
1874-
Arg = Arg.pack_begin()[S.ArgumentPackSubstitutionIndex];
1875-
if (Arg.isPackExpansion())
1876-
Arg = Arg.getPackExpansionPattern();
1877-
return Arg;
1878-
}
1879-
18801902
Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) {
18811903
if (!D)
18821904
return nullptr;

‎clang/lib/Sema/TreeTransform.h‎

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3660,6 +3660,9 @@ class TreeTransform {
36603660
return SemaRef.BuildCXXNoexceptExpr(Range.getBegin(), Arg, Range.getEnd());
36613661
}
36623662

3663+
std::optional<unsigned>
3664+
ComputeSizeOfPackExprWithoutSubstitution(ArrayRef<TemplateArgument> PackArgs);
3665+
36633666
/// Build a new expression to compute the length of a parameter pack.
36643667
ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc, NamedDecl *Pack,
36653668
SourceLocation PackLoc,
@@ -15877,6 +15880,49 @@ TreeTransform<Derived>::TransformPackExpansionExpr(PackExpansionExpr *E) {
1587715880
E->getNumExpansions());
1587815881
}
1587915882

15883+
template <typename Derived>
15884+
std::optional<unsigned>
15885+
TreeTransform<Derived>::ComputeSizeOfPackExprWithoutSubstitution(
15886+
ArrayRef<TemplateArgument> PackArgs) {
15887+
std::optional<unsigned> Result = 0;
15888+
for (const TemplateArgument &Arg : PackArgs) {
15889+
if (!Arg.isPackExpansion()) {
15890+
Result = *Result + 1;
15891+
continue;
15892+
}
15893+
15894+
TemplateArgumentLoc ArgLoc;
15895+
InventTemplateArgumentLoc(Arg, ArgLoc);
15896+
15897+
// Find the pattern of the pack expansion.
15898+
SourceLocation Ellipsis;
15899+
std::optional<unsigned> OrigNumExpansions;
15900+
TemplateArgumentLoc Pattern =
15901+
getSema().getTemplateArgumentPackExpansionPattern(ArgLoc, Ellipsis,
15902+
OrigNumExpansions);
15903+
15904+
// Substitute under the pack expansion. Do not expand the pack (yet).
15905+
TemplateArgumentLoc OutPattern;
15906+
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
15907+
if (getDerived().TransformTemplateArgument(Pattern, OutPattern,
15908+
/*Uneval*/ true))
15909+
return true;
15910+
15911+
// See if we can determine the number of arguments from the result.
15912+
std::optional<unsigned> NumExpansions =
15913+
getSema().getFullyPackExpandedSize(OutPattern.getArgument());
15914+
if (!NumExpansions) {
15915+
// No: we must be in an alias template expansion, and we're going to
15916+
// need to actually expand the packs.
15917+
Result = std::nullopt;
15918+
break;
15919+
}
15920+
15921+
Result = *Result + *NumExpansions;
15922+
}
15923+
return Result;
15924+
}
15925+
1588015926
template<typename Derived>
1588115927
ExprResult
1588215928
TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
@@ -15942,42 +15988,8 @@ TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
1594215988
}
1594315989

1594415990
// Try to compute the result without performing a partial substitution.
15945-
std::optional<unsigned> Result = 0;
15946-
for (const TemplateArgument &Arg : PackArgs) {
15947-
if (!Arg.isPackExpansion()) {
15948-
Result = *Result + 1;
15949-
continue;
15950-
}
15951-
15952-
TemplateArgumentLoc ArgLoc;
15953-
InventTemplateArgumentLoc(Arg, ArgLoc);
15954-
15955-
// Find the pattern of the pack expansion.
15956-
SourceLocation Ellipsis;
15957-
std::optional<unsigned> OrigNumExpansions;
15958-
TemplateArgumentLoc Pattern =
15959-
getSema().getTemplateArgumentPackExpansionPattern(ArgLoc, Ellipsis,
15960-
OrigNumExpansions);
15961-
15962-
// Substitute under the pack expansion. Do not expand the pack (yet).
15963-
TemplateArgumentLoc OutPattern;
15964-
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
15965-
if (getDerived().TransformTemplateArgument(Pattern, OutPattern,
15966-
/*Uneval*/ true))
15967-
return true;
15968-
15969-
// See if we can determine the number of arguments from the result.
15970-
std::optional<unsigned> NumExpansions =
15971-
getSema().getFullyPackExpandedSize(OutPattern.getArgument());
15972-
if (!NumExpansions) {
15973-
// No: we must be in an alias template expansion, and we're going to need
15974-
// to actually expand the packs.
15975-
Result = std::nullopt;
15976-
break;
15977-
}
15978-
15979-
Result = *Result + *NumExpansions;
15980-
}
15991+
std::optional<unsigned> Result =
15992+
getDerived().ComputeSizeOfPackExprWithoutSubstitution(PackArgs);
1598115993

1598215994
// Common case: we could determine the number of expansions without
1598315995
// substituting.

‎clang/test/SemaCXX/ctad.cpp‎

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-unused-value -std=c++20 %s
2-
// expected-no-diagnostics
32

43
namespace GH64347 {
54

@@ -17,3 +16,134 @@ void k() {
1716
}
1817

1918
} // namespace GH64347
19+
20+
namespace GH123591 {
21+
22+
23+
template < typename... _Types >
24+
struct variant {
25+
template <int N = sizeof...(_Types)>
26+
variant(_Types...);
27+
};
28+
29+
template <class T>
30+
using AstNode = variant<T, T, T>;
31+
32+
AstNode tree(42, 43, 44);
33+
34+
}
35+
36+
namespace GH123591_2 {
37+
38+
template <int>
39+
using enable_if_t = char;
40+
41+
template < typename... Types >
42+
struct variant {
43+
template < enable_if_t<sizeof...(Types)>>
44+
variant();
45+
};
46+
47+
template <int>
48+
using AstNode = variant<>;
49+
// expected-note@-1 {{couldn't infer template argument ''}} \
50+
// expected-note@-1 2{{implicit deduction guide declared as}} \
51+
// expected-note@-1 {{candidate function template not viable}}
52+
53+
54+
AstNode tree; // expected-error {{no viable constructor or deduction guide}}
55+
56+
}
57+
58+
namespace GH127539 {
59+
60+
template <class...>
61+
struct A {
62+
template <class... ArgTs>
63+
A(ArgTs...) {}
64+
};
65+
66+
template <class... ArgTs>
67+
A(ArgTs...) -> A<typename ArgTs::value_type...>;
68+
69+
template <class... Ts>
70+
using AA = A<Ts..., Ts...>;
71+
72+
AA a{};
73+
74+
}
75+
76+
namespace GH129077 {
77+
78+
using size_t = decltype(sizeof(0));
79+
80+
struct index_type
81+
{
82+
size_t value = 0;
83+
index_type() = default;
84+
constexpr index_type(size_t i) noexcept : value(i) {}
85+
};
86+
87+
template <index_type... Extents>
88+
struct extents
89+
{
90+
constexpr extents(decltype(Extents)...) noexcept {}
91+
};
92+
93+
template <class... Extents>
94+
extents(Extents...) -> extents<(requires { Extents::value; } ? Extents{} : ~0ull)...>;
95+
96+
template <index_type... Index>
97+
using index = extents<Index...>;
98+
99+
int main()
100+
{
101+
extents i{0,0};
102+
auto j = extents<64,{}>({}, 42);
103+
104+
index k{0,0};
105+
auto l = index<64,{}>({}, 42);
106+
107+
return 0;
108+
}
109+
110+
}
111+
112+
namespace GH129620 {
113+
114+
template <class... Ts>
115+
struct A {
116+
constexpr A(Ts...) {}
117+
};
118+
119+
template <class... Ts>
120+
using Foo = A<Ts...>;
121+
122+
template <class T>
123+
using Bar = Foo<T, T>;
124+
125+
Bar a{0, 0};
126+
127+
}
128+
129+
namespace GH129998 {
130+
131+
struct converible_to_one {
132+
constexpr operator int() const noexcept { return 1; }
133+
};
134+
135+
template <int... Extents>
136+
struct class_template {
137+
class_template() = default;
138+
constexpr class_template(auto&&...) noexcept {}
139+
};
140+
141+
template <class... Extents>
142+
class_template(Extents...) -> class_template<(true ? 0 : +Extents{})...>;
143+
144+
template <int... Extents>
145+
using alias_template = class_template<Extents...>;
146+
147+
alias_template var2{converible_to_one{}, 2};
148+
149+
}

0 commit comments

Comments
 (0)