Re: [RFC] Alternate approach to keeping convenience variables

[Jim Blandy <jimb@red-bean.com>]

On 12/9/05, Daniel Jacobowitz <drow@false.org> wrote:
> 2005-12-09  Daniel Jacobowitz  <dan@codesourcery.com>
>
>         * Makefile.in (gdbtypes_h, gdbtypes.o, utils.o): Update.
>         * defs.h (hashtab_obstack_allocate, dummy_obstack_deallocate): Add
>         prototypes.
>         * dwarf2read.c (read_subroutine_type): Use TYPE_ZALLOC.
>         (hashtab_obstack_allocate, dummy_obstack_deallocate): Moved to...
>         * utils.c (hashtab_obstack_allocate, dummy_obstack_deallocate):
>         ...here.
>         * gdbtypes.c: Include "hashtab.h".
>         (build_gdbtypes): Remove extra prototype.
>         (struct type_pair, type_pair_hash, type_pair_eq)
>         (create_copied_types_hash, copy_type_recursive): New.
>         * gdbtypes.h: Include "hashtab.h".
>         (TYPE_ZALLOC): New.
>         (create_copied_types_hash, copy_type_recursive): New prototypes.
>         * objfiles.c (free_objfile): Call preserve_values.
>         * symfile.c (reread_symbols): Likewise.
>         (clear_symtab_users): Remove calls to clear_value_history and
>         clear_internalvars.
>         * value.c (clear_value_history, clear_internalvars): Removed.
>         (preserve_one_value, preserve_values): New functions.
>         * value.h (clear_value_history, clear_internalvars): Removed.
>         (preserve_values): New prototype.
>
>         * tracepoint.c (_initialize_tracepoint): Do not initialize convenience
>         variables here.

Well, that's a head-on attack.  I think Mr. Stubbs will be pleased
when he comes back.

I don't understand what the tracepoint.c change has to do with the
rest of the patch.

Commit the removal of the extra build_gdbtypes prototype as a separate
obvious change.

Same for TYPE_ZALLOC.  Make this a function, not a macro.

It would be nice if the comments for hashtab_obstack_allocate and the
dummy free actually explained what sort of DATA argument they expect.

copy_type_recursive undoes the sharing given a bunch of 'struct type'
objects all referring to a given 'struct main_type' object.  You could
just stick both kinds of pointers in the type hash, at the cost of
some static typing.  And it doesn't preserve 'pointer_type',
'reference_type', or 'chain' groupings.

Go ahead and expand the comment in copy_type_recursive to spell out
why it is *necessary* to add the type pair to the hash table before
the type is completely constructed.  Don't just point out that we do
it where we do it.

Because they would have different lifetimes, we should never have a
type in one objfile pointing to a type in another objfile.  And if we
ever did, then that assert would catch it.  Good.  There should be a
comment to that effect in some appropriate place.

preserve_values also needs a comment indicating that it's meant to be
called only when we're about to free the given objfile, which ensures
that we never make more than one copy of a given type.