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

[David Carlton <carlton@math.stanford.edu>]

On Thu, 21 Nov 2002 12:40:10 -0500, Daniel Jacobowitz <drow@mvista.com> said:
> On Tue, Nov 19, 2002 at 03:07:45PM -0800, David Carlton wrote:

>> 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.

Whoops, thanks, that was a cut/paste problem.  Sorry about that.

>> +	      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.

Will do; that's probably not the only place I should fix this.

> 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.

Great, thanks.  Yeah, that really is in flux: I'm starting to
understand what the underlying conceptual issues are, but I still
haven't gotten those nailed down properly.  And even once I do have
them nailed down properly, there are implementations details: e.g. the
lookup_symbol stuff does too much temporary allocation of copies via
xmalloc, which I suspect I'll want to eventually replace either by
temporary allocation on an obstack (or via alloca, maybe), or else
rewrite the code so that it doesn't need to make copies at all.

I'll commit it with the two fixes you mentioned, and I'll also look
for other loops that I should rewrite for clarity before committing.

Thanks,
David Carlton
carlton@math.stanford.edu