| Message ID | 53D7AF73.2020507@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 07/29/2014 10:28 AM, Paolo Carlini wrote: > +unify_arity (bool explain_p, int have, int wanted, bool lb_p = false) I don't understand "lb_p". > @@ -16598,6 +16608,8 @@ type_unification_real (tree tparms, > tree argvec; > tree parmvec = make_tree_vec (1); > > + remaining_pack_p = true; > + > /* Allocate a TREE_VEC and copy in all of the arguments */ > argvec = make_tree_vec (nargs - ia); > for (i = 0; ia < nargs; ++ia, ++i) Why would we get here in the too few args case? Won't we only hit this code if we had enough args for the non-pack parms? Jason
Hi, On 07/29/2014 06:01 PM, Jason Merrill wrote: > On 07/29/2014 10:28 AM, Paolo Carlini wrote: >> +unify_arity (bool explain_p, int have, int wanted, bool lb_p = false) > > I don't understand "lb_p". lower_bound ;) The first name which came to my mind... > >> @@ -16598,6 +16608,8 @@ type_unification_real (tree tparms, >> tree argvec; >> tree parmvec = make_tree_vec (1); >> >> + remaining_pack_p = true; >> + >> /* Allocate a TREE_VEC and copy in all of the arguments */ >> argvec = make_tree_vec (nargs - ia); >> for (i = 0; ia < nargs; ++ia, ++i) > > Why would we get here in the too few args case? Won't we only hit > this code if we had enough args for the non-pack parms? Yeah, that is exactly for the cases I was mentioning at the end of the my first message, eg: template<class T1, class... T2, class... T3> void boo(T1, T2..., T3...) { } int main() { boo(1); } the patch as-is leads to a diagnostic similar to the current one, thus saying: "candidate expects at least 2 arguments, 1 provided" whereas the current one says: "candidate expects 2 arguments, 1 provided". That seems fine, given the current status, because indeed, passing: int main() { boo(1, 2); } or, for that matter: int main() { boo(1, 2, 3); } are both accepted. Paolo.
On 07/29/2014 12:23 PM, Paolo Carlini wrote: > On 07/29/2014 06:01 PM, Jason Merrill wrote: >> Why would we get here in the too few args case? Won't we only hit >> this code if we had enough args for the non-pack parms? > Yeah, that is exactly for the cases I was mentioning at the end of the > my first message, eg: > > template<class T1, class... T2, class... T3> > void boo(T1, T2..., T3...) > { } > > int main() > { > boo(1); > } Ah, right. And as you mention, we ought to accept that; it seems a bit odd to change the code to give a different wrong error instead of the current wrong error. :) Jason
Hi, On 07/29/2014 06:56 PM, Jason Merrill wrote: > On 07/29/2014 12:23 PM, Paolo Carlini wrote: >> On 07/29/2014 06:01 PM, Jason Merrill wrote: >>> Why would we get here in the too few args case? Won't we only hit >>> this code if we had enough args for the non-pack parms? >> Yeah, that is exactly for the cases I was mentioning at the end of the >> my first message, eg: >> >> template<class T1, class... T2, class... T3> >> void boo(T1, T2..., T3...) >> { } >> >> int main() >> { >> boo(1); >> } > > Ah, right. And as you mention, we ought to accept that; it seems a > bit odd to change the code to give a different wrong error instead of > the current wrong error. :) Yeah, it is, but then we aren't really handling c++/57397, IMHO. I bet there is even a different bug report or a defect report for this second issue. I'll see what I can do, but I have no idea how difficult it will turn out to be... Paolo.
Index: cp/pt.c =================================================================== --- cp/pt.c (revision 213123) +++ cp/pt.c (working copy) @@ -5517,13 +5517,21 @@ unify_method_type_error (bool explain_p, tree arg) } static int -unify_arity (bool explain_p, int have, int wanted) +unify_arity (bool explain_p, int have, int wanted, bool lb_p = false) { if (explain_p) - inform_n (input_location, wanted, - " candidate expects %d argument, %d provided", - " candidate expects %d arguments, %d provided", - wanted, have); + { + if (lb_p) + inform_n (input_location, wanted, + " candidate expects at least %d argument, %d provided", + " candidate expects at least %d arguments, %d provided", + wanted, have); + else + inform_n (input_location, wanted, + " candidate expects %d argument, %d provided", + " candidate expects %d arguments, %d provided", + wanted, have); + } return 1; } @@ -5534,9 +5542,10 @@ unify_too_many_arguments (bool explain_p, int have } static int -unify_too_few_arguments (bool explain_p, int have, int wanted) +unify_too_few_arguments (bool explain_p, int have, int wanted, + bool lb_p = false) { - return unify_arity (explain_p, have, wanted); + return unify_arity (explain_p, have, wanted, lb_p); } static int @@ -16546,6 +16555,7 @@ type_unification_real (tree tparms, const tree *args; unsigned int nargs; unsigned int ia; + bool remaining_pack_p = false; gcc_assert (TREE_CODE (tparms) == TREE_VEC); gcc_assert (xparms == NULL_TREE || TREE_CODE (xparms) == TREE_LIST); @@ -16598,6 +16608,8 @@ type_unification_real (tree tparms, tree argvec; tree parmvec = make_tree_vec (1); + remaining_pack_p = true; + /* Allocate a TREE_VEC and copy in all of the arguments */ argvec = make_tree_vec (nargs - ia); for (i = 0; ia < nargs; ++ia, ++i) @@ -16622,13 +16634,20 @@ type_unification_real (tree tparms, && TREE_PURPOSE (parms) == NULL_TREE) { unsigned int count = nargs; + unsigned int excess_packs_count = 0; tree p = parms; while (p && p != void_list_node) { count++; + excess_packs_count + += TREE_CODE (TREE_VALUE (p)) == TYPE_PACK_EXPANSION; p = TREE_CHAIN (p); } - return unify_too_few_arguments (explain_p, ia, count); + + if (excess_packs_count) + count -= excess_packs_count - remaining_pack_p; + return unify_too_few_arguments (explain_p, ia, count, + excess_packs_count); } if (!subr) Index: testsuite/g++.dg/cpp0x/vt-57397.C =================================================================== --- testsuite/g++.dg/cpp0x/vt-57397.C (revision 0) +++ testsuite/g++.dg/cpp0x/vt-57397.C (working copy) @@ -0,0 +1,16 @@ +// PR c++/57397 +// { dg-do compile { target c++11 } } + +template<class T1, class... Tn> +void foo(T1, Tn...); + +template<class T1, class T2, class... Tn> +void bar(T1, T2, Tn...); + +int main() +{ + foo(); // { dg-error "no matching" } + // { dg-message "candidate expects at least 1 argument, 0 provided" "" { target *-*-* } 12 } + bar(1); // { dg-error "no matching" } + // { dg-message "candidate expects at least 2 arguments, 1 provided" "" { target *-*-* } 14 } +}
Hi, in this diagnostic issue Jon noticed that for testcases like: template<class T1, class... Tn> void foo(T1, Tn...); int main() { foo(); } we provide diagnostic saying "candidate expects 2 arguments, 0 provided", whereas of course we want to say something like "candidate expects at least 1 argument, 0 provided". The below simply tries to refine the existing logic in type_unification_real. Note that while handling this bug, I noticed that we don't appear to be up to date wrt cases like: template<class T1, class... Tn, class... Un> void bar(T1, Tn..., Un...); int main() { bar(1); } which we do reject and very recently clang started accepting. On the other hand we accept: int main() { bar(1, 2); } which clang rejects. My patch doesn't even try to cope with that (do be honest, I didn't lookup the relevant DRs and resolutions), that seems to me an unrelated issue, definitely not just a diagnostic one. Thanks, Paolo. /////////////////////// /cp 2014-07-29 Paolo Carlini <paolo.carlini@oracle.com> PR c++/57397 * pt.c (unify_arity): Add boolean parameter. (unify_too_few_arguments): Likewise. (type_unification_real): Diagnose correctly insufficient arguments in the presence of trailing variadic parameters. /testsuite 2014-07-29 Paolo Carlini <paolo.carlini@oracle.com> PR c++/57397 * g++.dg/cpp0x/vt-57397.C: New.