GNU bug report logs - #21707
include-file cleanup for src directory

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21707 in the body.
You can then email your comments to 21707 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Emacs bug reports and feature requests <bug-gnu-emacs <at> gnu.org>
Subject: include-file cleanup for src directory
Date: Mon, 19 Oct 2015 00:05:52 -0700

[Message part 1 (text/plain, inline)]

Tags: patch

Attached is a patch to remove unnecessary #include directives from the src 
directory, and add a few #include directives that should be there.  I'm not 
installing this now, to give Eli a heads-up in case this affects the MS-Windows 
port.

[0001-Include-file-cleanup-for-src-directory.txt (text/plain, attachment)]

Message #8 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Mon, 19 Oct 2015 10:23:02 +0300

> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 19 Oct 2015 00:05:52 -0700
> 
> Attached is a patch to remove unnecessary #include directives from the src 
> directory, and add a few #include directives that should be there.  I'm not 
> installing this now, to give Eli a heads-up in case this affects the MS-Windows 
> port.

I had compilation warnings and errors due to redisplaying_p and
cancel_hourglass not being declared in eval.c and lread.c.  These two
files need to include dispextern.h to avoid that problem.  (This isn't
Windows specific, so I wonder how it compiled for you.)

Otherwise, LGTM, thanks.

Would someone with MinGW64 installed please try this patch and see
that it works there as well?

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Mon, 19 Oct 2015 13:54:15 +0100

On Mon 19 Oct 2015, Eli Zaretskii wrote:

>> From: Paul Eggert <eggert <at> cs.ucla.edu>
>> Date: Mon, 19 Oct 2015 00:05:52 -0700
>> 
>> Attached is a patch to remove unnecessary #include directives from the src 
>> directory, and add a few #include directives that should be there.  I'm not 
>> installing this now, to give Eli a heads-up in case this affects the MS-Windows 
>> port.
>
> I had compilation warnings and errors due to redisplaying_p and
> cancel_hourglass not being declared in eval.c and lread.c.  These two
> files need to include dispextern.h to avoid that problem.  (This isn't
> Windows specific, so I wonder how it compiled for you.)
>
> Otherwise, LGTM, thanks.
>
> Would someone with MinGW64 installed please try this patch and see
> that it works there as well?

I've done a full bootstrap without problems on MinGW64 from commit
f1575763c0d3 with the patch and your additional fixes (which are needed
on MinGW64 too).

    AndyM

Message #14 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Mon, 19 Oct 2015 16:23:46 +0300

> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Mon, 19 Oct 2015 13:54:15 +0100
> 
> > Would someone with MinGW64 installed please try this patch and see
> > that it works there as well?
> 
> I've done a full bootstrap without problems on MinGW64 from commit
> f1575763c0d3 with the patch and your additional fixes (which are needed
> on MinGW64 too).

Thanks.

Message #17 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Mon, 19 Oct 2015 22:54:39 -0700

[Message part 1 (text/plain, inline)]

Eli Zaretskii wrote:
> I had compilation warnings and errors due to redisplaying_p and
> cancel_hourglass not being declared in eval.c and lread.c.  These two
> files need to include dispextern.h to avoid that problem.  (This isn't
> Windows specific, so I wonder how it compiled for you.)

It's a problem with platform-specific inclusions: xterm.h includes dispextern.h, 
but eval.c and lread.c don't include xterm.h on MS-Windows. I attempted to 
detect this sort of thing by hand without actually building on MS-Windows, but 
it's an easy thing to get wrong. Revised patch attached. Quite possibly there 
will be similar issues on other platforms, but they should be easy to fix once 
encountered.

[0001-Include-file-cleanup-for-src-directory.txt (text/plain, attachment)]

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 09:23:04 +0100

On Mon 19 Oct 2015, Paul Eggert wrote:

> Eli Zaretskii wrote:
>> I had compilation warnings and errors due to redisplaying_p and
>> cancel_hourglass not being declared in eval.c and lread.c.  These two
>> files need to include dispextern.h to avoid that problem.  (This isn't
>> Windows specific, so I wonder how it compiled for you.)
>
> It's a problem with platform-specific inclusions: xterm.h includes
> dispextern.h, but eval.c and lread.c don't include xterm.h on MS-Windows. I
> attempted to detect this sort of thing by hand without actually building on
> MS-Windows, but it's an easy thing to get wrong. Revised patch attached. Quite
> possibly there will be similar issues on other platforms, but they should be
> easy to fix once encountered.

This builds without problems on top of commit 10df0310cb8f for:
 - mingw32 32bit
 - mingw32 32bit with --wide-int
 - mingw64 64bit
 - cygwin 64bit

Looks good to me.

    AndyM

Message #23 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 18:06:00 +0300

> Cc: 21707 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 19 Oct 2015 22:54:39 -0700
> 
> Eli Zaretskii wrote:
> > I had compilation warnings and errors due to redisplaying_p and
> > cancel_hourglass not being declared in eval.c and lread.c.  These two
> > files need to include dispextern.h to avoid that problem.  (This isn't
> > Windows specific, so I wonder how it compiled for you.)
> 
> It's a problem with platform-specific inclusions: xterm.h includes dispextern.h, 
> but eval.c and lread.c don't include xterm.h on MS-Windows. I attempted to 
> detect this sort of thing by hand without actually building on MS-Windows, but 
> it's an easy thing to get wrong.

I see.  Would it help if we avoid including any of our headers in any
other of our headers, so that the headers included by a particular C
file are visible by just looking at that single C file?  At least then
any system dependencies will be explicitly stated in each C file.

> Revised patch attached.

Thanks, this works for me.

Message #26 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andy Moreton <andrewjmoreton <at> gmail.com>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 18:06:26 +0300

> From: Andy Moreton <andrewjmoreton <at> gmail.com>
> Date: Tue, 20 Oct 2015 09:23:04 +0100
> 
> This builds without problems on top of commit 10df0310cb8f for:
>  - mingw32 32bit
>  - mingw32 32bit with --wide-int
>  - mingw64 64bit
>  - cygwin 64bit
> 
> Looks good to me.

Same here, thanks.

Message #29 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 11:50:34 -0700

Eli Zaretskii wrote:
> Would it help if we avoid including any of our headers in any
> other of our headers, so that the headers included by a particular C
> file are visible by just looking at that single C file?

That would run afoul of a more important design goal, which is that we should be 
able to include headers in any order. And anyway, compiling a typical C file 
ordinarily brings in oodles of system headers that the C file doesn't explicitly 
ask for, so regardless of our style the only practical way to see what headers a 
C file includes is to compile it and see what gcc -E outputs.

Ideally a C source file should include all headers that define symbols the 
source file directly uses, and no headers other than that. This should be true 
for both .h and .c files. We're not there by a long shot, but that should be the 
goal.

Message #32 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 22:15:54 +0300

> Cc: 21707 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 20 Oct 2015 11:50:34 -0700
> 
> Eli Zaretskii wrote:
> > Would it help if we avoid including any of our headers in any
> > other of our headers, so that the headers included by a particular C
> > file are visible by just looking at that single C file?
> 
> That would run afoul of a more important design goal, which is that we should be 
> able to include headers in any order.

Is that really more important?  Even standard C headers sometimes
require order, for example, sys/types.h should be included before
sys/stat.h.

> And anyway, compiling a typical C file ordinarily brings in oodles
> of system headers that the C file doesn't explicitly ask for

That is true, but C headers change much slower than our headers.  So
keeping track of the changes in our headers requires more attention
and work.

> so regardless of our style the only practical way to see what headers a 
> C file includes is to compile it and see what gcc -E outputs.

The problem with "gcc -E" is that it's system dependent.  I thought we
could perhaps come up with a method that would allow anyone easily see
which headers are needed on every supported platform, and thus lower
the probability of unintended breakage.

Of course, it isn't a catastrophe to continue the way we have been
doing this till now.

> Ideally a C source file should include all headers that define symbols the 
> source file directly uses, and no headers other than that. This should be true 
> for both .h and .c files. We're not there by a long shot, but that should be the 
> goal.

I think the real problem is to keep this state once we get there.
One-time efforts to rectify things are relatively easy; making that
stick is harder, especially since (AFAIU) there's no automated way for
producing a report about unnecessary headers.

Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andy Moreton <andrewjmoreton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 20:28:33 +0100

On Tue 20 Oct 2015, Paul Eggert wrote:

> Eli Zaretskii wrote:
>> Would it help if we avoid including any of our headers in any
>> other of our headers, so that the headers included by a particular C
>> file are visible by just looking at that single C file?
>
> That would run afoul of a more important design goal, which is that we should
> be able to include headers in any order. And anyway, compiling a typical C
> file ordinarily brings in oodles of system headers that the C file doesn't
> explicitly ask for, so regardless of our style the only practical way to see
> what headers a C file includes is to compile it and see what gcc -E outputs.
>
> Ideally a C source file should include all headers that define symbols the
> source file directly uses, and no headers other than that. This should be true
> for both .h and .c files. We're not there by a long shot, but that should be
> the goal.

...and also include headers for anything exported by the C file to
ensure that declarations and definitions are not incompatible.

    AndyM

Message #38 received at 21707 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 21707 <at> debbugs.gnu.org
Subject: Re: bug#21707: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 13:52:42 -0700

On 10/20/2015 12:15 PM, Eli Zaretskii wrote:
> Even standard C headers sometimes require order, for example, 
> sys/types.h should be included before sys/stat.h.

Oh my goodness your memory goes a long way back!  But the C Standard 
never required a particular header order (it never standardized 
sys/types.h or sys/stat.h). POSIX did originally require that one must 
include both <sys/types.h> and <sys/stat.h> before using functions like 
'stat', but that requirement was removed a while ago; e.g., nowadays 
only <sys/stat.h> must be included before using 'stat'.  And it's not 
hard to see why the old POSIX requirement was removed: a .h file should 
describe an API standalone, to avoid unnecessarily constraining the 
API's users.

> I thought we could perhaps come up with a method that would allow 
> anyone easily see which headers are needed on every supported platform

I started something along those lines, but it's harder than it looks. 
The current patch came from my woefully-inadequate approximation to the 
tools I wanted.

> I think the real problem is to keep this state once we get there.

Yes. Part of the job would be to write better checking tools. And part, 
I expect, would be to simplify Emacs's complicated uses of headers. For 
example, although we sometimes have headers A and B that use each 
others' symbols, it wouldn't be cool to have both A #include B and B 
#include A. Some of this simplification will likely be needed anyway, as 
we change more macros to inline functions.

Message #43 received at 21707-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 21707-done <at> debbugs.gnu.org
Subject: Re: include-file cleanup for src directory
Date: Tue, 20 Oct 2015 18:38:33 -0700

I've installed the patch and am marking this as done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.