This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Do not add partial_symbol again and again to the list
- From: Daniel Jacobowitz <drow at false dot org>
- To: Aleksandar Ristovski <aristovski at qnx dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 3 May 2008 16:54:13 -0400
- Subject: Re: [patch] Do not add partial_symbol again and again to the list
- References: <20080211203809.GA29560@caradoc.them.org> <47B0B56F.4010607@qnx.com> <20080211210935.GA31767@caradoc.them.org> <47B0C0F4.4090302@qnx.com> <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> <47B27EB1.6030606@qnx.com>
On Wed, Feb 13, 2008 at 12:22:57AM -0500, Aleksandar Ristovski wrote:
> 3. lookup_partial_symbol
> Function looks up global or static list of objfile, but only subset of it
> determined by partial symtab's offset and number of symbols. Returns partial
> symbol found. This function is always called from a loop through all partial
> symtabs (this fact is important).
Ah! That's the part I didn't understand before.
I was worried about the construct tested in scope.exp, "print
'scope0.c'::filelocal". But it uses full symtabs, not partial
symtabs.
> 2008-02-12 Aleksandar Ristovski <aristovski@qnx.com>
>
> * bcache.c (bcache_data): Call bcache_added function.
> (bcache_added): New function name. Body of function bcache_data
> is used here with the addition of 'added' argument. The argument
> is optional and if specified, bcache_added returns 1 if the data
> was added and 0 if it was found in the hash.
Whenever you find yourself writing this sort of explanation in the
changelog, it's a sign that you should have a shorter changelog entry
and a long comment in the source code. Please move the explanation to
bcache.h.
> * symfile.c (add_psymbol_to_bcache): New helper function, takes part of
> work from add_psymbol_to_list - initialises partial symbol and stashes
> it in objfile's cache.
> (append_psymbol_to_list): New helper function, takes other part of
> work from add_psymbol_to_list - adds partial symbol to the given list.
> (add_psymbol_to_list): Call helper functions instead of doing work
> here. Functionally, the function hasn't changed.
> (add_psymbol_to_global_list): New function, adds partial symbol to
> global list and does it only once per objfile.
Same thing for these.
> +const void *
> +bcache (const void *addr, int length, struct bcache *bcache)
> +{
> + return bcache_data (addr, length, bcache);
> +}
> +
> +void *
> +bcache_added (const void *addr, int length, struct bcache *bcache,
> + int *added)
It should return a const pointer, like bcache. Also the indentation
on the second line is too deep.
> case DW_TAG_namespace:
> - add_psymbol_to_list (actual_name, strlen (actual_name),
> + add_psymbol_to_global_list (actual_name, strlen (actual_name),
> VAR_DOMAIN, LOC_TYPEDEF,
> - &objfile->global_psymbols,
> 0, (CORE_ADDR) 0, cu->language, objfile);
Please adjust indentation on the subsequent lines, since the open
paren moved over.
> if (cu->language == language_cplus
> || cu->language == language_java
> || cu->language == language_ada)
> {
> /* For C++ and Java, these implicitly act as typedefs as well. */
> - add_psymbol_to_list (actual_name, strlen (actual_name),
> - VAR_DOMAIN, LOC_TYPEDEF,
> - &objfile->global_psymbols,
> - 0, (CORE_ADDR) 0, cu->language, objfile);
> + add_psymbol_to_global_list (actual_name, strlen (actual_name),
> + VAR_DOMAIN, LOC_TYPEDEF,
> + 0, (CORE_ADDR) 0, cu->language, objfile);
> }
You've deleted this bit in your other patch fortunately :-)
> case DW_TAG_enumerator:
> - add_psymbol_to_list (actual_name, strlen (actual_name),
> + if (cu->language == language_cplus
> + || cu->language == language_java)
> + add_psymbol_to_global_list (actual_name, strlen (actual_name),
> + VAR_DOMAIN, LOC_CONST,
> + 0, (CORE_ADDR) 0, cu->language, objfile);
This part I don't understand. Why does the behavior of enumerators
depend on the language? The logic for finding them during lookup is
the same.
In fact, if lookup_partial_symbol will never find a global symbol
with an identical name why do we need them for any duplicated symbol,
even legitimately duplicated things like static functions? Well,
the check in cp-support.c is one reason for functions. And I can
imagine other valid reasons in the future. So functions are special.
But for things other than functions.
--
Daniel Jacobowitz
CodeSourcery