On Thu, 2025-01-09 at 21:15 -0500, Jason Merrill wrote:
> On 1/9/25 7:00 PM, David Malcolm wrote:
> > On Thu, 2025-01-09 at 14:21 -0500, Jason Merrill wrote:
> >
> > Thanks for taking a look...
> >
> > > > On 1/9/25 2:11 PM, David Malcolm wrote:
> > > >
> > > > @@ -4743,7 +4769,38 @@ convert_arguments (tree typelist,
> > > > vec<tree,
> > > > va_gc> **values, tree fndecl,
> > > > if (typetail && typetail != void_list_node)
> > > > {
> > > > if (complain & tf_error)
> > > > - error_args_num (input_location, fndecl,
> > > > /*too_many_p=*/false);
> > > > + {
> > > > + /* Not enough args.
> > > > + Determine minimum number of arguments
> > > > required. */
> > > > + int min_expected_num = 0;
> > > > + bool at_least_p = false;
> > > > + tree iter = typelist;
> > > > + while (true)
> > > > + {
> > > > + if (!iter)
> > > > + {
> > > > + /* Variadic arguments; stop iterating.
> > > > */
> > > > + at_least_p = true;
> > > > + break;
> > > > + }
> > > > + if (iter == void_list_node)
> > > > + /* End of arguments; stop iterating. */
> > > > + break;
> > > > + if (fndecl && TREE_PURPOSE (iter)
> > > > + && TREE_CODE (TREE_PURPOSE (iter)) !=
> > > > DEFERRED_PARSE)
> > > >
> > >
> > > Why are you checking DEFERRED_PARSE? That indicates a default
> > > argument,
> > > even if it isn't parsed yet. For that case we should get the
> > > error
> > > in
> > > convert_default_arg rather than pretend there's no default
> > > argument.
> >
> > I confess that the check for DEFERRED_PARSE was a rather mindless
> > copy
> > and paste by me from the "See if there are default arguments that
> > can be
> > used" logic earlier in the function.
> >
> > I've removed it in the latest version of the patch.
> >
> > > > + {
> > > > + /* Found a default argument; skip this
> > > > one when
> > > > + counting minimum required. */
> > > > + at_least_p = true;
> > > > + iter = TREE_CHAIN (iter);
> > > > + continue;
> > >
> > > We could break here, once you have a default arg the rest of the
> > > parms
> > > need to have them as well.
> >
> > Indeed; I've updated this in the latest version of the patch, so
> > we break out as soon as we see an arg with a non-null TREE_PURPOSE.
> >
> > >
> > > > + }
> > > > + ++min_expected_num;
> > > > + iter = TREE_CHAIN (iter);
> > > > + }
> > > > + error_args_num (input_location, fndecl,
> > > > + min_expected_num, actual_num,
> > > > at_least_p);
> > > > + }
> > > > return -1;
> > > > }
> >
> > Here's a v3 version of the patch, which is currently going through
> > my tester.
> >
> > OK for trunk if it passes bootstrap®rtesting?
>
> OK.
Thanks. However, it turns out that I may have misspoke, and the v2
patch might have been the correct approach: if we bail out on the first
arg with a TREE_PURPOSE then in
gcc/testsuite/g++.dg/cpp0x/variadic169.C
1 │ // DR 2233
2 │ // { dg-do compile { target c++11 } }
3 │
4 │ template<typename ...T> void f(int n = 0, T ...t);
5 │
6 │ int main()
7 │ {
8 │ f<int>(); // { dg-error "too few arguments to
function '\[^\n\r\]*'; expected at least 1, have 0" }
9 │ }
we instead emit the nonsensical "expected at least 0, have 0":
error: too few arguments to function ‘void f(int, T ...) [with T =
{int}]’; expected at least 0, have 0
8 | f<int>(); // { dg-error "too few
arguments to function '\[^\n\r\]*'; expected at least 1, have 0" }
| ~~~~~~^~
../../src/gcc/testsuite/g++.dg/cpp0x/variadic169.C:4:30: note: declared
here
4 | template<typename ...T> void f(int n = 0, T ...t);
| ^
whereas with the v2 patch we count the trailing arg after the default
arg, and we emit "expected at least 1, have 0", which seems correct to
me.
I'm testing a version of the patch that continues to iterate after a
TREE_PURPOSE (as v2, but but dropping the check on DEFERRED_PARSE).
Thanks
Dave
