Remove deprecated_set_value_type

Remove deprecated_set_value_type

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 202 bytes --]

Hi,

This patch removes the deprecated_set_value_type() function from
value.c, value.h and all other places which use it.

I also plan to try and remove the other deprecated functions from value.c

Rob

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: remove_deprecated_set_value_type.patch --]
[-- Type: text/x-patch; name=remove_deprecated_set_value_type.patch, Size: 23153 bytes --]

diff -urN ./clean/src/gdb/ada-lang.c ./deprecated-removed/src/gdb/ada-lang.c
--- ./clean/src/gdb/ada-lang.c	2007-11-02 19:34:11.000000000 +0000
+++ ./deprecated-removed/src/gdb/ada-lang.c	2007-11-12 18:34:17.000000000 +0000
@@ -1670,7 +1670,7 @@
   struct value *mark = value_mark ();
   struct value *dummy = value_from_longest (builtin_type_long, 0);
   struct type *result;
-  deprecated_set_value_type (dummy, type);
+  dummy->type = type;
   result = ada_type_of_array (dummy, 0);
   value_free_to_mark (mark);
   return result;
@@ -2167,7 +2167,7 @@
       val = value_copy (toval);
       memcpy (value_contents_raw (val), value_contents (fromval),
               TYPE_LENGTH (type));
-      deprecated_set_value_type (val, type);
+      val->type = type;
 
       return val;
     }
@@ -7386,7 +7386,7 @@
           || TYPE_LENGTH (TYPE_TARGET_TYPE (type2))
           != TYPE_LENGTH (TYPE_TARGET_TYPE (type2)))
         error (_("Incompatible types in assignment"));
-      deprecated_set_value_type (val, type);
+      val->type = type;
     }
   return val;
 }
diff -urN ./clean/src/gdb/c-valprint.c ./deprecated-removed/src/gdb/c-valprint.c
--- ./clean/src/gdb/c-valprint.c	2007-10-25 18:57:34.000000000 +0100
+++ ./deprecated-removed/src/gdb/c-valprint.c	2007-11-12 23:44:40.000000000 +0000
@@ -565,7 +565,7 @@
 	       */
 	      struct value *temparg;
 	      temparg=value_copy(val);
-	      deprecated_set_value_type (temparg, lookup_pointer_type (TYPE_TARGET_TYPE(type)));
+	      temparg->type = lookup_pointer_type (TYPE_TARGET_TYPE(type));
 	      val=temparg;
 	    }
 	  /* Pointer to class, check real type of object */
diff -urN ./clean/src/gdb/eval.c ./deprecated-removed/src/gdb/eval.c
--- ./clean/src/gdb/eval.c	2007-11-11 17:08:42.000000000 +0000
+++ ./deprecated-removed/src/gdb/eval.c	2007-11-12 23:46:08.000000000 +0000
@@ -1007,8 +1007,8 @@
 	if (gnu_runtime && (method != NULL))
 	  {
 	    /* Function objc_msg_lookup returns a pointer.  */
-	    deprecated_set_value_type (argvec[0],
-				       lookup_function_type (lookup_pointer_type (value_type (argvec[0]))));
+	    argvec[0]->type =
+				       lookup_function_type (lookup_pointer_type (value_type (argvec[0])));
 	    argvec[0] = call_function_by_hand (argvec[0], nargs + 2, argvec + 1);
 	  }
 
@@ -1691,7 +1691,7 @@
 	   type, this will ensure that value_subscript()
 	   returns the correct type value */
 
-	deprecated_set_value_type (arg1, tmp_type);
+	arg1->type = tmp_type;
 	return value_ind (value_add (value_coerce_array (arg1), arg2));
       }
 
diff -urN ./clean/src/gdb/gnu-v2-abi.c ./deprecated-removed/src/gdb/gnu-v2-abi.c
--- ./clean/src/gdb/gnu-v2-abi.c	2007-09-05 01:07:07.000000000 +0100
+++ ./deprecated-removed/src/gdb/gnu-v2-abi.c	2007-11-13 09:39:57.000000000 +0000
@@ -173,7 +173,7 @@
   else
     error (_("I'm confused:  virtual function table has bad type"));
   /* Reinstantiate the function pointer with the correct type.  */
