c++: use auto_vec in cp_parser_template_argument_list

  Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

gcc/cp/ChangeLog:

	* parser.c (cp_parser_template_argument_list): Use auto_vec
	instead of manual memory management.
---
 gcc/cp/parser.c | 35 ++++++++---------------------------
 1 file changed, 8 insertions(+), 27 deletions(-)
  

On 11/9/21 11:02, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK, though I wonder about using releasing_vec instead of auto_vec; 
reusing a previously allocated vec vs. building one on the stack.
> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_parser_template_argument_list): Use auto_vec
> 	instead of manual memory management.
> ---
>   gcc/cp/parser.c | 35 ++++++++---------------------------
>   1 file changed, 8 insertions(+), 27 deletions(-)
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 32de97b08bd..8823399529e 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -18558,11 +18558,6 @@ cp_parser_template_name (cp_parser* parser,
>   static tree
>   cp_parser_template_argument_list (cp_parser* parser)
>   {
> -  tree fixed_args[10];
> -  unsigned n_args = 0;
> -  unsigned alloced = 10;
> -  tree *arg_ary = fixed_args;
> -  tree vec;
>     bool saved_in_template_argument_list_p;
>     bool saved_ice_p;
>     bool saved_non_ice_p;
> @@ -18581,16 +18576,15 @@ cp_parser_template_argument_list (cp_parser* parser)
>     parser->non_integral_constant_expression_p = false;
>   
>     /* Parse the arguments.  */
> +  auto_vec<tree, 10> args;
>     do
>       {
> -      tree argument;
> -
> -      if (n_args)
> +      if (!args.is_empty ())
>   	/* Consume the comma.  */
>   	cp_lexer_consume_token (parser->lexer);
>   
>         /* Parse the template-argument.  */
> -      argument = cp_parser_template_argument (parser);
> +      tree argument = cp_parser_template_argument (parser);
>   
>         /* If the next token is an ellipsis, we're expanding a template
>            argument pack. */
> @@ -18610,29 +18604,16 @@ cp_parser_template_argument_list (cp_parser* parser)
>             argument = make_pack_expansion (argument);
>           }
>   
> -      if (n_args == alloced)
> -	{
> -	  alloced *= 2;
> -
> -	  if (arg_ary == fixed_args)
> -	    {
> -	      arg_ary = XNEWVEC (tree, alloced);
> -	      memcpy (arg_ary, fixed_args, sizeof (tree) * n_args);
> -	    }
> -	  else
> -	    arg_ary = XRESIZEVEC (tree, arg_ary, alloced);
> -	}
> -      arg_ary[n_args++] = argument;
> +      args.safe_push (argument);
>       }
>     while (cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
>   
> -  vec = make_tree_vec (n_args);
> +  int n_args = args.length ();
> +  tree vec = make_tree_vec (n_args);
>   
> -  while (n_args--)
> -    TREE_VEC_ELT (vec, n_args) = arg_ary[n_args];
> +  for (int i = 0; i < n_args; i++)
> +    TREE_VEC_ELT (vec, i) = args[i];
>   
> -  if (arg_ary != fixed_args)
> -    free (arg_ary);
>     parser->non_integral_constant_expression_p = saved_non_ice_p;
>     parser->integral_constant_expression_p = saved_ice_p;
>     parser->in_template_argument_list_p = saved_in_template_argument_list_p;
>
  

On Tue, 9 Nov 2021, Jason Merrill wrote:

> On 11/9/21 11:02, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> OK, though I wonder about using releasing_vec instead of auto_vec; reusing a
> previously allocated vec vs. building one on the stack.

Thanks a lot.  And hmm, I think using reusing a previously allocated vec
here would be tricky since cp_parser_template_argument_list can be
called recursively, and so only the outermost call would be able to
benefit from reuse unless we perhaps maintain a freelist of such vecs
IIUC.  Seems like that would complicate the code enough to not be worth
it.

> > gcc/cp/ChangeLog:
> > 
> > 	* parser.c (cp_parser_template_argument_list): Use auto_vec
> > 	instead of manual memory management.
> > ---
> >   gcc/cp/parser.c | 35 ++++++++---------------------------
> >   1 file changed, 8 insertions(+), 27 deletions(-)
> > 
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 32de97b08bd..8823399529e 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -18558,11 +18558,6 @@ cp_parser_template_name (cp_parser* parser,
> >   static tree
> >   cp_parser_template_argument_list (cp_parser* parser)
> >   {
> > -  tree fixed_args[10];
> > -  unsigned n_args = 0;
> > -  unsigned alloced = 10;
> > -  tree *arg_ary = fixed_args;
> > -  tree vec;
> >     bool saved_in_template_argument_list_p;
> >     bool saved_ice_p;
> >     bool saved_non_ice_p;
> > @@ -18581,16 +18576,15 @@ cp_parser_template_argument_list (cp_parser*
> > parser)
> >     parser->non_integral_constant_expression_p = false;
> >       /* Parse the arguments.  */
> > +  auto_vec<tree, 10> args;
> >     do
> >       {
> > -      tree argument;
> > -
> > -      if (n_args)
> > +      if (!args.is_empty ())
> >   	/* Consume the comma.  */
> >   	cp_lexer_consume_token (parser->lexer);
> >           /* Parse the template-argument.  */
> > -      argument = cp_parser_template_argument (parser);
> > +      tree argument = cp_parser_template_argument (parser);
> >           /* If the next token is an ellipsis, we're expanding a template
> >            argument pack. */
> > @@ -18610,29 +18604,16 @@ cp_parser_template_argument_list (cp_parser*
> > parser)
> >             argument = make_pack_expansion (argument);
> >           }
> >   -      if (n_args == alloced)
> > -	{
> > -	  alloced *= 2;
> > -
> > -	  if (arg_ary == fixed_args)
> > -	    {
> > -	      arg_ary = XNEWVEC (tree, alloced);
> > -	      memcpy (arg_ary, fixed_args, sizeof (tree) * n_args);
> > -	    }
> > -	  else
> > -	    arg_ary = XRESIZEVEC (tree, arg_ary, alloced);
> > -	}
> > -      arg_ary[n_args++] = argument;
> > +      args.safe_push (argument);
> >       }
> >     while (cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
> >   -  vec = make_tree_vec (n_args);
> > +  int n_args = args.length ();
> > +  tree vec = make_tree_vec (n_args);
> >   -  while (n_args--)
> > -    TREE_VEC_ELT (vec, n_args) = arg_ary[n_args];
> > +  for (int i = 0; i < n_args; i++)
> > +    TREE_VEC_ELT (vec, i) = args[i];
> >   -  if (arg_ary != fixed_args)
> > -    free (arg_ary);
> >     parser->non_integral_constant_expression_p = saved_non_ice_p;
> >     parser->integral_constant_expression_p = saved_ice_p;
> >     parser->in_template_argument_list_p = saved_in_template_argument_list_p;
> > 
> 
>
  

On 11/9/21 13:42, Patrick Palka wrote:
> On Tue, 9 Nov 2021, Jason Merrill wrote:
> 
>> On 11/9/21 11:02, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>
>> OK, though I wonder about using releasing_vec instead of auto_vec; reusing a
>> previously allocated vec vs. building one on the stack.
> 
> Thanks a lot.  And hmm, I think using reusing a previously allocated vec
> here would be tricky since cp_parser_template_argument_list can be
> called recursively, and so only the outermost call would be able to
> benefit from reuse unless we perhaps maintain a freelist of such vecs
> IIUC.

We do maintain such a freelist, in make_tree_vector/release_tree_vector, 
which releasing_vec is a wrapper class for.

> Seems like that would complicate the code enough to not be worth
> it.
> 
>>> gcc/cp/ChangeLog:
>>>
>>> 	* parser.c (cp_parser_template_argument_list): Use auto_vec
>>> 	instead of manual memory management.
>>> ---
>>>    gcc/cp/parser.c | 35 ++++++++---------------------------
>>>    1 file changed, 8 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 32de97b08bd..8823399529e 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -18558,11 +18558,6 @@ cp_parser_template_name (cp_parser* parser,
>>>    static tree
>>>    cp_parser_template_argument_list (cp_parser* parser)
>>>    {
>>> -  tree fixed_args[10];
>>> -  unsigned n_args = 0;
>>> -  unsigned alloced = 10;
>>> -  tree *arg_ary = fixed_args;
>>> -  tree vec;
>>>      bool saved_in_template_argument_list_p;
>>>      bool saved_ice_p;
>>>      bool saved_non_ice_p;
>>> @@ -18581,16 +18576,15 @@ cp_parser_template_argument_list (cp_parser*
>>> parser)
>>>      parser->non_integral_constant_expression_p = false;
>>>        /* Parse the arguments.  */
>>> +  auto_vec<tree, 10> args;
>>>      do
>>>        {
>>> -      tree argument;
>>> -
>>> -      if (n_args)
>>> +      if (!args.is_empty ())
>>>    	/* Consume the comma.  */
>>>    	cp_lexer_consume_token (parser->lexer);
>>>            /* Parse the template-argument.  */
>>> -      argument = cp_parser_template_argument (parser);
>>> +      tree argument = cp_parser_template_argument (parser);
>>>            /* If the next token is an ellipsis, we're expanding a template
>>>             argument pack. */
>>> @@ -18610,29 +18604,16 @@ cp_parser_template_argument_list (cp_parser*
>>> parser)
>>>              argument = make_pack_expansion (argument);
>>>            }
>>>    -      if (n_args == alloced)
>>> -	{
>>> -	  alloced *= 2;
>>> -
>>> -	  if (arg_ary == fixed_args)
>>> -	    {
>>> -	      arg_ary = XNEWVEC (tree, alloced);
>>> -	      memcpy (arg_ary, fixed_args, sizeof (tree) * n_args);
>>> -	    }
>>> -	  else
>>> -	    arg_ary = XRESIZEVEC (tree, arg_ary, alloced);
>>> -	}
>>> -      arg_ary[n_args++] = argument;
>>> +      args.safe_push (argument);
>>>        }
>>>      while (cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
>>>    -  vec = make_tree_vec (n_args);
>>> +  int n_args = args.length ();
>>> +  tree vec = make_tree_vec (n_args);
>>>    -  while (n_args--)
>>> -    TREE_VEC_ELT (vec, n_args) = arg_ary[n_args];
>>> +  for (int i = 0; i < n_args; i++)
>>> +    TREE_VEC_ELT (vec, i) = args[i];
>>>    -  if (arg_ary != fixed_args)
>>> -    free (arg_ary);
>>>      parser->non_integral_constant_expression_p = saved_non_ice_p;
>>>      parser->integral_constant_expression_p = saved_ice_p;
>>>      parser->in_template_argument_list_p = saved_in_template_argument_list_p;
>>>
>>
>>
>
  

On Wed, Nov 10, 2021 at 12:16 AM Jason Merrill <jason@redhat.com> wrote:
>
> On 11/9/21 13:42, Patrick Palka wrote:
> > On Tue, 9 Nov 2021, Jason Merrill wrote:
> >
> >> On 11/9/21 11:02, Patrick Palka wrote:
> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >>
> >> OK, though I wonder about using releasing_vec instead of auto_vec; reusing a
> >> previously allocated vec vs. building one on the stack.
> >
> > Thanks a lot.  And hmm, I think using reusing a previously allocated vec
> > here would be tricky since cp_parser_template_argument_list can be
> > called recursively, and so only the outermost call would be able to
> > benefit from reuse unless we perhaps maintain a freelist of such vecs
> > IIUC.
>
> We do maintain such a freelist, in make_tree_vector/release_tree_vector,
> which releasing_vec is a wrapper class for.

Oops sorry, I had totally forgotten about that aspect of
make/release_tree_vector.  Hmm, so given that a template argument list
is very likely to be very small, whereas a vector obtained from the
freelist can have an arbitrary large capacity (depending on what it
stored for its previous users), it seems more overall resourceful to
use a stack-allocated auto_vec here IMHO.

>
> > Seems like that would complicate the code enough to not be worth
> > it.
> >
> >>> gcc/cp/ChangeLog:
> >>>
> >>>     * parser.c (cp_parser_template_argument_list): Use auto_vec
> >>>     instead of manual memory management.
> >>> ---
> >>>    gcc/cp/parser.c | 35 ++++++++---------------------------
> >>>    1 file changed, 8 insertions(+), 27 deletions(-)
> >>>
> >>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> >>> index 32de97b08bd..8823399529e 100644
> >>> --- a/gcc/cp/parser.c
> >>> +++ b/gcc/cp/parser.c
> >>> @@ -18558,11 +18558,6 @@ cp_parser_template_name (cp_parser* parser,
> >>>    static tree
> >>>    cp_parser_template_argument_list (cp_parser* parser)
> >>>    {
> >>> -  tree fixed_args[10];
> >>> -  unsigned n_args = 0;
> >>> -  unsigned alloced = 10;
> >>> -  tree *arg_ary = fixed_args;
> >>> -  tree vec;
> >>>      bool saved_in_template_argument_list_p;
> >>>      bool saved_ice_p;
> >>>      bool saved_non_ice_p;
> >>> @@ -18581,16 +18576,15 @@ cp_parser_template_argument_list (cp_parser*
> >>> parser)
> >>>      parser->non_integral_constant_expression_p = false;
> >>>        /* Parse the arguments.  */
> >>> +  auto_vec<tree, 10> args;
> >>>      do
> >>>        {
> >>> -      tree argument;
> >>> -
> >>> -      if (n_args)
> >>> +      if (!args.is_empty ())
> >>>     /* Consume the comma.  */
> >>>     cp_lexer_consume_token (parser->lexer);
> >>>            /* Parse the template-argument.  */
> >>> -      argument = cp_parser_template_argument (parser);
> >>> +      tree argument = cp_parser_template_argument (parser);
> >>>            /* If the next token is an ellipsis, we're expanding a template
> >>>             argument pack. */
> >>> @@ -18610,29 +18604,16 @@ cp_parser_template_argument_list (cp_parser*
> >>> parser)
> >>>              argument = make_pack_expansion (argument);
> >>>            }
> >>>    -      if (n_args == alloced)
> >>> -   {
> >>> -     alloced *= 2;
> >>> -
> >>> -     if (arg_ary == fixed_args)
> >>> -       {
> >>> -         arg_ary = XNEWVEC (tree, alloced);
> >>> -         memcpy (arg_ary, fixed_args, sizeof (tree) * n_args);
> >>> -       }
> >>> -     else
> >>> -       arg_ary = XRESIZEVEC (tree, arg_ary, alloced);
> >>> -   }
> >>> -      arg_ary[n_args++] = argument;
> >>> +      args.safe_push (argument);
> >>>        }
> >>>      while (cp_lexer_next_token_is (parser->lexer, CPP_COMMA));
> >>>    -  vec = make_tree_vec (n_args);
> >>> +  int n_args = args.length ();
> >>> +  tree vec = make_tree_vec (n_args);
> >>>    -  while (n_args--)
> >>> -    TREE_VEC_ELT (vec, n_args) = arg_ary[n_args];
> >>> +  for (int i = 0; i < n_args; i++)
> >>> +    TREE_VEC_ELT (vec, i) = args[i];
> >>>    -  if (arg_ary != fixed_args)
> >>> -    free (arg_ary);
> >>>      parser->non_integral_constant_expression_p = saved_non_ice_p;
> >>>      parser->integral_constant_expression_p = saved_ice_p;
> >>>      parser->in_template_argument_list_p = saved_in_template_argument_list_p;
> >>>
> >>
> >>
> >
>