Re: [patch 1/2] Search typedefs in namespaces also in other files

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> currently you can access static symbols from not-current CUs
Jan> (Compilation Units).  In some cases you cannot if it is a name
Jan> contained in namespace.

Jan> It is questionable how it should behave.  Such access is already
Jan> not compliant with the C++ standard.  The implementation tries only
Jan> to search for the fully qualified names - without any attempts for
Jan> imported namespaces.

Well... AFAIK, types don't have linkage.  And, in C++, due to the ODR,
presumably anything like this must have a single meaning (though of
course in reality we probably have fun cases with hidden symbols in .sos
and whatnot).  Finally, GCC may omit some debuginfo that it thinks is
not useful in the current CU... so in some cases this patch may actually
improve conformance; and anyway it is similar to what gdb already does,
and also helpful.

So I am inclined to think it is ok.

It is true that it can yield confusing results, just as gdb already can
in some cases.  I think that must be reasonably rare, because I haven't
seen any bug reports about it.  In any case I think the way forward is
to fix those cases somehow, not limit new ones.

Jan> Already the C++ parser calls various functions in not so correct
Jan> way IMHO

I'm curious to know what.

Jan> 	[Bug c++/11703] New: root namespace sometimes not working
Jan> 	 - Leading :: works only for types, not other objects.
Jan> 	 - IMHO a fix requires patching the C++ parser: -> archer-cpparser

I think it could probably be solved, but that isn't too important.
We'll fix it one way or another.  Thanks for filing that.

Jan> +  /* Now search all static file-level symbols.  Not strictly correct,
Jan> +     but more useful than an error.  We do not try to guess any imported
Jan> +     namespace as even the fully specified namespace seach is is already not
Jan> +     C++ compliant and more assumptions could make it too magic.  */
Jan> +
Jan> +  if (scope[0] == '\0')
Jan> +    {
Jan> +      sym = lookup_static_symbol_aux (name, domain);

I'm having trouble following all the logic.

I think lookup_symbol_aux calls la_lookup_symbol_nonlocal,
which for C++ calls cp_lookup_symbol_nonlocal, which calls
cp_lookup_symbol_namespace.

But maybe this call is needed here for other callers of
cp_lookup_symbol_namespace?

The symbol lookup stuff is starting to feel spaghetti-ish to me.

Jan> @@ -1121,11 +1120,20 @@ lookup_symbol_aux (const char *name, const struct block *block,
Jan>    if (sym != NULL)
Jan>      return sym;
 
Jan> -  /* Now search all static file-level symbols.  Not strictly correct,
Jan> -     but more useful than an error.  Do the symtabs first, then check
Jan> -     the psymtabs.  If a psymtab indicates the existence of the
Jan> -     desired name as a file-level static, then do psymtab-to-symtab
Jan> -     conversion on the fly and return the found symbol. */
Jan> +  return lookup_static_symbol_aux (name, domain);
Jan> +}
Jan> +
Jan> +/* Now search all static file-level symbols.  Not strictly correct,
Jan> +   but more useful than an error.  Do the symtabs first, then check
Jan> +   the psymtabs.  If a psymtab indicates the existence of the
Jan> +   desired name as a file-level static, then do psymtab-to-symtab
Jan> +   conversion on the fly and return the found symbol. */
Jan> +
Jan> +struct symbol *
Jan> +lookup_static_symbol_aux (const char *name, const domain_enum domain)
Jan> +{
Jan> +  struct objfile *objfile;
Jan> +  struct symbol *sym;

I think you should leave that comment in the previous function and write
a new comment for lookup_static_symbol_aux.

Tom