-  deprecated_set_value_type (vfn, lookup_pointer_type (TYPE_FN_FIELD_TYPE (f, j)));
+  vfn->type = lookup_pointer_type (TYPE_FN_FIELD_TYPE (f, j));
 
   *arg1p = arg1;
   return vfn;
diff -urN ./clean/src/gdb/gnu-v2-abi.c~ ./deprecated-removed/src/gdb/gnu-v2-abi.c~
--- ./clean/src/gdb/gnu-v2-abi.c~	2007-09-05 01:07:07.000000000 +0100
+++ ./deprecated-removed/src/gdb/gnu-v2-abi.c~	2007-11-13 09:39:57.000000000 +0000
@@ -173,7 +173,7 @@
   else
     error (_("I'm confused:  virtual function table has bad type"));
   /* Reinstantiate the function pointer with the correct type.  */
-  deprecated_set_value_type (vfn, lookup_pointer_type (TYPE_FN_FIELD_TYPE (f, j)));
+  vfn->type = lookup_pointer_type (TYPE_FN_FIELD_TYPE (f, j));
 
   *arg1p = arg1;
   return vfn;
diff -urN ./clean/src/gdb/jv-lang.c ./deprecated-removed/src/gdb/jv-lang.c
--- ./clean/src/gdb/jv-lang.c	2007-11-02 19:34:11.000000000 +0000
+++ ./deprecated-removed/src/gdb/jv-lang.c	2007-11-12 23:47:41.000000000 +0000
@@ -319,7 +319,7 @@
       temp = clas;
       /* Set array element type. */
       temp = value_struct_elt (&temp, NULL, "methods", NULL, "structure");
-      deprecated_set_value_type (temp, lookup_pointer_type (value_type (clas)));
+      temp->type = lookup_pointer_type (value_type (clas));
       TYPE_TARGET_TYPE (type) = type_from_class (temp);
     }
 
@@ -890,7 +890,7 @@
 	  /* Get CLASS_ELEMENT_TYPE of the array type. */
 	  temp = value_struct_elt (&temp, NULL, "methods",
 				   NULL, "structure");
-	  deprecated_set_value_type (temp, value_type (clas));
+	  temp->type = value_type (clas);
 	  el_type = type_from_class (temp);
 	  if (TYPE_CODE (el_type) == TYPE_CODE_STRUCT)
 	    el_type = lookup_pointer_type (el_type);
diff -urN ./clean/src/gdb/objc-lang.c ./deprecated-removed/src/gdb/objc-lang.c
--- ./clean/src/gdb/objc-lang.c	2007-11-02 19:34:11.000000000 +0000
+++ ./deprecated-removed/src/gdb/objc-lang.c	2007-11-12 23:48:16.000000000 +0000
@@ -202,7 +202,7 @@
   else
     error (_("NSString: internal error -- no way to create new NSString"));
 
-  deprecated_set_value_type (nsstringValue, type);
+  nsstringValue->type = type;
   return nsstringValue;
 }
 
diff -urN ./clean/src/gdb/printcmd.c ./deprecated-removed/src/gdb/printcmd.c
--- ./clean/src/gdb/printcmd.c	2007-11-05 11:32:31.000000000 +0000
+++ ./deprecated-removed/src/gdb/printcmd.c	2007-11-12 18:39:30.000000000 +0000
@@ -2040,9 +2040,9 @@
 	  {
 	    struct type *type = value_type (val_args[nargs]);
 	    if (TYPE_LENGTH (type) == sizeof (float))
-	      deprecated_set_value_type (val_args[nargs], builtin_type_float);
+	      val_args[nargs]->type = builtin_type_float;
 	    if (TYPE_LENGTH (type) == sizeof (double))
-	      deprecated_set_value_type (val_args[nargs], builtin_type_double);
+	      val_args[nargs]->type = builtin_type_double;
 	  }
 	nargs++;
 	s = s1;
diff -urN ./clean/src/gdb/tracepoint.c ./deprecated-removed/src/gdb/tracepoint.c
--- ./clean/src/gdb/tracepoint.c	2007-10-27 01:34:48.000000000 +0100
+++ ./deprecated-removed/src/gdb/tracepoint.c	2007-11-13 00:16:34.000000000 +0000
@@ -296,7 +296,9 @@
       func_string = create_array_type (func_string,
 				       builtin_type_char, func_range);
       func_val = allocate_value (func_string);
-      deprecated_set_value_type (func_val, func_string);
+
+      func_val->type = func_string;
+
       memcpy (value_contents_raw (func_val),
 	      DEPRECATED_SYMBOL_NAME (traceframe_fun),
 	      len);
@@ -318,7 +320,7 @@
       file_string = create_array_type (file_string,
 				       builtin_type_char, file_range);
       file_val = allocate_value (file_string);
-      deprecated_set_value_type (file_val, file_string);
+      file_val->type = file_string;
       memcpy (value_contents_raw (file_val),
 	      traceframe_sal.symtab->filename,
 	      len);
diff -urN ./clean/src/gdb/valops.c ./deprecated-removed/src/gdb/valops.c
--- ./clean/src/gdb/valops.c	2007-11-02 19:35:19.000000000 +0000
+++ ./deprecated-removed/src/gdb/valops.c	2007-11-13 00:19:25.000000000 +0000
@@ -224,7 +224,7 @@
 	  if (v)
 	    {
 	      v = value_addr (v);
-	      deprecated_set_value_type (v, type);
+	      v->type = type;
 	      return v;
 	    }
 	}
@@ -250,7 +250,7 @@
 
   /* No superclass found, just change the pointer type.  */
   arg2 = value_copy (arg2);
-  deprecated_set_value_type (arg2, type);
+  arg2->type = type;
   arg2 = value_change_enclosing_type (arg2, type);
   set_value_pointed_to_offset (arg2, 0);	/* pai: chk_val */
   return arg2;
@@ -307,10 +307,9 @@
 					  TYPE_TARGET_TYPE (range_type),
 					  low_bound,
 					  new_length + low_bound - 1);
-	  deprecated_set_value_type (arg2, 
-				     create_array_type ((struct type *) NULL,
-							element_type, 
-							range_type));
+	  arg2->type = create_array_type ((struct type *) NULL,
+					  element_type, 
+					  range_type);
 	  return arg2;
 	}
     }
@@ -351,7 +350,7 @@
 					     arg2, 0, type2, 1);
       if (v)
 	{
-	  deprecated_set_value_type (v, type);
+	  v->type = type;
 	  return v;
 	}
     }
@@ -423,7 +422,7 @@
 	return value_cast_pointers (type, arg2);
 
       arg2 = value_copy (arg2);
-      deprecated_set_value_type (arg2, type);
+      arg2->type = type;
       arg2 = value_change_enclosing_type (arg2, type);
       set_value_pointed_to_offset (arg2, 0);	/* pai: chk_val */
       return arg2;
@@ -739,7 +738,7 @@
   val = value_copy (toval);
   memcpy (value_contents_raw (val), value_contents (fromval),
 	  TYPE_LENGTH (type));
-  deprecated_set_value_type (val, type);
+  val->type = type;
   val = value_change_enclosing_type (val, 
 				     value_enclosing_type (fromval));
   set_value_embedded_offset (val, value_embedded_offset (fromval));
@@ -867,8 +866,7 @@
          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->type = lookup_pointer_type (TYPE_TARGET_TYPE (type));
       return arg2;
     }
   if (TYPE_CODE (type) == TYPE_CODE_FUNC)
@@ -905,7 +903,7 @@
     return arg1;
 
   arg2 = value_addr (arg1);
-  deprecated_set_value_type (arg2, lookup_reference_type (type));
+  arg2->type = lookup_reference_type (type);
   return arg2;
 }
 
