Re: [drow-cplus-branch] handle namespace scope

[Daniel Jacobowitz <drow at mvista dot com>]

On Tue, Nov 19, 2002 at 03:07:45PM -0800, David Carlton wrote:
> Here are some patches for drow-cplus-branch that teach lookup_symbol
> and buildsym.c about the concept of "namespace scope".  Basically, if
> you have a function like this
> 
> namespace A
> {
>   void foo ()
>   {
>     // body of foo
>   }
> }
> 
> then, when doing name lookup within A::foo, when you hit the part
> where you'd normally search the global environment, you first search
> namespace A and then search the global namespace.  My previous patches
> had faked this by behaving as if there were a using directive "using
> namespace A;" at the start of the body of foo; this patch does it
> right.
> 
> One side effect of this is that struct block now has to have room for
> the current namespace as well as the current using directives; I
> decided to implement this in such a way as to use a little more memory
> in the C++ case but a little less memory in the non-C++ case.  (This
> also lead to a bunch of block accessor functions; I put them in
> symtab.{c,h}, but the block stuff should really be in block.{c,h}.
> I've done that on my branch, but I won't worry about that in
> drow-cplus-branch until it's that way on the mainline.)
> 
> There's also a little bit of futzing around with lookup_symbol_aux, to
> treat minsyms in a better way than I treated them in my last patch.  I
> didn't include the patch from
> <http://sources.redhat.com/ml/gdb-patches/2002-11/msg00378.html> that
> moves the field_of_this check to the right place, since that's
> somewhat orthogonal to this issue, but if you want I can include that
> as well.
> 
> This should get everything important out of the way before I start
> trying to add symbols associated to namespaces...
> 
> Okay to commit?

The idea is sound, some nits...

> @@ -485,21 +483,36 @@ finish_block (struct symbol *symbol, str
>  	  const char *name = SYMBOL_CPLUS_DEMANGLED_NAME (symbol);
>  	  const char *next;
>  
> -	  for (next = cp_find_first_component (name);
> -	       *next == ':';
> -	       /* The '+ 2' is to skip the '::'.  */
> -	       next = cp_find_first_component (next + 2))
> +if (processing_has_namespace_info)

Mailer/patch glitch?  Careful of your indentation.

> +	    block_set_scope (block, processing_current_namespace,
> +			     &objfile->symbol_obstack);
> +	  else
>  	    {
> -	      BLOCK_USING (block)
> -		= cp_add_using_obstack (name, 0, next - name,
> -					BLOCK_USING (block),
> -					&objfile->symbol_obstack);
> -	    }
> +	      const char *current, *next;
>  
> -	  /* FIMXE: carlton/2002-10-09: Until I understand the
> -	     possible pitfalls of demangled names a lot better, I want
> -	     to make sure I'm not running into surprises.  */
> -	  gdb_assert (*next == '\0');
> +	      /* FIXME: carlton/2002-11-14: For members of classes,
> +		 with this include the class name as well?  I don't
> +		 think that's a problem yet, but it will be.  */
> +	      
> +	      for (current = name, next = cp_find_first_component (current);
> +		   *next == ':';
> +		   /* The '+ 2' is to skip the '::'.  */
> +		   current = next,
> +		     next = cp_find_first_component (current + 2))
> +		;

I see that you're just moving this bit but I should have commented on
it last time: please do not use for loops this way.  Something like:
  current = name;
  next = cp_find_first_component (current);
  while (*next == ':')
    {
      current = next;
      /* The '+ 2' is to skip the '::'.  */
      *next = cp_find_first_component (current + 2);
    }

In general, if all the work is inside the parentheses a for loop is
inappropriate.

I did not extensively examine your lookup_symbol_* changes in this
patch; I eyeballed them and they looked good but that's it.  That's
enough for me while you're still evolving this.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer