Re: [PATCH] Improvements to Fortran support

[Daniel Jacobowitz <drow at mvista dot com>]

On Fri, May 16, 2003 at 12:21:03PM +0100, David Lecomber wrote:
> Reposting as I now have the copyright assignment in place..
> 
> Fortran arrays are presently allocated in their entirety and then the
> correct element is pulled out.  This (a) doesn't scale, (b) doesn't
> work if the array is a parameter to a subroutine and supplied with a
> (*) in the dimensions (GDB runs out of memory doing malloc( -1 )..)
> 
> The fix here makes the behaviour the same as the code for doing C
> arrays..

Thanks for the patch and your patience.  I have a couple of small
comments on your changes.

First of all, when submitting a patch, please include a standard
ChangeLog entry - even for small patches like this one, it makes it
easier to see at a glance what's going on and where.  For this patch it
would look something like:

2003-05-16  David Lecomber  <david@streamline-computing.com>

	* eval.c (evaluate_subexp_standard): Correct handling
	for Fortran multidimensional array subscripts.

Secondly, some nits about formatting:
> ! 	/* Construct a value node with the value of the offset */
> ! 	/* lower will get subtracted off in value_subscript, hence add it here */

Should be:
	/* Construct a value node with the value of the offset.  LOWER
	   will get subtracted off in value_subscript, hence add it
	   here.  */

(i.e. full sentences, two spaces after a full stop.)


And, my only substantive comments:

 - Could you explain why this makes a difference?  You change value_ind
   (value_add (value_coerce_array (), idx))) into value_subscript (array,
   idx).  value_subscript will call value_coerce_array, then value_add and
   value_ind on the result.

 - How does f77_get_dynamic_lowerbound end up being called from
   value_subscript?  I couldn't figure it out, which says that either
   I woke up denser than usual this morning or a better comment is in
   order.

> *** eval.c	Sun Dec 15 22:29:59 2002
> --- eval.c	Sun Dec 15 22:28:41 2002
> *************** evaluate_subexp_standard (struct type *e
> *** 1383,1392 ****
>   	  offset_item =
>   	    array_size_array[i] * offset_item + subscript_array[i];
>   
> - 	/* Construct a value node with the value of the offset */
> - 
> - 	arg2 = value_from_longest (builtin_type_f_integer, offset_item);
> - 
>   	/* Let us now play a dirty trick: we will take arg1 
>   	   which is a value node pointing to the topmost level
>   	   of the multidimensional array-set and pretend
> --- 1383,1388 ----
> *************** evaluate_subexp_standard (struct type *e
> *** 1395,1401 ****
>   	   returns the correct type value */
>   
>   	VALUE_TYPE (arg1) = tmp_type;
> ! 	return value_ind (value_add (value_coerce_array (arg1), arg2));
>         }
>   
>       case BINOP_LOGICAL_AND:
> --- 1391,1405 ----
>   	   returns the correct type value */
>   	
>   	VALUE_TYPE (arg1) = tmp_type;
> ! 
> ! 	f77_get_dynamic_lowerbound (tmp_type, &lower);
> ! 
> ! 	/* Construct a value node with the value of the offset */
> ! 	/* lower will get subtracted off in value_subscript, hence add it here */
> ! 
> ! 	arg2 = value_from_longest (builtin_type_f_integer, offset_item + lower);
> ! 
> ! 	return value_subscript(arg1, arg2);
>         }
>   
>       case BINOP_LOGICAL_AND:
> 
> 
> David
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer