From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28839 invoked by alias); 21 Jan 2008 04:53:25 -0000 Received: (qmail 28829 invoked by uid 22791); 21 Jan 2008 04:53:24 -0000 X-Spam-Check-By: sourceware.org Received: from wa-out-1112.google.com (HELO wa-out-1112.google.com) (209.85.146.180) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 21 Jan 2008 04:53:06 +0000 Received: by wa-out-1112.google.com with SMTP id l35so3482979waf.12 for ; Sun, 20 Jan 2008 20:53:05 -0800 (PST) Received: by 10.115.94.1 with SMTP id w1mr4977356wal.85.1200891185032; Sun, 20 Jan 2008 20:53:05 -0800 (PST) Received: by 10.114.102.11 with HTTP; Sun, 20 Jan 2008 20:53:05 -0800 (PST) Message-ID: Date: Mon, 21 Jan 2008 04:53:00 -0000 From: "Rob Quill" To: gdb-patches@sourceware.org, "Daniel Jacobowitz" Subject: Remove deprecated_set_value_type (part 1) MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00501.txt.bz2 On 16/12/2007, Daniel Jacobowitz 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. > Hi all, The attached patch goes part way to removing deprecated_set_value_type. It adds a new function value_copy_and_change_type which copies a copies a value but changes hte type of the new value. There are also some replacements which create new values of a certain type and copy data between the old and new values, as necessary. The remaining uses of deprecated_set_value_type seem to be in more difficult to understand places, such as in the language specific files like jv-lang and ada-lang where they seem to be used for coercing arrays. So far I have not dealt with these cases as I am unsure why they are need to change the type. However, as I see it, unless we are to have a set of separate very similar functions such as coerce_array_and_change_type() etc, then it seems that the possible solutions are either copy the value and change the type or make a new value of the same type and copy some other attributes of the type. Is this the right idea or have I missed the point? If this it right then for the remaining uses I intend to figure out which of the above methods is the best (right) way. Rob 2008-01-21 Rob Quill 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. 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; @@ -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; } 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 */ 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; Index: gdb/valops.c =================================================================== RCS file: /cvs/src/src/gdb/valops.c,v retrieving revision 1.181 diff -u -p -r1.181 valops.c --- gdb/valops.c 7 Jan 2008 22:33:57 -0000 1.181 +++ gdb/valops.c 21 Jan 2008 00:00:55 -0000 @@ -250,8 +250,7 @@ value_cast_pointers (struct type *type, } /* No superclass found, just change the pointer type. */ - 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; @@ -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; @@ -754,10 +752,9 @@ value_assign (struct value *toval, struc fromval = value_from_longest (type, fieldval); } - 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); val = value_change_enclosing_type (val, value_enclosing_type (fromval)); set_value_embedded_offset (val, value_embedded_offset (fromval)); @@ -884,9 +881,8 @@ value_addr (struct value *arg1) /* Copy the value, but change the type from (T&) to (T*). We keep the same location information, which is efficient, and allows &(&X) to get the location containing the reference. */ - arg2 = value_copy (arg1); - deprecated_set_value_type (arg2, - lookup_pointer_type (TYPE_TARGET_TYPE (type))); + arg2 = value_copy_and_change_type (arg1, + lookup_pointer_type (TYPE_TARGET_TYPE (type))); return arg2; } if (TYPE_CODE (type) == TYPE_CODE_FUNC) 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) +{ + struct type *encl_type = value_enclosing_type (arg); + struct value *val = allocate_value (encl_type); + val->type = type; + VALUE_LVAL (val) = VALUE_LVAL (arg); + val->location = arg->location; + val->offset = arg->offset; + val->bitpos = arg->bitpos; + val->bitsize = arg->bitsize; + VALUE_FRAME_ID (val) = VALUE_FRAME_ID (arg); + VALUE_REGNUM (val) = VALUE_REGNUM (arg); + val->lazy = arg->lazy; + val->optimized_out = arg->optimized_out; + val->embedded_offset = value_embedded_offset (arg); + val->pointed_to_offset = arg->pointed_to_offset; + val->modifiable = arg->modifiable; + if (!value_lazy (val)) + { + memcpy (value_contents_all_raw (val), value_contents_all_raw (arg), + TYPE_LENGTH (value_enclosing_type (arg))); + + } + return val; +} + int value_offset (struct value *value) { Index: gdb/value.h =================================================================== RCS file: /cvs/src/src/gdb/value.h,v retrieving revision 1.107 diff -u -p -r1.107 value.h --- gdb/value.h 18 Jan 2008 17:07:40 -0000 1.107 +++ gdb/value.h 21 Jan 2008 00:00:57 -0000 @@ -56,6 +56,13 @@ extern struct type *value_type (struct v extern void deprecated_set_value_type (struct value *value, struct type *type); +/* 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); + /* Only used for bitfields; number of bits contained in them. */ extern int value_bitsize (struct value *);