@@ -950,7 +948,7 @@
 			       - value_pointed_to_offset (arg1)));
 
       /* Re-adjust type.  */
-      deprecated_set_value_type (arg2, TYPE_TARGET_TYPE (base_type));
+      arg2->type = TYPE_TARGET_TYPE (base_type);
       /* Add embedding info.  */
       arg2 = value_change_enclosing_type (arg2, enc_type);
       set_value_embedded_offset (arg2, value_pointed_to_offset (arg1));
@@ -2596,7 +2594,7 @@
      value_rtti_type used for its computation.  */
   new_val = value_at_lazy (real_type, VALUE_ADDRESS (argp) - top +
 			   (using_enc ? 0 : value_embedded_offset (argp)));
-  deprecated_set_value_type (new_val, value_type (argp));
+  new_val->type = value_type (argp);
   set_value_embedded_offset (new_val, (using_enc
 				       ? top + value_embedded_offset (argp)
 				       : top));
diff -urN ./clean/src/gdb/value.c ./deprecated-removed/src/gdb/value.c
--- ./clean/src/gdb/value.c	2007-10-25 19:01:58.000000000 +0100
+++ ./deprecated-removed/src/gdb/value.c	2007-11-13 00:17:14.000000000 +0000
@@ -40,142 +40,6 @@
 
 void _initialize_values (void);
 
-struct value
-{
-  /* Type of value; either not an lval, or one of the various
-     different possible kinds of lval.  */
-  enum lval_type lval;
-
-  /* Is it modifiable?  Only relevant if lval != not_lval.  */
-  int modifiable;
-
-  /* Location of value (if lval).  */
-  union
-  {
-    /* If lval == lval_memory, this is the address in the inferior.
-       If lval == lval_register, this is the byte offset into the
-       registers structure.  */
-    CORE_ADDR address;
-
-    /* Pointer to internal variable.  */
-    struct internalvar *internalvar;
-  } location;
-
-  /* Describes offset of a value within lval of a structure in bytes.
-     If lval == lval_memory, this is an offset to the address.  If
-     lval == lval_register, this is a further offset from
-     location.address within the registers structure.  Note also the
-     member embedded_offset below.  */
-  int offset;
-
-  /* Only used for bitfields; number of bits contained in them.  */
-  int bitsize;
-
-  /* Only used for bitfields; position of start of field.  For
-     BITS_BIG_ENDIAN=0 targets, it is the position of the LSB.  For
-     BITS_BIG_ENDIAN=1 targets, it is the position of the MSB. */
-  int bitpos;
-
-  /* Frame register value is relative to.  This will be described in
-     the lval enum above as "lval_register".  */
-  struct frame_id frame_id;
-
-  /* Type of the value.  */
-  struct type *type;
-
-  /* If a value represents a C++ object, then the `type' field gives
-     the object's compile-time type.  If the object actually belongs
-     to some class derived from `type', perhaps with other base
-     classes and additional members, then `type' is just a subobject
-     of the real thing, and the full object is probably larger than
-     `type' would suggest.
-
-     If `type' is a dynamic class (i.e. one with a vtable), then GDB
-     can actually determine the object's run-time type by looking at
-     the run-time type information in the vtable.  When this
-     information is available, we may elect to read in the entire
-     object, for several reasons:
-
-     - When printing the value, the user would probably rather see the
-     full object, not just the limited portion apparent from the
-     compile-time type.
-
-     - If `type' has virtual base classes, then even printing `type'
-     alone may require reaching outside the `type' portion of the
-     object to wherever the virtual base class has been stored.
-
-     When we store the entire object, `enclosing_type' is the run-time
-     type -- the complete object -- and `embedded_offset' is the
-     offset of `type' within that larger type, in bytes.  The
-     value_contents() macro takes `embedded_offset' into account, so
-     most GDB code continues to see the `type' portion of the value,
-     just as the inferior would.
-
-     If `type' is a pointer to an object, then `enclosing_type' is a
-     pointer to the object's run-time type, and `pointed_to_offset' is
-     the offset in bytes from the full object to the pointed-to object
-     -- that is, the value `embedded_offset' would have if we followed
-     the pointer and fetched the complete object.  (I don't really see
-     the point.  Why not just determine the run-time type when you
-     indirect, and avoid the special case?  The contents don't matter
-     until you indirect anyway.)
-
-     If we're not doing anything fancy, `enclosing_type' is equal to
-     `type', and `embedded_offset' is zero, so everything works
-     normally.  */
-  struct type *enclosing_type;
-  int embedded_offset;
-  int pointed_to_offset;
-
-  /* Values are stored in a chain, so that they can be deleted easily
-     over calls to the inferior.  Values assigned to internal
-     variables or put into the value history are taken off this
-     list.  */
-  struct value *next;
-
-  /* Register number if the value is from a register.  */
-  short regnum;
-
-  /* If zero, contents of this value are in the contents field.  If
-     nonzero, contents are in inferior memory at address in the
-     location.address field plus the offset field (and the lval field
-     should be lval_memory).
-
-     WARNING: This field is used by the code which handles watchpoints
-     (see breakpoint.c) to decide whether a particular value can be
-     watched by hardware watchpoints.  If the lazy flag is set for
-     some member of a value chain, it is assumed that this member of
-     the chain doesn't need to be watched as part of watching the
-     value itself.  This is how GDB avoids watching the entire struct
-     or array when the user wants to watch a single struct member or
-     array element.  If you ever change the way lazy flag is set and
-     reset, be sure to consider this use as well!  */
-  char lazy;
-
-  /* If nonzero, this is the value of a variable which does not
-     actually exist in the program.  */
-  char optimized_out;
-
-  /* If value is a variable, is it initialized or not.  */
-  int initialized;
-
-  /* Actual contents of the value.  For use of this value; setting it
-     uses the stuff above.  Not valid if lazy is nonzero.  Target
-     byte-order.  We force it to be aligned properly for any possible
-     value.  Note that a value therefore extends beyond what is
-     declared here.  */
-  union
-  {
-    gdb_byte contents[1];
-    DOUBLEST force_doublest_align;
-    LONGEST force_longest_align;
-    CORE_ADDR force_core_addr_align;
-    void *force_pointer_align;
-  } aligner;
-  /* Do not add any new members here -- contents above will trash
-     them.  */
-};
-
 /* Prototypes for local functions. */
 
 static void show_values (char *, int);
