[v2,4/8] Search global block from basic_lookup_symbol_nonlocal

  This changes basic_lookup_symbol_nonlocal to look in the global block
of the passed-in block.  If no block was passed in, it reverts to the
previous behavior.

This change is needed to ensure that 'FILENAME'::NAME lookups work
properly.  As debugging Pedro's test case showed, this was not working
properly in the case where multiple identical names could be found
(the one situation where this feature is truly needed :-).

This also removes some old comments from basic_lookup_symbol_nonlocal
that no longer apply once this patch goes in.

gdb/ChangeLog
2019-08-01  Tom Tromey  <tromey@adacore.com>

	* symtab.c (basic_lookup_symbol_nonlocal): Search global block.
	Remove old comments.
---
 gdb/ChangeLog |  5 +++++
 gdb/symtab.c  | 44 ++++++++++++++++----------------------------
 2 files changed, 21 insertions(+), 28 deletions(-)
  

* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:08 -0600]:

> This changes basic_lookup_symbol_nonlocal to look in the global block
> of the passed-in block.  If no block was passed in, it reverts to the
> previous behavior.
> 
> This change is needed to ensure that 'FILENAME'::NAME lookups work
> properly.  As debugging Pedro's test case showed, this was not working
> properly in the case where multiple identical names could be found
> (the one situation where this feature is truly needed :-).
> 
> This also removes some old comments from basic_lookup_symbol_nonlocal
> that no longer apply once this patch goes in.

So I guess the tests for this are going to be in the
gdb.base/print-file-var.exp changes that are part of patch #8.  It
would be great if the commit message could mention this - it just
makes life easier later on.

I wonder if we need to update other *_lookup_symbol_nonlocal functions
in a similar way?  For example can the C tests be compiled as C++,
which should cause GDB to use cp_lookup_symbol_nonlocal.

Looking at both basic_lookup_symbol_nonlocal and
cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into
lookup_global_symbol?  And just have 'block_global_block (block)'
checked before the search of all global blocks?

Some other languages provide their own *_lookup_symbol_nonlocal, I
don't know if these would also need fixing.

Thanks,
Andrew


> 
> gdb/ChangeLog
> 2019-08-01  Tom Tromey  <tromey@adacore.com>
> 
> 	* symtab.c (basic_lookup_symbol_nonlocal): Search global block.
> 	Remove old comments.
> ---
>  gdb/ChangeLog |  5 +++++
>  gdb/symtab.c  | 44 ++++++++++++++++----------------------------
>  2 files changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 0ff212e0d97..b8f33509c09 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
>  {
>    struct block_symbol result;
>  
> -  /* NOTE: carlton/2003-05-19: The comments below were written when
> -     this (or what turned into this) was part of lookup_symbol_aux;
> -     I'm much less worried about these questions now, since these
> -     decisions have turned out well, but I leave these comments here
> -     for posterity.  */
> -
> -  /* NOTE: carlton/2002-12-05: There is a question as to whether or
> -     not it would be appropriate to search the current global block
> -     here as well.  (That's what this code used to do before the
> -     is_a_field_of_this check was moved up.)  On the one hand, it's
> -     redundant with the lookup in all objfiles search that happens
> -     next.  On the other hand, if decode_line_1 is passed an argument
> -     like filename:var, then the user presumably wants 'var' to be
> -     searched for in filename.  On the third hand, there shouldn't be
> -     multiple global variables all of which are named 'var', and it's
> -     not like decode_line_1 has ever restricted its search to only
> -     global variables in a single filename.  All in all, only
> -     searching the static block here seems best: it's correct and it's
> -     cleanest.  */
> -
> -  /* NOTE: carlton/2002-12-05: There's also a possible performance
> -     issue here: if you usually search for global symbols in the
> -     current file, then it would be slightly better to search the
> -     current global block before searching all the symtabs.  But there
> -     are other factors that have a much greater effect on performance
> -     than that one, so I don't think we should worry about that for
> -     now.  */
> -
>    /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
>       the current objfile.  Searching the current objfile first is useful
>       for both matching user expectations as well as performance.  */
> @@ -2453,6 +2425,22 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
>    if (result.symbol != NULL)
>      return result;
>  
> +  /* If a block was passed in, we want to search the corresponding
> +     global block now.  This yields "more expected" behavior, and is
> +     needed to support 'FILENAME'::VARIABLE lookups.  */
> +  const struct block *global_block = block_global_block (block);
> +  if (global_block != nullptr)
> +    {
> +      result.symbol = lookup_symbol_in_block (name,
> +					      symbol_name_match_type::FULL,
> +					      global_block, domain);
> +      if (result.symbol != nullptr)
> +	{
> +	  result.block = global_block;
> +	  return result;
> +	}
> +    }
> +
>    /* If we didn't find a definition for a builtin type in the static block,
>       search for it now.  This is actually the right thing to do and can be
>       a massive performance win.  E.g., when debugging a program with lots of
> -- 
> 2.20.1
>