Skip to content

Commit

Permalink
Always deduce the lengths of contained parameter packs when deducing a
Browse files Browse the repository at this point in the history
pack expansion.

Previously, if all parameter / argument pairs for a pack expansion
deduction were non-deduced contexts, we would not deduce the arity of
the pack, and could end up deducing a different arity (leading to
failures during substitution) or defaulting to an arity of 0 (leading to
bad diagnostics about passing the wrong number of arguments to a
variadic function). Instead, we now always deduce the arity for all
involved packs any time we deduce a pack expansion.

This will result in less substitution happening in some cases, which
could avoid non-SFINAEable errors, and should generally improve the
quality of diagnostics when passing initializer lists to variadic
functions.
  • Loading branch information
zygoloid committed Jan 7, 2020
1 parent e93b1ff commit 907cefe
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 34 deletions.
8 changes: 5 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -3875,12 +3875,14 @@ def note_ovl_candidate_bad_deduction : Note<
"candidate template ignored: failed template argument deduction">;
def note_ovl_candidate_incomplete_deduction : Note<"candidate template ignored: "
"couldn't infer template argument %0">;
def note_ovl_candidate_incomplete_deduction_pack : Note<"candidate template ignored: "
def note_ovl_candidate_incomplete_deduction_pack : Note<
"candidate template ignored: "
"deduced too few arguments for expanded pack %0; no argument for %ordinal1 "
"expanded parameter in deduced argument pack %2">;
def note_ovl_candidate_inconsistent_deduction : Note<
"candidate template ignored: deduced conflicting %select{types|values|"
"templates}0 for parameter %1%diff{ ($ vs. $)|}2,3">;
"candidate template ignored: deduced %select{conflicting types|"
"conflicting values|conflicting templates|packs of different lengths}0 "
"for parameter %1%diff{ ($ vs. $)|}2,3">;
def note_ovl_candidate_inconsistent_deduction_types : Note<
"candidate template ignored: deduced values %diff{"
"of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|"
Expand Down
16 changes: 16 additions & 0 deletions clang/lib/Sema/SemaOverload.cpp
Expand Up @@ -10348,6 +10348,16 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl *Found, Decl *Templated,
which = 2;
}

// Tweak the diagnostic if the problem is that we deduced packs of
// different arities. We'll print the actual packs anyway in case that
// includes additional useful information.
if (DeductionFailure.getFirstArg()->getKind() == TemplateArgument::Pack &&
DeductionFailure.getSecondArg()->getKind() == TemplateArgument::Pack &&
DeductionFailure.getFirstArg()->pack_size() !=
DeductionFailure.getSecondArg()->pack_size()) {
which = 3;
}

