[clang] 907cefe - Always deduce the lengths of contained parameter packs when deducing a

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 6 17:26:02 PST 2020


Author: Richard Smith
Date: 2020-01-06T17:24:29-08:00
New Revision: 907cefe721437fa8950c1b6c1c028038b175f921

URL: https://github.com/llvm/llvm-project/commit/907cefe721437fa8950c1b6c1c028038b175f921
DIFF: https://github.com/llvm/llvm-project/commit/907cefe721437fa8950c1b6c1c028038b175f921.diff

LOG: Always deduce the lengths of contained parameter packs when deducing a
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.

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaOverload.cpp
    clang/lib/Sema/SemaTemplateDeduction.cpp
    clang/test/CXX/drs/dr13xx.cpp
    clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp
    clang/test/SemaTemplate/alias-templates.cpp
    clang/test/SemaTemplate/deduction.cpp
    clang/test/SemaTemplate/pack-deduction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 99ce42dd7533..a8f49fef9f1f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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 
diff erent 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 $)|"

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 74a0bc7c78ff..83b7f497f99d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -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
+    // 
diff erent 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()
@@ -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;
@@ -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.
@@ -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)

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 822ea12246a9..521160d1ad23 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -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 {
@@ -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;
     }
 

diff  --git a/clang/test/CXX/drs/dr13xx.cpp b/clang/test/CXX/drs/dr13xx.cpp
index 3a0b7d7ed4bc..2373a3967ddc 100644
--- a/clang/test/CXX/drs/dr13xx.cpp
+++ b/clang/test/CXX/drs/dr13xx.cpp
@@ -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 {{
diff erent 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 {{
diff erent 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 
diff erent 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}}

diff  --git a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp
index 697412995b33..2d9896b90adf 100644
--- a/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp
+++ b/clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp
@@ -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}}
+  }
+}

diff  --git a/clang/test/SemaTemplate/alias-templates.cpp b/clang/test/SemaTemplate/alias-templates.cpp
index 240a6eeff2e7..80678bf22985 100644
--- a/clang/test/SemaTemplate/alias-templates.cpp
+++ b/clang/test/SemaTemplate/alias-templates.cpp
@@ -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 
diff erent lengths for parameter 'Ts' (<void, void, void> vs. <(no value), (no value)>)}}
 
   void g() {
     f1(U<void>()); // expected-error {{no match}}

diff  --git a/clang/test/SemaTemplate/deduction.cpp b/clang/test/SemaTemplate/deduction.cpp
index 64fef8e5f522..1f1c30a8b4ab 100644
--- a/clang/test/SemaTemplate/deduction.cpp
+++ b/clang/test/SemaTemplate/deduction.cpp
@@ -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 
diff erent lengths}}
   void j() {
     i(0);
     i(0, 1); // expected-error {{no match}}
@@ -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 
diff erent lengths for parameter 'T' (<> vs. <int>)}}
   void q() { p(X<int>(0), 0); } // expected-error {{no match}}
 
   struct A {

diff  --git a/clang/test/SemaTemplate/pack-deduction.cpp b/clang/test/SemaTemplate/pack-deduction.cpp
index 478b19731b67..6368f16ebe8b 100644
--- a/clang/test/SemaTemplate/pack-deduction.cpp
+++ b/clang/test/SemaTemplate/pack-deduction.cpp
@@ -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 
diff erent lengths for parameter 'T'}}
+  template<typename ...T> int f2(P<X<T...>, T> ...a); // expected-note +{{packs of 
diff erent lengths for parameter 'T'}}
 
   int a1 = f1(X<int, int, double>(), X<double, int, double>());
   int a2 = f1(X<int, int>());


        


More information about the cfe-commits mailing list