Re: [RFA] special case (anonymous namespace)::foo in linespec.c

[Keith Seitz <keiths at redhat dot com>]

Hi, Doug,

This patch makes sense to me. When I rewrote most of the linespec 
parser, I (perhaps too ignorantly) re-used as much existing code as 
possible.

As you've discovered, there are some performance issues that I 
overlooked. I suspect there are a lot more hidden in here, too.

I really only have two /really/ minor comments about this patch. [And I 
*do* mean minor. Feel free to ignore them/me!]

On 02/01/2013 04:58 PM, Doug Evans wrote:
> +  /* It's important to not call expand_symtabs_matching unnecessarily
> +     as it can really slow things down (by unnecessarily expanding
> +     potentially 1000s of symtabs).  If we know we only want classes,
> +     we'd like to skip symtabs where NAME isn't a class.  However,
> +     NAME could be a class in some symtabs and a namespace in others,
> +     and we can't tell which until we expand the symtab!  Ugh!
> +     What we need is for "quick" symbols (partial syms and .gdb_index)
> +     to at least record whether a symbol is a class.  Until then, there's
> +     not much we can do.  One thing we can do is watch for "(anonymous
> +     namespace)" which is actually significant enough to warrant it.
> +     That will at least handle "b [foo::](anonymous namespace)::bar".
> +     In some apps this can speed up the time to set the breakpoint from
> +     100s of seconds to a few seconds.  */

That's a really long comment. Might I recommend breaking this into more 
paragraphs to make this comment a bit easier to digest? You've presented 
three important comments here; 1) the problem; 2) the ideal solution; 3) 
what we can actually do today.

> +      if (strcmp_iw (klass, CP_ANONYMOUS_NAMESPACE_STR) == 0)
> +	have_anon_ns = 1;
> +    }
> +

You can use cp_is_anonymous for this, too. That's declared in 
cp-support.h which is already included in this file.

I recommend that you commit this patch.

Keith