From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13661 invoked by alias); 11 Mar 2003 21:23:17 -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 13653 invoked from network); 11 Mar 2003 21:23:16 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by 172.16.49.205 with SMTP; 11 Mar 2003 21:23:16 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18st6n-0003Qu-00 for ; Tue, 11 Mar 2003 17:24:33 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18srDO-0004tJ-00 for ; Tue, 11 Mar 2003 16:23:14 -0500 Date: Tue, 11 Mar 2003 21:23:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: [rfa] annotate blocks with C++ namespace information Message-ID: <20030311212313.GA18680@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <20030311171133.GA3362@nevyn.them.org> 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: 2003-03/txt/msg00262.txt.bz2 On Tue, Mar 11, 2003 at 01:14:16PM -0800, David Carlton wrote: > On Tue, 11 Mar 2003 12:11:33 -0500, Daniel Jacobowitz said: > > On Fri, Feb 21, 2003 at 05:47:35PM -0800, David Carlton wrote: > > >> --- buildsym.c 20 Feb 2003 17:17:23 -0000 1.29 > >> +++ buildsym.c 22 Feb 2003 00:46:55 -0000 > >> @@ -44,6 +44,7 @@ > >> #include "macrotab.h" > >> #include "demangle.h" /* Needed by SYMBOL_INIT_DEMANGLED_NAME. */ > >> #include "block.h" > >> +#include "cp-support.h" > >> /* Ask buildsym.h to define the vars it normally declares `extern'. */ > > > Blank line :) > > You weren't joking about the 'nit-picking pedantic'. :-) > > >> + /* We've found a component of the name that's an anonymous > >> + namespace. So add symbols in it to the namespace given > >> + by the previous component if there is one, or to the > >> + global namespace if there isn't. */ > >> + add_using_directive (name, > >> + previous_component == 0 > >> + ? 0 : previous_component - 2, > >> + next_component); > > > Use NULL for zero pointers, please. > > They're ints, not pointers. You might be remembering an earlier > version of this where I used pointers in this loop; I decided that it > was cleaner for cp_find_first_component to return an int instead of a > const char *. Oh, right. > I don't really care one way or another; I just figure that, since I > typed most of the characters in that file (at least in the version on > the branch), I might as well get some credit. :-) Honestly, though, > I'm happy just to leave it as "Contributed by MontaVista Software." That wasn't one of the options :) You should certainly ave your name in there. > > - This may be obvious, but it implies that you have an employer > > disclaimer from Stanford in addition to a personal assignment, if > > Stanford is contributing code. I'd just like to double-check that > > that's accurate. I don't have access to the assignments data. > > My situation is, I am learning (because I get asked this question not > infrequently), a bit unusual: no, I don't have an employer disclaimer > from Stanford, but that's okay since the FSF and I agree that I don't > need one. Stanford's employment contract for faculty members makes it > quite clear that Stanford doesn't own the copyright for software that > I write (with some exceptions that clearly don't apply to me), even if > I write it using Stanford's computers. (Similarly, Stanford doesn't > own the copyright to articles or books that I write.) I'm just > mentioning Stanford out of politeness. In this case Stanford is not contributing the code; contribution is a copyright related action, and they never had the copyright. Please say "Namespace support contributed by David Carlton" instead of "Contributed by Stanford University". > >> +/* Here are some random pieces of trivia to keep in mind while trying > >> + to take apart demangled names: > > > You're adding cp_find_first_component, which seems to me to duplicate > > logic from find_last_component among other places. I think we have > > either three or four subtly different copies of this logic now. Is it > > really necessary? It's not precisely the same (you're going in the > > other direction) but it would be really nice to condense this. > > Yes; it should be easy enough to rewrite, say, find_last_component > using cp_find_first_component. The only reason why I didn't do that > (other than laziness) was that then I'd want to go figure out a > situation where find_last_component actually gets called, to make sure > I didn't make a boneheaded mistake while doing so, and I didn't feel > like doing that. But I'll definitely add a FIXME comment about that? OK. find_last_component gets called for dealing with stub methods, if I recall correctly why I wrote it. > >> +unsigned int > >> +cp_find_first_component (const char *name) > >> +{ > >> + /* Names like 'operator<<' screw up the recursion, so let's > >> + special-case them. I _hope_ they can only occur at the start of > >> + a component. */ > >> + > >> + unsigned int index = 0; > >> + > >> + if (strncmp (name, "operator", LENGTH_OF_OPERATOR) == 0) > > > I think that handling operators correctly would be simpler than I > > thought previously. All we should have to do is, when we hit a '<', > > check if the preceding word is "operator". It's still not entirely > > trivial (there might be a space after operator, or not; there must be > > something indicating word-break or beginning-of-string before operator) > > but it's pretty simple. > > There's also operator-> to worry about. And, now that you mention it, > I'm not handling the possibility of whitespace between 'operator' and > the actual operator. The function isn't entirely bullet-proof when > given user input (you could construct malformed user input containing > single colons that would cause it problems, I think); I should > probably add a comment saying it's intended for internal use only, at > least for now. (Though the similar function in linespec.c has the > same flaw.) Do any demanglers put in spaces after 'operator'? I hope > not... I thought one of them did, but I might have been mistaken. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer