Re: [fortran patch] Don't use TREE_LISTs for storing arguments to intrinsic functions

[Brooks Moses <brooks dot moses at codesourcery dot com>]

Lee Millward wrote:
> Attached is an updated patch that incorporates the changes requested
> by Brooks and the comments from Bernhard. This has been bootstrapped
> and regression tested successfully on i686-pc-linux-gnu. Same
> changelog as the original.
>
> Ok to apply?

This is OK, with the following small changes.


> @@ -163,24 +163,27 @@ real_compnt_info;
>  
>  enum rounding_mode { RND_ROUND, RND_TRUNC, RND_CEIL, RND_FLOOR };
>  
> -/* Evaluate the arguments to an intrinsic function.  */
> -/* FIXME: This function and its callers should be rewritten so that it's
> -   not necessary to cons up a list to hold the arguments.  */
> +/* Evaluate the arguments to an intrinsic function.  The value
> +   of NARGS may be less than the actual number of arguments in EXPR
> +   to allow optional "KIND" arguments tothat are not included in the
> +   generated code to be ignored.  */

There's a "tothat" on the second-to-last line that should be just "that".

> @@ -195,7 +198,8 @@ gfc_conv_intrinsic_function_args (gfc_se
>  	{
>  	  gfc_conv_expr (&argse, e);
>  	  gfc_conv_string_parameter (&argse);
> -	  args = gfc_chainon_list (args, argse.string_length);
> +	  gcc_assert (actual);
> +          argarray[curr_arg++] = argse.string_length;

This assert still needs to be "gcc_assert (curr_arg < nargs)", and it 
should come directly after the line with curr_arg++ in it.  (The 
existence of "actual" is being checked elsewhere; here the potential 
problem to guard against is if we come across a string argument with 
only one slot left in nargs.)

Also, the last line there is indented with spaces rather than a tab.

>  	}
>        else
>          gfc_conv_expr_val (&argse, e);
> @@ -210,9 +214,32 @@ gfc_conv_intrinsic_function_args (gfc_se
>  
>        gfc_add_block_to_block (&se->pre, &argse.pre);
>        gfc_add_block_to_block (&se->post, &argse.post);
> -      args = gfc_chainon_list (args, argse.expr);
> +      gcc_assert (actual);
> +      argarray[curr_arg] = argse.expr;

This assert needs to be moved to the very front of this block, right 
before the line with "e = actual->expr".  (We want to assert that actual 
exists before we try to pull a pointer out of it.)

Thanks!
- Brooks