Re: [PATCHv2 05/10] gdb/fortran: Clean up array/string expression evaluation

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Andrew Burgess <andrew.burgess@embecosm.com> [2020-08-26 15:49:12 +0100]:

> In preparation for adding Fortan array stride expression support, this
> is the first phase of some clean up to the expression evaluation for
> Fortran arrays and strings.
> 
> The current code is split into two blocks, linked, weirdly, with a
> goto.  After this commit all the code is moved to its own function,
> and arrays and strings are now handled using the same code; this will
> be useful later when I want to add array stride support where strings
> will want to be treated just like arrays.
> 
> For now the new function is added as a static within eval.c, even
> though the function is Fortran only.  A following commit will remove
> some of the Fortran specific code from eval.c into one of the Fortran
> specific files, including this new function.
> 
> There should be no user visible changes after this commit.
> 
> gdb/ChangeLog:
> 
> 	* eval.c (fortran_value_subarray): New function, content is taken
> 	from...
> 	(evaluate_subexp_standard): ...here, in two places.  Now arrays
> 	and strings both call the new function.
> 	(calc_f77_array_dims): Add header comment, handle strings.

I went ahead and pushed this patch with a slightly modified commit
message.

Thanks,
Andrew

> ---
>  gdb/ChangeLog |   8 +++
>  gdb/eval.c    | 136 +++++++++++++++++++++++++-------------------------
>  2 files changed, 75 insertions(+), 69 deletions(-)
> 
> diff --git a/gdb/eval.c b/gdb/eval.c
> index cd300ddfef6..660edbe34af 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -1260,6 +1260,67 @@ is_integral_or_integral_reference (struct type *type)
>  	  && is_integral_type (TYPE_TARGET_TYPE (type)));
>  }
>  
> +/* Called from evaluate_subexp_standard to perform array indexing, and
> +   sub-range extraction, for Fortran.  As well as arrays this function
> +   also handles strings as they can be treated like arrays of characters.
> +   ARRAY is the array or string being accessed.  EXP, POS, and NOSIDE are
> +   as for evaluate_subexp_standard, and NARGS is the number of arguments
> +   in this access (e.g. 'array (1,2,3)' would be NARGS 3).  */
> +
> +static struct value *
> +fortran_value_subarray (struct value *array, struct expression *exp,
> +			int *pos, int nargs, enum noside noside)
> +{
> +  if (exp->elts[*pos].opcode == OP_RANGE)
> +    return value_f90_subarray (array, exp, pos, noside);
> +
> +  if (noside == EVAL_SKIP)
> +    {
> +      skip_undetermined_arglist (nargs, exp, pos, noside);
> +      /* Return the dummy value with the correct type.  */
> +      return array;
> +    }
> +
> +  LONGEST subscript_array[MAX_FORTRAN_DIMS];
> +  int ndimensions = 1;
> +  struct type *type = check_typedef (value_type (array));
> +
> +  if (nargs > MAX_FORTRAN_DIMS)
> +    error (_("Too many subscripts for F77 (%d Max)"), MAX_FORTRAN_DIMS);
> +
> +  ndimensions = calc_f77_array_dims (type);
> +
> +  if (nargs != ndimensions)
> +    error (_("Wrong number of subscripts"));
> +
> +  gdb_assert (nargs > 0);
> +
> +  /* Now that we know we have a legal array subscript expression let us
> +     actually find out where this element exists in the array.  */
> +
> +  /* Take array indices left to right.  */
> +  for (int i = 0; i < nargs; i++)
> +    {
> +      /* Evaluate each subscript; it must be a legal integer in F77.  */
> +      value *arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
> +
> +      /* Fill in the subscript array.  */
> +      subscript_array[i] = value_as_long (arg2);
> +    }
> +
> +  /* Internal type of array is arranged right to left.  */
> +  for (int i = nargs; i > 0; i--)
> +    {
> +      struct type *array_type = check_typedef (value_type (array));
> +      LONGEST index = subscript_array[i - 1];
> +
> +      array = value_subscripted_rvalue (array, index,
> +					f77_get_lowerbound (array_type));
> +    }
> +
> +  return array;
> +}
> +
>  struct value *
>  evaluate_subexp_standard (struct type *expect_type,
>  			  struct expression *exp, int *pos,
> @@ -1954,33 +2015,8 @@ evaluate_subexp_standard (struct type *expect_type,
>        switch (code)
>  	{
>  	case TYPE_CODE_ARRAY:
> -	  if (exp->elts[*pos].opcode == OP_RANGE)
> -	    return value_f90_subarray (arg1, exp, pos, noside);
> -	  else
> -	    {
> -	      if (noside == EVAL_SKIP)
> -		{
> -		  skip_undetermined_arglist (nargs, exp, pos, noside);
> -		  /* Return the dummy value with the correct type.  */
> -		  return arg1;
> -		}
> -	      goto multi_f77_subscript;
> -	    }
> -
>  	case TYPE_CODE_STRING:
> -	  if (exp->elts[*pos].opcode == OP_RANGE)
> -	    return value_f90_subarray (arg1, exp, pos, noside);
> -	  else
> -	    {
> -	      if (noside == EVAL_SKIP)
> -		{
> -		  skip_undetermined_arglist (nargs, exp, pos, noside);
> -		  /* Return the dummy value with the correct type.  */
> -		  return arg1;
> -		}
> -	      arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
> -	      return value_subscript (arg1, value_as_long (arg2));
> -	    }
> +	  return fortran_value_subarray (arg1, exp, pos, nargs, noside);
>  
>  	case TYPE_CODE_PTR:
>  	case TYPE_CODE_FUNC:
> @@ -2400,49 +2436,6 @@ evaluate_subexp_standard (struct type *expect_type,
>  	}
>        return (arg1);
>  
> -    multi_f77_subscript:
> -      {
> -	LONGEST subscript_array[MAX_FORTRAN_DIMS];
> -	int ndimensions = 1, i;
> -	struct value *array = arg1;
> -
> -	if (nargs > MAX_FORTRAN_DIMS)
> -	  error (_("Too many subscripts for F77 (%d Max)"), MAX_FORTRAN_DIMS);
> -
> -	ndimensions = calc_f77_array_dims (type);
> -
> -	if (nargs != ndimensions)
> -	  error (_("Wrong number of subscripts"));
> -
> -	gdb_assert (nargs > 0);
> -
> -	/* Now that we know we have a legal array subscript expression 
> -	   let us actually find out where this element exists in the array.  */
> -
> -	/* Take array indices left to right.  */
> -	for (i = 0; i < nargs; i++)
> -	  {
> -	    /* Evaluate each subscript; it must be a legal integer in F77.  */
> -	    arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
> -
> -	    /* Fill in the subscript array.  */
> -
> -	    subscript_array[i] = value_as_long (arg2);
> -	  }
> -
> -	/* Internal type of array is arranged right to left.  */
> -	for (i = nargs; i > 0; i--)
> -	  {
> -	    struct type *array_type = check_typedef (value_type (array));
> -	    LONGEST index = subscript_array[i - 1];
> -
> -	    array = value_subscripted_rvalue (array, index,
> -					      f77_get_lowerbound (array_type));
> -	  }
> -
> -	return array;
> -      }
> -
>      case BINOP_LOGICAL_AND:
>        arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
>        if (noside == EVAL_SKIP)
> @@ -3356,12 +3349,17 @@ parse_and_eval_type (char *p, int length)
>    return expr->elts[1].type;
>  }
>  
> +/* Return the number of dimensions for a Fortran array or string.  */
> +
>  int
>  calc_f77_array_dims (struct type *array_type)
>  {
>    int ndimen = 1;
>    struct type *tmp_type;
>  
> +  if ((array_type->code () == TYPE_CODE_STRING))
> +    return 1;
> +
>    if ((array_type->code () != TYPE_CODE_ARRAY))
>      error (_("Can't get dimensions for a non-array type"));
>  
> -- 
> 2.25.4
>