This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 00/12] (somewhat) clean up struct value ownership
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Fri, 6 Apr 2018 20:33:10 +0100
- Subject: Re: [RFA 00/12] (somewhat) clean up struct value ownership
- References: <20180405211507.6103-1-tom@tromey.com>
On 04/05/2018 10:14 PM, Tom Tromey wrote:
> struct value has long suffered from a complicated approach to
> ownership. In particular, values are reference counted, but they are
> also handled specially if they are on the "value chain". I believe
> this has led to bugs on occasion; and anyway requires oddities like
> release_value_or_incref.
>
> This series goes some way toward cleaning up ownership for values. It
> introduces a gdb_ref_ptr specialization for values and then changes
> various things to use it. After this, it cleans up various code doing
> manual memory mangement related to value; and in particular unifies
> the value chain with the reference counting mechanism.
>
This is seriously cool. Thanks much for doing this.
I've sent a few comments here and there, but overall this looks
good to me.
> There is still more work that could be done in this area. For
> example:
>
> * struct lval_funcs could be turned into a base class and then the
> implementers rewritten as ordinary objects.
>
> * Likewise struct internalvar_funcs; and this would allow better type
> safety through the removal of union internalvar_data.
>
> * Perhaps the "location" union could be removed from struct value,
> also providing more type safety.
>
Yeah, this would be nice.
> However, I didn't do these, as this series seemed to reach a
> reasonable stopping point.
Definitely agreed.
As I'm reading the series, I'm also wishing for making some of the
value-related free-functions methods of value instead, so that
instead of having to write ref_ptr::get() calls like in:
struct type *t = value_type (val.get ());
we'd be able to write instead:
struct type *t = val->type ();
Thanks,
Pedro Alves