@@ -269,11 +133,6 @@
 {
   return value->type;
 }
-void
-deprecated_set_value_type (struct value *value, struct type *type)
-{
-  value->type = type;
-}
 
 int
 value_offset (struct value *value)
diff -urN ./clean/src/gdb/value.h ./deprecated-removed/src/gdb/value.h
--- ./clean/src/gdb/value.h	2007-11-02 19:35:19.000000000 +0000
+++ ./deprecated-removed/src/gdb/value.h	2007-11-13 00:18:38.000000000 +0000
@@ -25,6 +25,142 @@
 #include "doublest.h"
 #include "frame.h"		/* For struct frame_id.  */
 
+struct value
+{
+  /* Type of value; either not an lval, or one of the various
+     different possible kinds of lval.  */
+  enum lval_type lval;
+
+  /* Is it modifiable?  Only relevant if lval != not_lval.  */
+  int modifiable;
+
+  /* Location of value (if lval).  */
+  union
+  {
+    /* If lval == lval_memory, this is the address in the inferior.
+       If lval == lval_register, this is the byte offset into the
+       registers structure.  */
+    CORE_ADDR address;
+
+    /* Pointer to internal variable.  */
+    struct internalvar *internalvar;
+  } location;
+
+  /* Describes offset of a value within lval of a structure in bytes.
+     If lval == lval_memory, this is an offset to the address.  If
+     lval == lval_register, this is a further offset from
+     location.address within the registers structure.  Note also the
+     member embedded_offset below.  */
+  int offset;
+
+  /* Only used for bitfields; number of bits contained in them.  */
+  int bitsize;
+
+  /* Only used for bitfields; position of start of field.  For
+     BITS_BIG_ENDIAN=0 targets, it is the position of the LSB.  For
+     BITS_BIG_ENDIAN=1 targets, it is the position of the MSB. */
+  int bitpos;
+
+  /* Frame register value is relative to.  This will be described in
+     the lval enum above as "lval_register".  */
+  struct frame_id frame_id;
+
+  /* Type of the value.  */
+  struct type *type;
+
+  /* If a value represents a C++ object, then the `type' field gives
+     the object's compile-time type.  If the object actually belongs
+     to some class derived from `type', perhaps with other base
+     classes and additional members, then `type' is just a subobject
+     of the real thing, and the full object is probably larger than
+     `type' would suggest.
+
+     If `type' is a dynamic class (i.e. one with a vtable), then GDB
+     can actually determine the object's run-time type by looking at
+     the run-time type information in the vtable.  When this
+     information is available, we may elect to read in the entire
+     object, for several reasons:
+
+     - When printing the value, the user would probably rather see the
+     full object, not just the limited portion apparent from the
+     compile-time type.
+
+     - If `type' has virtual base classes, then even printing `type'
+     alone may require reaching outside the `type' portion of the
+     object to wherever the virtual base class has been stored.
+
+     When we store the entire object, `enclosing_type' is the run-time
+     type -- the complete object -- and `embedded_offset' is the
+     offset of `type' within that larger type, in bytes.  The
+     value_contents() macro takes `embedded_offset' into account, so
+     most GDB code continues to see the `type' portion of the value,
+     just as the inferior would.
+
+     If `type' is a pointer to an object, then `enclosing_type' is a
+     pointer to the object's run-time type, and `pointed_to_offset' is
+     the offset in bytes from the full object to the pointed-to object
+     -- that is, the value `embedded_offset' would have if we followed
+     the pointer and fetched the complete object.  (I don't really see
+     the point.  Why not just determine the run-time type when you
+     indirect, and avoid the special case?  The contents don't matter
+     until you indirect anyway.)
+
+     If we're not doing anything fancy, `enclosing_type' is equal to
+     `type', and `embedded_offset' is zero, so everything works
+     normally.  */
+  struct type *enclosing_type;
+  int embedded_offset;
+  int pointed_to_offset;
+
+  /* Values are stored in a chain, so that they can be deleted easily
+     over calls to the inferior.  Values assigned to internal
+     variables or put into the value history are taken off this
+     list.  */
+  struct value *next;
+
+  /* Register number if the value is from a register.  */
+  short regnum;
+
+  /* If zero, contents of this value are in the contents field.  If
+     nonzero, contents are in inferior memory at address in the
+     location.address field plus the offset field (and the lval field
+     should be lval_memory).
+
+     WARNING: This field is used by the code which handles watchpoints
+     (see breakpoint.c) to decide whether a particular value can be
+     watched by hardware watchpoints.  If the lazy flag is set for
+     some member of a value chain, it is assumed that this member of
+     the chain doesn't need to be watched as part of watching the
+     value itself.  This is how GDB avoids watching the entire struct
+     or array when the user wants to watch a single struct member or
+     array element.  If you ever change the way lazy flag is set and
+     reset, be sure to consider this use as well!  */
+  char lazy;
+
+  /* If nonzero, this is the value of a variable which does not
+     actually exist in the program.  */
+  char optimized_out;
+
+  /* If value is a variable, is it initialized or not.  */
+  int initialized;
+
+  /* Actual contents of the value.  For use of this value; setting it
+     uses the stuff above.  Not valid if lazy is nonzero.  Target
+     byte-order.  We force it to be aligned properly for any possible
+     value.  Note that a value therefore extends beyond what is
+     declared here.  */
+  union
+  {
+    gdb_byte contents[1];
+    DOUBLEST force_doublest_align;
+    LONGEST force_longest_align;
+    CORE_ADDR force_core_addr_align;
+    void *force_pointer_align;
+  } aligner;
+  /* Do not add any new members here -- contents above will trash
+     them.  */
+};
+
 struct block;
 struct expression;
 struct regcache;
@@ -32,13 +168,6 @@
 struct type;
 struct ui_file;
 
-/* The structure which defines the type of a value.  It should never
-   be possible for a program lval value to survive over a call to the
-   inferior (i.e. to be put into the history list or an internal
-   variable).  */
-
-struct value;
-
 /* Values are stored in a chain, so that they can be deleted easily
    over calls to the inferior.  Values assigned to internal variables
    or put into the value history are taken off this list.  */
@@ -49,13 +178,6 @@
 
 extern struct type *value_type (struct value *);
 
-/* This is being used to change the type of an existing value, that
-   code should instead be creating a new value with the changed type
-   (but possibly shared content).  */
-
-extern void deprecated_set_value_type (struct value *value,
-				       struct type *type);
-
 /* Only used for bitfields; number of bits contained in them.  */
 
 extern int value_bitsize (struct value *);

Re: Remove deprecated_set_value_type

[Daniel Jacobowitz]

On Tue, Nov 13, 2007 at 10:01:11AM +0000, Rob Quill wrote:
> Hi,
> 
> This patch removes the deprecated_set_value_type() function from
> value.c, value.h and all other places which use it.
> 
> I also plan to try and remove the other deprecated functions from value.c

Sorry, but you've missed the point.  It was deprecated because you're
not supposed to bang on a value's type after it is created (or at
least, that's the hope).  And struct value was moved from the header
to the C file in order to stop other places from poking at it.

