Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@is.elta.co.il>
To: Daniel Berlin <dan@cgsoftware.com>
Cc: Jim Blandy <jimb@zwingli.cygnus.com>,
	David Taylor <taylor@candd.org>,
	gdb-patches@sources.redhat.com, Anthony Green <green@redhat.com>
Subject: Re: RFA: abstract C++ ABI dependencies
Date: Tue, 24 Apr 2001 23:52:00 -0000	[thread overview]
Message-ID: <Pine.SUN.3.91.1010425094152.22758R-100000@is> (raw)
In-Reply-To: <m2wv895vxj.fsf@dynamic-addr-83-177.resnet.rochester.edu>

On 25 Apr 2001, Daniel Berlin wrote:

> Eli Zaretskii <eliz@is.elta.co.il> writes:
> 
> > On 24 Apr 2001, Jim Blandy wrote:
> > 
> > > > 	    * c-typeprint.c, c-valprint.c, dbxread.c, eval.c, gdbtypes.c,
> > > > 	    jv-typeprint.c, linespec.c, symtab.c, typeprint.c, valops.c:
> > > > 	    #include "cp-abi.h".
> > > > 
> > > > For c-valprint.c, eval.c, typeprint.c, and valops.c, it is unclear to
> > > > me why you are including cp-abi.h.
> > > 
> > > Because they use functions which used to be declared elsewhere, but
> > > are now declared in cp-abi.h.  I've clarified the ChangeLog entry.
> > 
> > Isn't it better to put such explanations in the files which include 
> > cp-abi.h, rather than in a ChangeLog?
> 
> Errr, i don't see any other comments on header includes.

I see _lots_ of them.  Two examples (you can see them all with
"grep '#include..*/\*' *.c"):

  utils.c:#include "inferior.h" /* for signed_pointer_to_address */
  linux-thread.c:#include "gdb_wait.h" /* for WUNTRACED and __WCLONE flags */

But even if these comments were not present, that doesn't mean they 
shouldn't be there.

> Why would you list the reasons for including a given header? 

Because people who read the code afterwards might ask themselves why
was that header included, if the name of the header doesn't make that 
obvious.

This seems especially important since lately some of the maintainers are 
actively terying to remove unneeded headers in order to  minimize 
dependencies and slash the build time.

> If you want to know what's in the header, and you can't tell from the
> top of the header itself, or the filename of the header, well, then,
> there's probably something else wrong.

Maybe something _is_ wrong.  If so, the code should be reworked 
accordingly.

However, if, after all the deliberations, it is decided to leave the 
header inclusion, and that header's name does not explain why it is 
needed, like in the case in point (see David's question), then I think
the explanation should be in the code, not in the ChangeLog.

In other words, if there were no reason to have an explanation, why would 
Jim feel he should provide one on the ChangeLog?


  reply	other threads:[~2001-04-24 23:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-04-24 12:27 David Taylor
2001-04-24 15:33 ` Jim Blandy
2001-04-24 16:23   ` Daniel Berlin
2001-04-24 18:27     ` Jim Blandy
2001-04-24 23:21   ` Eli Zaretskii
2001-04-24 23:35     ` Daniel Berlin
2001-04-24 23:52       ` Eli Zaretskii [this message]
2001-04-25  0:07         ` Daniel Berlin
2001-04-25  0:24           ` Eli Zaretskii
2001-04-24 17:42 ` Jim Blandy
2001-04-25  5:04   ` Anthony Green
  -- strict thread matches above, loose matches on Subject: below --
2001-04-25  6:40 David Taylor
2001-04-26 17:23 ` Jim Blandy
2001-04-24 10:30 Jim Blandy

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=Pine.SUN.3.91.1010425094152.22758R-100000@is \
    --to=eliz@is.elta.co.il \
    --cc=dan@cgsoftware.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=green@redhat.com \
    --cc=jimb@zwingli.cygnus.com \
    --cc=taylor@candd.org \
    /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