Re: [RFA 2/6] Change representation of psymbol to flush out accessors

[Keith Seitz <keiths at redhat dot com>]

On 05/03/2018 03:36 PM, Tom Tromey wrote:
> ChangeLog
> 2018-05-03  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf-index-write.c (write_psymbols, debug_names::insert)
> 	(debug_names::write_psymbols): Update.
> 	* psympriv.h (struct partial_symbol) <pginfo>: Rename from
> 	'ginfo'.
> 	(PSYMBOL_VALUE, PSYMBOL_VALUE_ADDRESS, PSYMBOL_LANGUAGE)
> 	(PSYMBOL_SECTION, PSYMBOL_OBJ_SECTION, PSYMBOL_SET_LANGUAGE)
> 	(PSYMBOL_SET_NAMES, PSYMBOL_LINKAGE_NAME, PSYMBOL_DEMANGLED_NAME)
> 	(PSYMBOL_SEARCH_NAME, PSYMBOL_MATCHES_SEARCH_NAME): New macros.
> 	* psymtab.c (find_pc_sect_psymtab_closer, find_pc_sect_psymtab)
> 	(find_pc_sect_psymbol, fixup_psymbol_section)
> 	(match_partial_symbol, lookup_partial_symbol, relocate_psymtabs)
> 	(print_partial_symbols, recursively_search_psymtabs)
> 	(compare_psymbols, psymbol_hash, psymbol_compare)
> 	(add_psymbol_to_bcache, maintenance_check_psymtabs)
> 	(psymbol_name_matches, psym_fill_psymbol_map): Update.

One trivial comment, otherwise IANAM, but LGTM.

Keith

> diff --git a/gdb/psymtab.c b/gdb/psymtab.c
> index ac0ee0a5a6..e9a6a23b9d 100644
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -1621,17 +1623,17 @@ psymbol_hash (const void *addr, int length)
>  {
>    unsigned long h = 0;
>    struct partial_symbol *psymbol = (struct partial_symbol *) addr;
> -  unsigned int lang = psymbol->ginfo.language;
> +  unsigned int lang = PSYMBOL_LANGUAGE (psymbol);
>    unsigned int domain = PSYMBOL_DOMAIN (psymbol);
>    unsigned int theclass = PSYMBOL_CLASS (psymbol);
>  
> -  h = hash_continue (&psymbol->ginfo.value, sizeof (psymbol->ginfo.value), h);
> +  h = hash_continue (&psymbol->pginfo.value, sizeof (psymbol->pginfo.value), h);
>    h = hash_continue (&lang, sizeof (unsigned int), h);
>    h = hash_continue (&domain, sizeof (unsigned int), h);
>    h = hash_continue (&theclass, sizeof (unsigned int), h);
>    /* Note that psymbol names are interned via symbol_set_names, so
>       there's no need to hash the contents of the name here.  */
> -  h = hash_continue (&psymbol->ginfo.name, sizeof (psymbol->ginfo.name), h);
> +  h = hash_continue (&psymbol->pginfo.name, sizeof (psymbol->pginfo.name), h);

I realize this is just a renaming of the existing code, but since there is an accessor for `name', I think it better to use it (in case someone ends up grepping for this).

>  
>    return h;
>  }
> @@ -1646,15 +1648,15 @@ psymbol_compare (const void *addr1, const void *addr2, int length)
>    struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
>    struct partial_symbol *sym2 = (struct partial_symbol *) addr2;
>  
> -  return (memcmp (&sym1->ginfo.value, &sym2->ginfo.value,
> -                  sizeof (sym1->ginfo.value)) == 0
> -	  && sym1->ginfo.language == sym2->ginfo.language
> +  return (memcmp (&sym1->pginfo.value, &sym2->pginfo.value,
> +                  sizeof (sym1->pginfo.value)) == 0
> +	  && sym1->pginfo.language == sym2->pginfo.language
>            && PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2)
>            && PSYMBOL_CLASS (sym1) == PSYMBOL_CLASS (sym2)
>  	  /* Note that psymbol names are interned via
>  	     symbol_set_names, so there's no need to compare the
>  	     contents of the name here.  */
> -          && sym1->ginfo.name == sym2->ginfo.name);
> +          && sym1->pginfo.name == sym2->pginfo.name);
>  }

Same here for PSYMBOL_LANGUAGE and PSYMBOL_LINKAGE_NAME?

>  /* Initialize a partial symbol bcache.  */
Keith