Re: [rfa] namespace scope, take 2

[Daniel Jacobowitz <drow@mvista.com>]

On Sun, May 18, 2003 at 03:56:21PM -0700, David Carlton wrote:
> Here's a new version of my namespace scope patch, using the language
> hook that I just posted an RFA for.  Assuming that's okay, then is
> this okay, Daniel?  I know you said that it was fine a few weeks ago,
> but I wanted to give you another chance to complain since it's going
> in a file you maintain. :-)  (Except for a few functions in block.c,
> but Elena already said those were okay.  Well, block_global_block is
> new to this version of the patch, but I'm sure it's
> non-controversial.)  I added the extra test to namespace.exp that you
> requested.

Just a couple small comments.  Most of it looks good.

> +/* Lookup NAME at namespace scope (or, in C terms, in static and
> +   global variables).  SCOPE is the namespace that the current
> +   function is defined within; only consider namespaces whose length
> +   is at least SCOPE_LEN.  (This is to make the recursion easier.)  */
> +
> +static struct symbol *
> +lookup_namespace_scope (const char *name,
> +			const char *linkage_name,
> +			const struct block *block,
> +			const domain_enum domain,
> +			struct symtab **symtab,
> +			const char *scope,
> +			int scope_len)
> +{
> +  char *namespace;
> +
> +  if (scope[scope_len] != '\0')
> +    {
> +      /* Recursively search for names in child namespaces first.  */
> +
> +      struct symbol *sym;
> +      int new_scope_len = scope_len;
> +
> +      /* If the current scope is followed by "::", skip past that.  */
> +      if (new_scope_len != 0)
> +	{
> +	  gdb_assert (scope[new_scope_len] == ':');
> +	  new_scope_len += 2;
> +	}
> +      new_scope_len += cp_find_first_component (scope + new_scope_len);
> +      sym = lookup_namespace_scope (name, linkage_name, block,
> +				    domain, symtab,
> +				    scope, new_scope_len);
> +      if (sym != NULL)
> +	return sym;
> +    }
> +
> +  /* Okay, we didn't find a match in our parents, so look for the name
> +     in the current namespace.  */

I don't think you mean "parents" here - isn't it children instead?

> +  namespace = alloca (scope_len + 1);
> +  strncpy (namespace, scope, scope_len);
> +  namespace[scope_len] = '\0';
> +  return cp_lookup_symbol_namespace (namespace, name, linkage_name,
> +				     block, domain, symtab);
> +}


> +/* Look up NAME in BLOCK's static block and in global blocks.  If
> +   ANONYMOUS_NAMESPACE is nonzero, don't look in other files' global
> +   blocks, just in the one belonging to this file.  */

Hmm, would you mind breaking this up into two comments?  One before the
function which says what ANONYMOUS_NAMESPACE means, and one in the
function which describes what it does.

Also, you're only documenting three of the six arguments to this
function.  Similarly for a lot of the other functions.

Aside from comment corrections, if your previous patch is approved this
one is also.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer