Skip to content

Commit 7fcba00

Browse files
spaitscor3ntinerichkeane
authored
[Sema] Substitute parameter packs when deduced from function arguments (#79371)
This pull request would solve #78449 . There is also a discussion about this on stackoverflow: https://stackoverflow.com/questions/77832658/stdtype-identity-to-support-several-variadic-argument-lists . The following program is well formed: ```cpp #include <type_traits> template <typename... T> struct args_tag { using type = std::common_type_t<T...>; }; template <typename... T> void bar(args_tag<T...>, std::type_identity_t<T>..., int, std::type_identity_t<T>...) {} // example int main() { bar(args_tag<int, int>{}, 4, 8, 15, 16, 23); } ``` but Clang rejects it, while GCC and MSVC doesn't. The reason for this is that, in `Sema::DeduceTemplateArguments` we are not prepared for this case. # Substitution/deduction of parameter packs The logic that handles substitution when we have explicit template arguments (`SubstituteExplicitTemplateArguments`) does not work here, since the types of the pack are not pushed to `ParamTypes` before the loop starts that does the deduction. The other "candidate" that may could have handle this case would be the loop that does the deduction for trailing packs, but we are not dealing with trailing packs here. # Solution proposed in this PR The solution proposed in this PR works similar to the trailing pack deduction. The main difference here is the end of the deduction cycle. When a non-trailing template pack argument is found, whose type is not explicitly specified and the next type is not a pack type, the length of the previously deduced pack is retrieved (let that length be `s`). After that the next `s` arguments are processed in the same way as in the case of non trailing packs. # Another possible solution There is another possible approach that would be less efficient. In the loop when we get to an element of `ParamTypes` that is a pack and could be substituted because the type is deduced from a previous argument, then `s` number of arg types would be inserted before the current element of `ParamTypes` type. Then we would "cancel" the processing of the current element, first process the previously inserted elements and the after that re-process the current element. Basically we would do what `SubstituteExplicitTemplateArguments` does but during deduction. # Adjusted test cases In `clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp` there is a test case named `test_pack_not_at_end` that should work, but still does not. This test case is relevant because the note for the error message has changed. This is what the test case looks like currently: ```cpp template<typename ...Types> void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <int, int>}} void test_pack_not_at_end(tuple<int*, double*> t2) { pack_not_at_end(t2, 0, 0, 0); // expected-error {{no match}} // FIXME: Should the "original argument type must match deduced parameter // type" rule apply here? pack_not_at_end<int*, double*>(t2, 0, 0, 0); // ok } ``` The previous note said (before my changes): ``` deduced conflicting types for parameter 'Types' (<int *, double *> vs. <>) ```` The current note says (after my changesand also clang 14 would say this if the pack was not trailing): ``` deduced conflicting types for parameter 'Types' (<int *, double *> vs. <int, int>) ``` GCC says: ``` error: no matching function for call to ‘pack_not_at_end(std::tuple<int*, double*>&, int, int, int)’ 70 | pack_not_at_end(t2, 0, 0, 9); // expected-error {{no match}} ```` --------- Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Erich Keane <[email protected]>
1 parent ad1a65f commit 7fcba00

File tree

4 files changed

+85
-1
lines changed

4 files changed

+85
-1
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ C++2c Feature Support
7373

7474
Resolutions to C++ Defect Reports
7575
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
76+
- Substitute template parameter pack, when it is not explicitly specified
77+
in the template parameters, but is deduced from a previous argument.
78+
(`#78449: <https://github.com/llvm/llvm-project/issues/78449>`_).
7679

7780
C Language Changes
7881
------------------

clang/lib/Sema/SemaTemplateDeduction.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,7 @@ class PackDeductionScope {
731731
void addPack(unsigned Index) {
732732
// Save the deduced template argument for the parameter pack expanded
733733
// by this pack expansion, then clear out the deduction.
734+
DeducedFromEarlierParameter = !Deduced[Index].isNull();
734735
DeducedPack Pack(Index);
735736
Pack.Saved = Deduced[Index];
736737
Deduced[Index] = TemplateArgument();
@@ -859,6 +860,23 @@ class PackDeductionScope {
859860
Info.PendingDeducedPacks[Pack.Index] = Pack.Outer;
860861
}
861862

863+
// Return the size of the saved packs if all of them has the same size.
864+
std::optional<unsigned> getSavedPackSizeIfAllEqual() const {
865+
unsigned PackSize = Packs[0].Saved.pack_size();
866+
867+
if (std::all_of(Packs.begin() + 1, Packs.end(), [&PackSize](const auto &P) {
868+
return P.Saved.pack_size() == PackSize;
869+
}))
870+
return PackSize;
871+
return {};
872+
}
873+
874+
/// Determine whether this pack has already been deduced from a previous
875+
/// argument.
876+
bool isDeducedFromEarlierParameter() const {
877+
return DeducedFromEarlierParameter;
878+
}
879+
862880
/// Determine whether this pack has already been partially expanded into a
863881
/// sequence of (prior) function parameters / template arguments.
864882
bool isPartiallyExpanded() { return IsPartiallyExpanded; }
@@ -1004,6 +1022,7 @@ class PackDeductionScope {
10041022
unsigned PackElements = 0;
10051023
bool IsPartiallyExpanded = false;
10061024
bool DeducePackIfNotAlreadyDeduced = false;
1025+
bool DeducedFromEarlierParameter = false;
10071026
/// The number of expansions, if we have a fully-expanded pack in this scope.
10081027
std::optional<unsigned> FixedNumExpansions;
10091028

@@ -4381,6 +4400,34 @@ Sema::TemplateDeductionResult Sema::DeduceTemplateArguments(
43814400
// corresponding argument is a list?
43824401
PackScope.nextPackElement();
43834402
}
4403+
} else if (!IsTrailingPack && !PackScope.isPartiallyExpanded() &&
4404+
PackScope.isDeducedFromEarlierParameter()) {
4405+
// [temp.deduct.general#3]
4406+
// When all template arguments have been deduced
4407+
// or obtained from default template arguments, all uses of template
4408+
// parameters in the template parameter list of the template are
4409+
// replaced with the corresponding deduced or default argument values
4410+
//
4411+
// If we have a trailing parameter pack, that has been deduced
4412+
// previously we substitute the pack here in a similar fashion as
4413+
// above with the trailing parameter packs. The main difference here is
4414+
// that, in this case we are not processing all of the remaining
4415+
// arguments. We are only process as many arguments as we have in
4416+
// the already deduced parameter.
4417+
std::optional<unsigned> ArgPosAfterSubstitution =
4418+
PackScope.getSavedPackSizeIfAllEqual();
4419+
if (!ArgPosAfterSubstitution)
4420+
continue;
4421+
4422+
unsigned PackArgEnd = ArgIdx + *ArgPosAfterSubstitution;
4423+
for (; ArgIdx < PackArgEnd && ArgIdx < Args.size(); ArgIdx++) {
4424+
ParamTypesForArgChecking.push_back(ParamPattern);
4425+
if (auto Result = DeduceCallArgument(ParamPattern, ArgIdx,
4426+
/*ExplicitObjetArgument=*/false))
4427+
return Result;
4428+
4429+
PackScope.nextPackElement();
4430+
}
43844431
}
43854432
}
43864433

clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ void test_pair_deduction(int *ip, float *fp, double *dp) {
8181
template<typename ...Types> struct tuple { };
8282

8383
template<typename ...Types>
84-
void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <>}}
84+
void pack_not_at_end(tuple<Types...>, Types... values, int); // expected-note {{<int *, double *> vs. <int, int>}}
8585

8686
void test_pack_not_at_end(tuple<int*, double*> t2) {
8787
pack_not_at_end(t2, 0, 0, 0); // expected-error {{no match}}

clang/test/SemaTemplate/deduction.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@ struct X0<int, A> {
1313
static const unsigned value = 1;
1414
};
1515

16+
template<class T>
17+
struct type_identity {
18+
using type = T;
19+
};
20+
21+
template<class T>
22+
using type_identity_t = typename type_identity<T>::type;
23+
24+
template <typename... T>
25+
struct args_tag {};
26+
1627
template<int> struct X0i;
1728
template<long> struct X0l;
1829
int array_x0a[X0<long, X0l>::value == 0? 1 : -1];
@@ -431,6 +442,29 @@ namespace deduction_after_explicit_pack {
431442
i<int, int>(0, 1, 2, 3, 4, 5); // expected-error {{no match}}
432443
}
433444

445+
template <typename... T>
446+
void bar(args_tag<T...>, type_identity_t<T>..., int mid, type_identity_t<T>...) {}
447+
void call_bar() {
448+
bar(args_tag<int, int>{}, 4, 8, 1001, 16, 23);
449+
}
450+
451+
template <typename... Y, typename... T>
452+
void foo(args_tag<Y...>, args_tag<T...>, type_identity_t<T>..., int mid, type_identity_t<T>...) {}
453+
void call_foo() {
454+
foo(args_tag<const int,const int, const int>{}, args_tag<int, int, int>{}, 4, 8, 9, 15, 16, 23, 1);
455+
}
456+
457+
template <typename... Y, typename... T>
458+
void foo2(args_tag<Y...>, args_tag<T...>, type_identity_t<T>..., type_identity_t<T>...) {}
459+
void call_foo2() {
460+
foo2(args_tag<const int,const int, const int>{}, args_tag<int, int, int>{}, 4, 8, 9, 15, 16, 23);
461+
}
462+
463+
template <typename... Y, typename... T> void baz(args_tag<T...>, T..., T...) {}
464+
void call_baz() {
465+
baz(args_tag<int, int>{}, 1, 2, 3, 4);
466+
}
467+
434468
// GCC alarmingly accepts this by deducing T={int} by matching the second
435469
// parameter against the first argument, then passing the first argument
436470
// through the first parameter.

0 commit comments

Comments
 (0)