From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 405 invoked by alias); 21 Nov 2002 20:29:46 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 390 invoked from network); 21 Nov 2002 20:29:45 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by sources.redhat.com with SMTP; 21 Nov 2002 20:29:45 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id gALKThb09713; Thu, 21 Nov 2002 12:29:43 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com Subject: Re: [drow-cplus-branch] handle namespace scope References: <20021121174010.GA12554@nevyn.them.org> From: David Carlton Date: Thu, 21 Nov 2002 12:29:00 -0000 In-Reply-To: <20021121174010.GA12554@nevyn.them.org> Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-11/txt/msg00524.txt.bz2 On Thu, 21 Nov 2002 12:40:10 -0500, Daniel Jacobowitz 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