Re: [patch 1/2] Perform a namespace lookup at every block level

[Tom Tromey <tromey@redhat.com>]

>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:

Sami> Same as above, but I fixed a couple of issues with the previous patch.
Sami> The first issue is that even while we iterate over blocks the original
Sami> scope should be used. Secondly, the call to lookup_symbol_name space
Sami> in valops.c must also perform a per block search. The final issue is
Sami> recursive search which is discussed in part 2 of this patch.

Sorry about the delay on this.  For best results, ping a pending patch
every week or two -- the lack of a response can mean either that
everybody is busy or that everybody somehow missed the posting entirely,
and there's no way to tell the difference.

Sami> @@ -265,8 +277,41 @@ cp_lookup_symbol_nonlocal (const char *name,
[...]
Sami> +  if ( sym != NULL)

Extra space after the "(".

Sami> +  return cp_lookup_symbol_namespace(scope, name, linkage_name, block, domain);

Missing a space before "(".  There are several of these.

Sami> +static struct symbol *
Sami> +cp_lookup_symbol_imports (const char *scope,
Sami> +                            const char *name,
Sami> +                            const char *linkage_name,
Sami> +                            const struct block *block,
Sami> +                            const domain_enum domain)
[...]
Sami> +  if ( sym != NULL)

The extra space again :)

Sami> @@ -1304,13 +1304,14 @@ lookup_symbol_aux (const char *name, const char *linkage_name,
[...]
Sami> +      /* C++ language specific lookup requires the lowest block level.  */
Sami> +      if (language != language_cplus)
Sami> +        block = function_block;

I like to avoid special cases for a specific language in the generic
code.  These make gdb more brittle over time; also they make it harder
to understand.

One alternative approach would be to pass `block' as an argument to the
la_lookup_symbol_nonlocal method.  This would mean adding an ignored
parameter to most of the implementations of this method, but that is no
big deal, we've done that sort of thing before.

Sami> diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc b/gdb/testsuite/gdb.cp/namespace-using.cc

I think this needs more tests.  At the very least it should have a test
case for the bug Daniel pointed out upthread, but I think it would be
good to have tests for other confounding cases.

Tom