Re: [RFA] "lazily" allocate the raw content of lazy values

[Joel Brobecker <brobecker@adacore.com>]

I'll join Jerome in thanking Tom for taking time to review this patch,
and help improving in. Very very much appreciated. Thanks, Tom!

> 2008-11-25  Jerome Guitton  <guitton@adacore.com>
> 
> 	* value.h (allocate_value_lazy): New function declaration.
> 	(value_free): Remove macro, make it a function.
> 	* value.c (value): Move actual content outside of the memory space
>         of the struct; add a pointer to this actual content.
> 	(allocate_value_lazy, allocate_value_contents): New function.
> 	(allocate_value): Reimplement using these two new functions.
> 	(value_contents_raw, value_contents_all_raw): If no memory
> 	has been allocated yet for the actual content, allocate it.
> 	(value_contents_all): Resync with struct value's changes.
> 	(value_free): New function.
> 	(value_copy, value_primitive_field): Use new function
> 	allocate_value_lazy to allocate lazy values.
> 	(value_change_enclosing_type): Resync with struct value's changes.
> 	As the value is not reallocated, remove the special handling for
> 	the value chain (now obsolete).
> 	* valops.c (value_at_lazy): Use new function allocate_value_lazy.
> 	(value_fetch_lazy): Allocate value content. Use allocate_value_lazy
> 	to allocate lazy values.
> 	(value_slice): Use allocate_value_lazy to allocate lazy values.

Pre-approved after the following minor nits are fixed.

Also, we discussed on the phone the possibility of having contents == NULL
being equivalent to lazy (thus allowing us to get rid of lazy). But it
turns out that it makes the value structure a little less convenient
in certain cases (getting the value of each element in an array using
the same value). At least for now, Jerome elected to keep the two
concepts separate, but I think a comment explaining that would be
useful. This can be done as a followup patch so that people get a chance
to see the new comment and make comments.

> +  /* Actual contents of the value. Target byte-order. NULL or not valid if
                                                        ^^ 2 spaces here.

> +/* Allocate a  value  that has the correct length for type TYPE.  */
> +
> +struct value *
> +allocate_value (struct type *type)

In this case, I think it's important to say in the function description
that this function allocates the value contents. To me "has the correct
length for type TYPE" is a little unclear (I realize that this comes
from the original implementation).

>        /* Lazy register values with offsets are not supported.  */
>        if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
>  	value_fetch_lazy (arg1);
>  
>        if (value_lazy (arg1))
> -	set_value_lazy (v, 1);
> +        v = allocate_value_lazy (value_enclosing_type (arg1));

1 tab instead of 8 spaces. (I hate tabs)

> @@ -2969,13 +2974,15 @@ value_slice (struct value *array, int lo
>  				      slice_range_type);
>        TYPE_CODE (slice_type) = TYPE_CODE (array_type);
>  
> -      slice = allocate_value (slice_type);
>        if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
> -	set_value_lazy (slice, 1);
> +        slice = allocate_value_lazy (slice_type);

Same here...

-- 
Joel