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?
next prev parent 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