Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: David Carlton <carlton@math.stanford.edu>
To: Elena Zannoni <ezannoni@redhat.com>
Cc: gdb-patches@sources.redhat.com,
	Daniel Jacobowitz <drow@mvista.com>, Jim Blandy <jimb@redhat.com>
Subject: Re: [rfa] annotate blocks with C++ namespace information
Date: Mon, 14 Apr 2003 21:33:00 -0000	[thread overview]
Message-ID: <ro1r884dasw.fsf@jackfruit.Stanford.EDU> (raw)
In-Reply-To: <16027.2953.467195.516437@localhost.redhat.com>

On Mon, 14 Apr 2003 15:27:05 -0400, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> Just for reference, here's a slightly updated version of my namespace
>> patch, following Daniel's suggestions.  The only real change is that
>> it adds a new command "maint cplus first_component" and a new file
>> gdb.c++/maint.exp to test it.

> Ok, I got around to this finally.  It is basically ok, except for the
> line between what is c++ and what is symbol table stuff. I think that
> more stuff can be pushed into cp-support.c. See below...

I have mixed feelings about your comments. My first reaction was the
'using_list' stuff more logically belongs in buildsym.c: it's about
building a symtab, after all!  So if the only reason to move it to
cp-support is to shift the maintenance responsibilities (which is
sensible enough, no need for you to look at changes that only affect
C++ support), then I'd rather fix the maintenance process: make Daniel
a symtab maintainer (he's certainly done enough work on symtabs), or
at least allow him to approve C++-specific symtab changes.

Having said that, I'm tentatively coming around to your point of view.
After all, it's easy enough for me to say that everything related to
building symtabs should be in buildsym.c, but if lots of different
languages develop their own special needs for the symbol table, then
buildsym.c will quickly degenerate into a mess of language-specific
special cases.  So maybe you're right.  And, after all, cp-support.c
is a lot smaller than buildsym.c, so it will be a while before it gets
too bloated.

Daniel, what do you think?

A few other issues:

> You could tighten the interface a bit, by passing in the name of the
> symbol only, instead of the symbol.

Yup, good idea.

> The struct using_direct could also be made opaque, I think.

It can't really be profitably made opaque: it probably looks that way
from this patch, but that's only because this patch doesn't actually
use struct using_direct, it just constructs them.  My next patch will
use them in symtab.c.

> processing_has_namespace_info: there is one per symtab, yes?
> I mean, it is set to 0 only here, just want to make sure.

Right.

>> +	  block_set_using (BLOCKVECTOR_BLOCK (blockvector, STATIC_BLOCK),
>> +			   cp_copy_usings (using_list,

> Should this function be called something like cp_dup_usings? But you
> are freeing the original copy, right? Why do you need to copy it
> over?

The relevant memory-management issues are a mess.  Basically, the list
of using directives has to eventually be stored on the objfile's
obstack.  Unfortunately, that obstack isn't stashed anywhere that
buildsym.c (or cp-support.c) can get access to it while constructing
using_list.  I thought about adding it as an extra argument to
start_symtab, but it would have involved making changes to all of
start_symtab's callers that I didn't feel entirely comfortable with.
So I reluctantly settled on allocating it with xmalloc, and then
copying it onto the obstack at a location where we actually know what
the obstack is.

Thanks a bunch for looking over this; I really appreciate it.  I'll
check it in once we agree about the proper location of using_list and
friends.  Actually, the more I think about it, the better cp-support
looks as a location: many of the extern functions that I want to add
to cp-support are only used by buildsym.c, so that's a good argument
for putting those bits of buildsym.c in cp-support.c.

David Carlton
carlton@math.stanford.edu


  reply	other threads:[~2003-04-14 21:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-02-22  1:47 David Carlton
2003-03-11 17:11 ` Daniel Jacobowitz
2003-03-11 21:14   ` David Carlton
2003-03-11 21:23     ` Daniel Jacobowitz
2003-03-11 22:43       ` David Carlton
2003-03-11 22:59         ` David Carlton
2003-03-11 23:01           ` Daniel Jacobowitz
2003-03-17 22:33   ` David Carlton
2003-04-14 19:22     ` Elena Zannoni
2003-04-14 21:33       ` David Carlton [this message]
2003-04-15  2:08         ` Daniel Jacobowitz
2003-04-15  2:26           ` Elena Zannoni
2003-04-15  3:07             ` David Carlton
2003-04-15 23:12             ` David Carlton
2003-04-16  1:22               ` Elena Zannoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ro1r884dasw.fsf@jackfruit.Stanford.EDU \
    --to=carlton@math.stanford.edu \
    --cc=drow@mvista.com \
    --cc=ezannoni@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=jimb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox