* [patch] value_change_enclosing_type -> set_value_enclosing_type
@ 2010-11-06 18:52 Doug Evans
2010-11-08 16:41 ` Joel Brobecker
0 siblings, 1 reply; 2+ messages in thread
From: Doug Evans @ 2010-11-06 18:52 UTC (permalink / raw)
To: gdb-patches
Hi.
The comment for value_change_enclosing_type has this:
"The return value is a pointer to the new version of this value structure."
which is not true, the input value is modified as a side-effect.
Most callers essentially expect this side-effect. Sure they use the
result, but they always (except in one case) have just made a new value
so there's no need to create a new value object.
The exception is value_full_object.
It also has a similar comment:
"... if that is different from the enclosing type, create a new value ..."
This patch does two things:
- rename value_change_enclosing_type to set_value_enclosing_type,
and make the result void, for clarity and consistency with other
set_value_foo functions
- have value_full_object always return a new value if it needs to
modify the input value
I will check it in in a few days if there are no objections.
Tested on amd64-linux, no regressions.
2010-11-06 Doug Evans <dje@google.com>
* value.c (set_value_enclosing_type): Renamed from
value_change_enclosing_type. All callers updated.
* value.h (set_value_enclosing_type): Update.
* valops.c (value_full_object): Always return a copy if we need to
make changes to the input value.
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.256
diff -u -p -r1.256 valops.c
--- valops.c 4 Nov 2010 20:26:23 -0000 1.256
+++ valops.c 6 Nov 2010 18:38:16 -0000
@@ -335,7 +335,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_change_enclosing_type (arg2, type);
+ set_value_enclosing_type (arg2, type);
set_value_pointed_to_offset (arg2, 0); /* pai: chk_val */
return arg2;
}
@@ -569,7 +569,7 @@ value_cast (struct type *type, struct va
arg2 = value_copy (arg2);
deprecated_set_value_type (arg2, type);
- arg2 = value_change_enclosing_type (arg2, type);
+ set_value_enclosing_type (arg2, type);
set_value_pointed_to_offset (arg2, 0); /* pai: chk_val */
return arg2;
}
@@ -1139,11 +1139,9 @@ value_assign (struct value *toval, struc
case lval_internalvar:
set_internalvar (VALUE_INTERNALVAR (toval), fromval);
val = value_copy (fromval);
- val = value_change_enclosing_type (val,
- value_enclosing_type (fromval));
+ set_value_enclosing_type (val, value_enclosing_type (fromval));
set_value_embedded_offset (val, value_embedded_offset (fromval));
- set_value_pointed_to_offset (val,
- value_pointed_to_offset (fromval));
+ set_value_pointed_to_offset (val, value_pointed_to_offset (fromval));
return val;
case lval_internalvar_component:
@@ -1334,8 +1332,7 @@ value_assign (struct value *toval, struc
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_enclosing_type (val, value_enclosing_type (fromval));
set_value_embedded_offset (val, value_embedded_offset (fromval));
set_value_pointed_to_offset (val, value_pointed_to_offset (fromval));
@@ -1583,7 +1580,8 @@ value_addr (struct value *arg1)
/* This may be a pointer to a base subobject; so remember the
full derived object's type ... */
- arg2 = value_change_enclosing_type (arg2, lookup_pointer_type (value_enclosing_type (arg1)));
+ set_value_enclosing_type (arg2,
+ lookup_pointer_type (value_enclosing_type (arg1)));
/* ... and also the relative position of the subobject in the full
object. */
set_value_pointed_to_offset (arg2, value_embedded_offset (arg1));
@@ -1644,7 +1642,7 @@ value_ind (struct value *arg1)
/* Re-adjust type. */
deprecated_set_value_type (arg2, TYPE_TARGET_TYPE (base_type));
/* Add embedding info. */
- arg2 = value_change_enclosing_type (arg2, enc_type);
+ set_value_enclosing_type (arg2, enc_type);
set_value_embedded_offset (arg2, value_pointed_to_offset (arg1));
/* We may be pointing to an object of some derived type. */
@@ -3413,7 +3411,8 @@ value_full_object (struct value *argp,
/* pai: FIXME -- sounds iffy */
if (full)
{
- argp = value_change_enclosing_type (argp, real_type);
+ argp = value_copy (argp);
+ set_value_enclosing_type (argp, real_type);
return argp;
}
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.114
diff -u -p -r1.114 value.c
--- value.c 3 Nov 2010 13:49:37 -0000 1.114
+++ value.c 6 Nov 2010 18:38:16 -0000
@@ -1933,21 +1933,20 @@ value_static_field (struct type *type, i
return retval;
}
-/* Change the enclosing type of a value object VAL to NEW_ENCL_TYPE.
- You have to be careful here, since the size of the data area for the value
- is set by the length of the enclosing type. So if NEW_ENCL_TYPE is bigger
- than the old enclosing type, you have to allocate more space for the data.
- The return value is a pointer to the new version of this value structure. */
+/* Change the enclosing type of a value object VAL to NEW_ENCL_TYPE.
+ You have to be careful here, since the size of the data area for the value
+ is set by the length of the enclosing type. So if NEW_ENCL_TYPE is bigger
+ than the old enclosing type, you have to allocate more space for the
+ data. */
-struct value *
-value_change_enclosing_type (struct value *val, struct type *new_encl_type)
+void
+set_value_enclosing_type (struct value *val, struct type *new_encl_type)
{
if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val)))
val->contents =
(gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
val->enclosing_type = new_encl_type;
- return val;
}
/* Given a value ARG1 (offset by OFFSET bytes)
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.163
diff -u -p -r1.163 value.h
--- value.h 3 Nov 2010 13:49:37 -0000 1.163
+++ value.h 6 Nov 2010 18:38:16 -0000
@@ -136,8 +136,9 @@ extern void deprecated_set_value_modifia
normally. */
extern struct type *value_enclosing_type (struct value *);
-extern struct value *value_change_enclosing_type (struct value *val,
- struct type *new_type);
+extern void set_value_enclosing_type (struct value *val,
+ struct type *new_type);
+
extern int value_pointed_to_offset (struct value *value);
extern void set_value_pointed_to_offset (struct value *value, int val);
extern int value_embedded_offset (struct value *value);
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [patch] value_change_enclosing_type -> set_value_enclosing_type
2010-11-06 18:52 [patch] value_change_enclosing_type -> set_value_enclosing_type Doug Evans
@ 2010-11-08 16:41 ` Joel Brobecker
0 siblings, 0 replies; 2+ messages in thread
From: Joel Brobecker @ 2010-11-08 16:41 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
> This patch does two things:
> - rename value_change_enclosing_type to set_value_enclosing_type,
> and make the result void, for clarity and consistency with other
> set_value_foo functions
> - have value_full_object always return a new value if it needs to
> modify the input value
>
> I will check it in in a few days if there are no objections.
>
> Tested on amd64-linux, no regressions.
>
> 2010-11-06 Doug Evans <dje@google.com>
>
> * value.c (set_value_enclosing_type): Renamed from
> value_change_enclosing_type. All callers updated.
> * value.h (set_value_enclosing_type): Update.
> * valops.c (value_full_object): Always return a copy if we need to
> make changes to the input value.
FWIW, I like this change.
--
Joel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-11-08 16:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-06 18:52 [patch] value_change_enclosing_type -> set_value_enclosing_type Doug Evans
2010-11-08 16:41 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).