S.Diag(Templated->getLocation(),
diag::note_ovl_candidate_inconsistent_deduction)
<< which << ParamD->getDeclName() << *DeductionFailure.getFirstArg()
Expand Down Expand Up @@ -10385,6 +10395,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl *Found, Decl *Templated,
TemplateArgString = " ";
TemplateArgString += S.getTemplateArgumentBindingsText(
getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
if (TemplateArgString.size() == 1)
TemplateArgString.clear();
S.Diag(Templated->getLocation(),
diag::note_ovl_candidate_unsatisfied_constraints)
<< TemplateArgString;
Expand Down Expand Up @@ -10412,6 +10424,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl *Found, Decl *Templated,
TemplateArgString = " ";
TemplateArgString += S.getTemplateArgumentBindingsText(
getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
if (TemplateArgString.size() == 1)
TemplateArgString.clear();
}

// If this candidate was disabled by enable_if, say so.
Expand Down Expand Up @@ -10461,6 +10475,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl *Found, Decl *Templated,
TemplateArgString = " ";
TemplateArgString += S.getTemplateArgumentBindingsText(
getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
if (TemplateArgString.size() == 1)
TemplateArgString.clear();
}

S.Diag(Templated->getLocation(), diag::note_ovl_candidate_deduced_mismatch)
Expand Down
39 changes: 18 additions & 21 deletions clang/lib/Sema/SemaTemplateDeduction.cpp
Expand Up @@ -860,34 +860,31 @@ class PackDeductionScope {
/// Finish template argument deduction for a set of argument packs,
/// producing the argument packs and checking for consistency with prior
/// deductions.
Sema::TemplateDeductionResult
finish(bool TreatNoDeductionsAsNonDeduced = true) {
Sema::TemplateDeductionResult finish() {
// Build argument packs for each of the parameter packs expanded by this
// pack expansion.
for (auto &Pack : Packs) {
// Put back the old value for this pack.
Deduced[Pack.Index] = Pack.Saved;

// If we are deducing the size of this pack even if we didn't deduce any
// values for it, then make sure we build a pack of the right size.
// FIXME: Should we always deduce the size, even if the pack appears in
// a non-deduced context?
if (!TreatNoDeductionsAsNonDeduced)
Pack.New.resize(PackElements);
// Always make sure the size of this pack is correct, even if we didn't
// deduce any values for it.
//
// FIXME: This isn't required by the normative wording, but substitution
// and post-substitution checking will always fail if the arity of any
// pack is not equal to the number of elements we processed. (Either that
// or something else has gone *very* wrong.) We're permitted to skip any
// hard errors from those follow-on steps by the intent (but not the
// wording) of C++ [temp.inst]p8:
//
// If the function selected by overload resolution can be determined
// without instantiating a class template definition, it is unspecified
// whether that instantiation actually takes place
Pack.New.resize(PackElements);

// Build or find a new value for this pack.
DeducedTemplateArgument NewPack;
if (PackElements && Pack.New.empty()) {
if (Pack.DeferredDeduction.isNull()) {
// We were not able to deduce anything for this parameter pack
// (because it only appeared in non-deduced contexts), so just
// restore the saved argument pack.
continue;
}

NewPack = Pack.DeferredDeduction;
Pack.DeferredDeduction = TemplateArgument();
} else if (Pack.New.empty()) {
if (Pack.New.empty()) {
// If we deduced an empty argument pack, create it now.
NewPack = DeducedTemplateArgument(TemplateArgument::getEmptyPack());
} else {
Expand Down Expand Up @@ -2636,8 +2633,8 @@ static Sema::TemplateDeductionResult ConvertDeducedTemplateArguments(
// be deduced to an empty sequence of template arguments.
// FIXME: Where did the word "trailing" come from?
if (Deduced[I].isNull() && Param->isTemplateParameterPack()) {
if (auto Result = PackDeductionScope(S, TemplateParams, Deduced, Info, I)
.finish(/*TreatNoDeductionsAsNonDeduced*/false))
if (auto Result =
PackDeductionScope(S, TemplateParams, Deduced, Info, I).finish())
return Result;
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/CXX/drs/dr13xx.cpp
Expand Up @@ -348,9 +348,9 @@ namespace dr1388 { // dr1388: 4
// we know exactly how many arguments correspond to it.
template<typename T, typename U> struct pair {};
template<typename ...T> struct tuple { typedef char type; }; // expected-error 0-2{{C++11}}
template<typename ...T, typename ...U> void f_pair_1(pair<T, U>..., int); // expected-error 0-2{{C++11}} expected-note {{different lengths (2 vs. 0)}}
template<typename ...T, typename ...U> void f_pair_1(pair<T, U>..., int); // expected-error 0-2{{C++11}} expected-note {{[with T = <int, long>]: deduced incomplete pack <(no value), (no value)> for template parameter 'U'}}
template<typename ...T, typename U> void f_pair_2(pair<T, char>..., U); // expected-error 0-2{{C++11}}
template<typename ...T, typename ...U> void f_pair_3(pair<T, U>..., tuple<U...>); // expected-error 0-2{{C++11}} expected-note {{different lengths (2 vs. 1)}}
template<typename ...T, typename ...U> void f_pair_3(pair<T, U>..., tuple<U...>); // expected-error 0-2{{C++11}} expected-note {{deduced packs of different lengths for parameter 'U' (<(no value), (no value)> vs. <char>)}}
template<typename ...T> void f_pair_4(pair<T, char>..., T...); // expected-error 0-2{{C++11}} expected-note {{<int, long> vs. <int, long, const char *>}}
void g(pair<int, char> a, pair<long, char> b, tuple<char, char> c) {
f_pair_1<int, long>(a, b, 0); // expected-error {{no match}}
Expand Down
Expand Up @@ -35,3 +35,34 @@ void (*ptr_has_non_trailing_pack)(char, int) = has_non_trailing_pack<char>;
template<typename ...T, typename U> void has_non_trailing_pack_and_more(T ..., U); // expected-note {{failed}}
void (*ptr_has_non_trailing_pack_and_more_1)(float, double, int) = &has_non_trailing_pack_and_more<float, double>;
void (*ptr_has_non_trailing_pack_and_more_2)(float, double, int) = &has_non_trailing_pack_and_more<float>; // expected-error {{does not match}}

// - A function parameter for which the associated argument is an initializer
// list but the parameter does not have a type for which deduction from an
// initializer list is specified.

// We interpret these "non-deduced context"s as actually deducing the arity --
// but not the contents -- of a function parameter pack appropriately for the
// number of arguments.
namespace VariadicVsInitList {
template<typename T, typename ...> struct X { using type = typename T::error; };
template<typename ...T, typename X<int, T...>::type = 0> void f(T ...) = delete;
void f(long);
void f(long, long);
void f(long, long, long);

// FIXME: We shouldn't say "substitution failure: " here.
template<typename ...T> void g(T ...) = delete; // expected-note {{substitution failure: deduced incomplete pack <(no value)> for template parameter 'T'}}

void h() {
// These all call the non-template overloads of 'f', because of a deduction
// failure due to incomplete deduction of the pack 'T'. If deduction
// succeeds and deduces an empty pack instead, we would get a hard error
// instantiating 'X'.
f({0}); // expected-warning {{braces around scalar}}
f({0}, {0}); // expected-warning 2{{braces around scalar}}
f(1, {0}); // expected-warning {{braces around scalar}}
f(1, {0}, 2); // expected-warning {{braces around scalar}}

g({0}); // expected-error {{no matching function}}
}
}
6 changes: 2 additions & 4 deletions clang/test/SemaTemplate/alias-templates.cpp
Expand Up @@ -77,14 +77,12 @@ namespace PR11848 {
return i + f1<Ts...>(is...);
}

// FIXME: This note is technically correct, but could be better. We
// should really say that we couldn't infer template argument 'Ts'.
template<typename ...Ts>
void f2(U<Ts> ...is) { } // expected-note {{requires 0 arguments, but 1 was provided}}
void f2(U<Ts> ...is) { } // expected-note {{deduced incomplete pack <(no value)> for template parameter 'Ts'}}

template<typename...> struct type_tuple {};
template<typename ...Ts>
void f3(type_tuple<Ts...>, U<Ts> ...is) {} // expected-note {{requires 4 arguments, but 3 were provided}}
void f3(type_tuple<Ts...>, U<Ts> ...is) {} // expected-note {{deduced packs of different lengths for parameter 'Ts' (<void, void, void> vs. <(no value), (no value)>)}}

void g() {
f1(U<void>()); // expected-error {{no match}}
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaTemplate/deduction.cpp
Expand Up @@ -365,7 +365,7 @@ namespace deduction_after_explicit_pack {
int test(ExtraArgs..., unsigned vla_size, const char *input);
int n = test(0, "");

template <typename... T> void i(T..., int, T..., ...); // expected-note 5{{deduced conflicting}}
template <typename... T> void i(T..., int, T..., ...); // expected-note 5{{deduced packs of different lengths}}
void j() {
i(0);
i(0, 1); // expected-error {{no match}}
Expand All @@ -381,7 +381,7 @@ namespace deduction_after_explicit_pack {
// parameter against the first argument, then passing the first argument
// through the first parameter.
template<typename... T> struct X { X(int); operator int(); };
template<typename... T> void p(T..., X<T...>, ...); // expected-note {{deduced conflicting}}
template<typename... T> void p(T..., X<T...>, ...); // expected-note {{deduced packs of different lengths for parameter 'T' (<> vs. <int>)}}
void q() { p(X<int>(0), 0); } // expected-error {{no match}}

struct A {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaTemplate/pack-deduction.cpp
Expand Up @@ -5,8 +5,8 @@ template<typename ...T> struct X {};
template<typename T, typename U> struct P {};

namespace Nested {
template<typename ...T> int f1(X<T, T...>... a); // expected-note +{{conflicting types for parameter 'T'}}
template<typename ...T> int f2(P<X<T...>, T> ...a); // expected-note +{{conflicting types for parameter 'T'}}
template<typename ...T> int f1(X<T, T...>... a); // expected-note +{{packs of different lengths for parameter 'T'}}
template<typename ...T> int f2(P<X<T...>, T> ...a); // expected-note +{{packs of different lengths for parameter 'T'}}

int a1 = f1(X<int, int, double>(), X<double, int, double>());
int a2 = f1(X<int, int>());
Expand Down

0 comments on commit 907cefe

Please sign in to comment.