So this is going the wrong direction :-(

> -/* This is being used to change the type of an existing value, that
> -   code should instead be creating a new value with the changed type
> -   (but possibly shared content).  */

As that says.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Remove deprecated_set_value_type

[Rob Quill]

On 13/11/2007, Daniel Jacobowitz <drow@false.org> wrote:
> On Tue, Nov 13, 2007 at 10:01:11AM +0000, Rob Quill wrote:
> > Hi,
> >
> > This patch removes the deprecated_set_value_type() function from
> > value.c, value.h and all other places which use it.
> >
> > I also plan to try and remove the other deprecated functions from value.c
>
> Sorry, but you've missed the point.  It was deprecated because you're
> not supposed to bang on a value's type after it is created (or at
> least, that's the hope).  And struct value was moved from the header
> to the C file in order to stop other places from poking at it.
>
> So this is going the wrong direction :-(
>
> > -/* This is being used to change the type of an existing value, that
> > -   code should instead be creating a new value with the changed type
> > -   (but possibly shared content).  */
>
> As that says.

Ah, ok, I thought it seemed a bit simple. I'll try and fix it properly now :)

Rob

Re: Remove deprecated_set_value_type

[Rob Quill]

On 13/11/2007, Rob Quill <rob.quill@gmail.com> wrote:
> On 13/11/2007, Daniel Jacobowitz <drow@false.org> wrote:
> > On Tue, Nov 13, 2007 at 10:01:11AM +0000, Rob Quill wrote:
> > > Hi,
> > >
> > > This patch removes the deprecated_set_value_type() function from
> > > value.c, value.h and all other places which use it.
> > >
> > > I also plan to try and remove the other deprecated functions from value.c
> >
> > Sorry, but you've missed the point.  It was deprecated because you're
> > not supposed to bang on a value's type after it is created (or at
> > least, that's the hope).  And struct value was moved from the header
> > to the C file in order to stop other places from poking at it.
> >
> > So this is going the wrong direction :-(
> >
> > > -/* This is being used to change the type of an existing value, that
> > > -   code should instead be creating a new value with the changed type
> > > -   (but possibly shared content).  */
> >
> > As that says.

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.

Thanks for your help,
Rob

Re: Remove deprecated_set_value_type

[Daniel Jacobowitz]

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?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Remove deprecated_set_value_type

[Rob Quill]

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?

Yep, I think so :) I'll look into this later today or tomorrow.

Rob

Re: Remove deprecated_set_value_type

[Rob Quill]

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