Re: [rfa] annotate blocks with C++ namespace information

[Daniel Jacobowitz <drow at mvista dot com>]

On Tue, Mar 11, 2003 at 01:14:16PM -0800, David Carlton wrote:
> On Tue, 11 Mar 2003 12:11:33 -0500, Daniel Jacobowitz <drow at mvista dot com> 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