Re: Remove deprecated_set_value_type

["Rob Quill" <rob.quill@gmail.com>]

On 16/12/2007, Daniel Jacobowitz <drow@false.org> wrote:
> On Tue, Nov 13, 2007 at 07:23:18PM +0000, Rob Quill wrote:
> > Is it a correct solution to add a function something like
> > copy_val_except_type, which copies all the fields from a value struct
> > except the type? So an new value is created of the right type, then
> > cop_val_except_type is called, which would replace the other fields.
>
> Sorry for losing this message.  That may be right, but only for some
> of the calls.  The trick is to consider not just what the effect of
> the current calls is, but what this means in terms of the abstraction
> of a "struct value".  Does it make sense to change the type of a value
> without changing anything else about it?  In a few cases yes, but
> not usually.
>
> I picked one call to deprecated_set_value_type; the one in
> c-valprint.c.
>
>               /* Copy value, change to pointer, so we don't get an
>                * error about a non-pointer type in value_rtti_target_type
>                */
>               struct value *temparg;
>               temparg=value_copy(val);
>               deprecated_set_value_type (temparg, lookup_pointer_type (TYPE_TARGET_TYPE(type)));
>               val=temparg;
>
> There's a better way to do this: value_addr of a reference becomes
> a pointer to the same object.  Of course, I see that the body of
> value_addr does it the same way as this code, using the
> deprecated method.  So this call should use value_addr and the
> implementation of value_addr probably needs a new method that
> doesn't exist yet.  I suggest "value_copy_and_change_type".
>
> To pick another example, printcmd.c uses it to do unchecked
> conversions from integers to bit patterns of floating point numbers -
> much to my surprise!  I had no idea this was there until I read the
> code.  Assuming we want to keep that behavior, the right way
> is not to smash the type of some existing value; instead, use
> value_zero (not_lval) to create a new value, and copy the
> bit pattern into it.
>
> Does that make sense?

Hi Daniel,

I've been working my way through the cases where
deprecated_set_value_type is used, and am reasonably sure (pending
testing) that I have removed them in the right way. However, I a
unsure about this case from eval.c

	/* 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
	   that it is actually a array of the final element
	   type, this will ensure that value_subscript()
	   returns the correct type value */

	deprecated_set_value_type (arg1, tmp_type);
	return value_ind (value_add (value_coerce_array (arg1), arg2));

I'm not sure how to handle this as I'm not really sure what it is
doing. Intuitively it seems that using a "dirty trick" is a bad idea
so maybe now might be the time to find a better way of doing this now
while we're sorting out other things. I was just wondering if you had
any suggestions or advice?

Thanks.

Rob