From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6834 invoked by alias); 21 Nov 2002 17:40:23 -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 6797 invoked from network); 21 Nov 2002 17:40:22 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by sources.redhat.com with SMTP; 21 Nov 2002 17:40:22 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18ExBd-0007fS-00; Thu, 21 Nov 2002 13:40:29 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18EvJC-0003OR-00; Thu, 21 Nov 2002 12:40:10 -0500 Date: Thu, 21 Nov 2002 09:40:00 -0000 From: Daniel Jacobowitz To: David Carlton Cc: gdb-patches@sources.redhat.com Subject: Re: [drow-cplus-branch] handle namespace scope Message-ID: <20021121174010.GA12554@nevyn.them.org> Mail-Followup-To: David Carlton , gdb-patches@sources.redhat.com References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2002-11/txt/msg00519.txt.bz2 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 > 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