Re: Remove deprecated_set_value_type (part 1)

[Daniel Jacobowitz <drow@false.org>]

On Mon, Jan 21, 2008 at 04:53:05AM +0000, Rob Quill wrote:
> 2008-01-21   Rob Quill <rob.quill@gmail.com>
> 
>          Removes some uses of deprecated_set_value_type
>          * ada-lang.c (ada_coerce_to_simple_array_type): Create a new
> zero value of the correct type instead of changing type.
>          (ada_value_assign): Call copy_value_and_change_type instead.
>          * c-valprint.c (c_value_print): Call copy_value_and_change_type.
>          * printcmd.c (printf_command): Create a zero value of the
> correct type and copy the contents of the old value into it.
>          * valops.c (value_cast_pointers, value_cast, value_assign,
> value_addr): Call value_value_copy_and_change_type instead of copying
> the value and then changing the type.
>          * value.c (value_copy_and_change_type): Define.
>          * value.h (value_copy_and_change_type): Declare.

Formatting: a changelog entry is single tab indented, and each line
should wrap at a sensible margin (somewhere between 72 and 79
columns).

> Index: gdb/ada-lang.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ada-lang.c,v
> retrieving revision 1.131
> diff -u -p -r1.131 ada-lang.c
> --- gdb/ada-lang.c	8 Jan 2008 19:28:08 -0000	1.131
> +++ gdb/ada-lang.c	21 Jan 2008 00:00:46 -0000
> @@ -1737,9 +1737,8 @@ struct type *
>  ada_coerce_to_simple_array_type (struct type *type)
>  {
>    struct value *mark = value_mark ();
> -  struct value *dummy = value_from_longest (builtin_type_long, 0);
> +  struct value *dummy = value_zero(type, not_lval);
>    struct type *result;
> -  deprecated_set_value_type (dummy, type);
>    result = ada_type_of_array (dummy, 0);
>    value_free_to_mark (mark);
>    return result;

Spaces before open parentheses, please.  This one's certainly correct.

> @@ -2241,10 +2240,9 @@ ada_value_assign (struct value *toval, s
>        if (deprecated_memory_changed_hook)
>  	deprecated_memory_changed_hook (to_addr, len);
> 
> -      val = value_copy (toval);
> +      val = value_copy_and_change_type(toval, type);
>        memcpy (value_contents_raw (val), value_contents (fromval),
>                TYPE_LENGTH (type));
> -      deprecated_set_value_type (val, type);
> 
>        return val;
>      }

I can't tell if this one is right.  I don't understand what it's for
either.  Mind leaving it out for now?

> Index: gdb/c-valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/c-valprint.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 c-valprint.c
> --- gdb/c-valprint.c	11 Jan 2008 13:34:14 -0000	1.48
> +++ gdb/c-valprint.c	21 Jan 2008 00:00:46 -0000
> @@ -562,8 +562,8 @@ c_value_print (struct value *val, struct
>  	       * 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)));
> +	      temparg=value_copy_and_change_type(val,
> +				  lookup_pointer_type (TYPE_TARGET_TYPE(type)));
>  	      val=temparg;
>  	    }
>  	  /* Pointer to class, check real type of object */

This one should just call value_addr; that's how you convert a
reference into the corresponding pointer.

> Index: gdb/printcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/printcmd.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 printcmd.c
> --- gdb/printcmd.c	11 Jan 2008 13:34:14 -0000	1.116
> +++ gdb/printcmd.c	21 Jan 2008 00:00:50 -0000
> @@ -2035,9 +2035,19 @@ printf_command (char *arg, int from_tty)
>  	  {
>  	    struct type *type = value_type (val_args[nargs]);
>  	    if (TYPE_LENGTH (type) == sizeof (float))
> -	      deprecated_set_value_type (val_args[nargs], builtin_type_float);
> +		{
> +			struct value *temp = value_zero(builtin_type_float, not_lval);
> +			memcpy(value_contents_all_raw(temp),
> value_contents_all_raw(val_args[nargs]),
> +					TYPE_LENGTH(builtin_type_float));
> +			val_args[nargs] = temp;
> +		}
>  	    if (TYPE_LENGTH (type) == sizeof (double))
> -	      deprecated_set_value_type (val_args[nargs], builtin_type_double);
> +		{	
> +			struct value *temp = value_zero(builtin_type_double, not_lval);
> +			memcpy(value_contents_all_raw(temp),
> value_contents_all_raw(val_args[nargs]),
> +					TYPE_LENGTH(builtin_type_double));
> +			val_args[nargs] = temp;
> +		}
>  	  }
>  	nargs++;
>  	s = s1;

Formatting: two spaces for the braces, two more for the body, spaces
before open parentheses.  Also be careful of the right margin.  The
code change looks correct to me.  You can use just
value_contents_writeable for temp (the _all_raw bit is only for cases
where you might have a different dynamic type), and use just
value_contents for the right hand type (value_contents is the data
that goes along with value_type; value_contents_all* might be an
enclosing object).

> @@ -440,8 +439,7 @@ value_cast (struct type *type, struct va
>        if (code1 == TYPE_CODE_PTR && code2 == TYPE_CODE_PTR)
>  	return value_cast_pointers (type, arg2);
> 
> -      arg2 = value_copy (arg2);
> -      deprecated_set_value_type (arg2, type);
> +      arg2 = value_copy_and_change_type(arg2, type);
>        arg2 = value_change_enclosing_type (arg2, type);
>        set_value_pointed_to_offset (arg2, 0);	/* pai: chk_val */
>        return arg2;

Space before apren.

> Index: gdb/value.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/value.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 value.c
> --- gdb/value.c	16 Jan 2008 16:16:44 -0000	1.56
> +++ gdb/value.c	21 Jan 2008 00:00:56 -0000
> @@ -275,6 +275,33 @@ deprecated_set_value_type (struct value
>    value->type = type;
>  }
> 
> +struct value *
> +value_copy_and_change_type (struct value *arg, struct type *type)
> +{

You can just use value_copy for most of this, to avoid the
duplication; that's not a problem here, since we're inside value.c.

> +/* This function acts as a partial replacement for deprecated_set_value_type.
> +   The function is exactly the same as value_copy() but it also changes
> +   the type.  */
> +
> + extern struct value *value_copy_and_change_type(struct value *arg,
> +					          struct type *type);

Space before paren.

/* Copy ARG to a new value.  The new value is exactly the same, except
   for its type, which is set to TYPE.  */

Don't want to add new references to deprecated_set_value_type; just
one more thing to delete when it's gone.

-- 
Daniel Jacobowitz
CodeSourcery