Re: [PATCH 2/2] maint: use pragma consistently across replacement headers

[Bruno Haible]

Hi Eric,

> >> -#ifndef _GL_CTYPE_H
> >> -
> >>  #if __GNUC__ >= 3
> >>  @PRAGMA_SYSTEM_HEADER@
> >>  #endif
> >>
> >> +#ifndef _GL_CTYPE_H
> >> +
> >>  /* Include the original <ctype.h>.  */
> >>  /* The include_next requires a split double-inclusion guard.  */
> >>  address@hidden@ @NEXT_CTYPE_H@
> > 
> > Did you know that GCC has a special optimization for include files? ...

> are you saying that the optimization is useless for those files, because they
> are split includes, and thus already fail to meet the pattern that gcc
> is looking for?

No, on the contrary: The previous shape of most of our *.in.h files matches
what GCC is looking for: It starts with a "#ifndef _GL_STRING_H" and ends
with the corresponding "#endif". The fact that _GL_STRING_H is tested again
inside is irrelevant for this optimization.

> Maybe it's worth a bug report to the gcc folks to get them to also
> recognize idempotent preprocessor directives (such as the hoisted
> #pragma) that do not affect the optimization of reparsing the file?

Hmm, it's not only the #pragma, it's also the #if __GNUC__ >= 3 before it.
I can hardly imagine that the GCC people would implement such a feature
request - for them, it will be a rare use-case with an easy workaround.

The only .in.h files on which the GCC optimization does not apply anyway
are those with a special invocation convention, namely
  fcntl.in.h
  inttypes.in.h
  signal.in.h
  stddef.in.h
  stdio.in.h
  stdlib.in.h
  sys_select.in.h
  sys_stat.in.h
  time.in.h
  unistd.in.h
  wchar.in.h

> Well, we could revert this patch

Yes, this is what I would propose. We don't know how many people would notice
the GCC slowdown, but even if one single gnulib user complained, I wouldn't
like to have slowed down his compilation for no convincing reason.

> instead patch fcntl.in.h, stdlib.in.h, and so forth to follow this 
> optimization.

We can't do this: it's a special invocation convention. These include files are
not no-ops if they are included a second time with __need_system_fcntl_h or
_GL_JUST_INCLUDE_SYSTEM_INTTYPES_H set. If we were to change this logic, we are
likely to reintroduce bugs that we fixed long ago.

Here's the proposed patch:


2010-03-28  Bruno Haible  <address@hidden>

        Emit #pragma system_header after the inclusion guard, not before.
        * lib/arpa_inet.in.h: Emit #pragma system_header after the inclusion
        guard that spans the entire file, not before. This enables an
        optimization in GCC's preprocessor.
        * lib/ctype.in.h: Likewise.
        * lib/dirent.in.h: Likewise.
        * lib/errno.in.h: Likewise.
        * lib/float.in.h: Likewise.
        * lib/getopt.in.h: Likewise.
        * lib/iconv.in.h: Likewise.
        * lib/langinfo.in.h: Likewise.
        * lib/locale.in.h: Likewise.
        * lib/math.in.h: Likewise.
        * lib/netdb.in.h: Likewise.
        * lib/netinet_in.in.h: Likewise.
        * lib/pty.in.h: Likewise.
        * lib/sched.in.h: Likewise.
        * lib/se-selinux.in.h: Likewise.
        * lib/search.in.h: Likewise.
        * lib/spawn.in.h: Likewise.
        * lib/stdarg.in.h: Likewise.
        * lib/stdint.in.h: Likewise.
        * lib/string.in.h: Likewise.
        * lib/strings.in.h: Likewise.
        * lib/sys_file.in.h: Likewise.
        * lib/sys_ioctl.in.h: Likewise.
        * lib/sys_time.in.h: Likewise.
        * lib/sys_times.in.h: Likewise.
        * lib/sys_utsname.in.h: Likewise.
        * lib/sys_wait.in.h: Likewise.
        * lib/sysexits.in.h: Likewise.
        * lib/wctype.in.h: Likewise.

--- lib/arpa_inet.in.h.orig     Sun Mar 28 12:28:35 2010
+++ lib/arpa_inet.in.h  Sun Mar 28 12:07:53 2010
@@ -16,12 +16,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
-# if __GNUC__ >= 3
address@hidden@
-# endif
-
 #ifndef _GL_ARPA_INET_H
 
+#if __GNUC__ >= 3
address@hidden@
+#endif
+
 /* Gnulib's sys/socket.h is responsible for pulling in winsock2.h etc
    under MinGW.
    But avoid namespace pollution on glibc systems.  */
--- lib/ctype.in.h.orig Sun Mar 28 12:28:35 2010
+++ lib/ctype.in.h      Sun Mar 28 12:08:07 2010
@@ -23,12 +23,12 @@
  * <http://www.opengroup.org/onlinepubs/9699919799/basedefs/ctype.h.html>
  */
 
+#ifndef _GL_CTYPE_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_CTYPE_H
-
 /* Include the original <ctype.h>.  */
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_CTYPE_H@
--- lib/dirent.in.h.orig        Sun Mar 28 12:28:35 2010
+++ lib/dirent.in.h     Sun Mar 28 12:08:17 2010
@@ -14,12 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_DIRENT_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_DIRENT_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_DIRENT_H@
 
--- lib/errno.in.h.orig Sun Mar 28 12:28:35 2010
+++ lib/errno.in.h      Sun Mar 28 12:08:30 2010
@@ -16,12 +16,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_ERRNO_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_ERRNO_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_ERRNO_H@
 
--- lib/float.in.h.orig Sun Mar 28 12:28:35 2010
+++ lib/float.in.h      Sun Mar 28 12:09:49 2010
@@ -15,12 +15,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_FLOAT_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_FLOAT_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_FLOAT_H@
 
--- lib/getopt.in.h.orig        Sun Mar 28 12:28:35 2010
+++ lib/getopt.in.h     Sun Mar 28 12:09:58 2010
@@ -16,12 +16,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_GETOPT_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_GETOPT_H
-
 /* The include_next requires a split double-inclusion guard.  We must
    also inform the replacement unistd.h to not recursively use
    <getopt.h>; our definitions will be present soon enough.  */
--- lib/iconv.in.h.orig Sun Mar 28 12:28:35 2010
+++ lib/iconv.in.h      Sun Mar 28 12:10:09 2010
@@ -16,12 +16,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_ICONV_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_ICONV_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_ICONV_H@
 
--- lib/langinfo.in.h.orig      Sun Mar 28 12:28:35 2010
+++ lib/langinfo.in.h   Sun Mar 28 12:10:31 2010
@@ -20,12 +20,12 @@
  * <http://www.opengroup.org/onlinepubs/9699919799/basedefs/langinfo.h.html>
  */
 
+#ifndef _GL_LANGINFO_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_LANGINFO_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_LANGINFO_H@
 # @INCLUDE_NEXT@ @NEXT_LANGINFO_H@
--- lib/locale.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/locale.in.h     Sun Mar 28 12:10:43 2010
@@ -14,12 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_LOCALE_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_LOCALE_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_LOCALE_H@
 
--- lib/math.in.h.orig  Sun Mar 28 12:28:36 2010
+++ lib/math.in.h       Sun Mar 28 12:10:52 2010
@@ -15,12 +15,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_MATH_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_MATH_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_AS_FIRST_DIRECTIVE_MATH_H@
 
--- lib/netdb.in.h.orig Sun Mar 28 12:28:36 2010
+++ lib/netdb.in.h      Sun Mar 28 12:11:07 2010
@@ -20,12 +20,12 @@
    It is intended to provide definitions and prototypes needed by an
    application.  */
 
-# if __GNUC__ >= 3
address@hidden@
-# endif
-
 #ifndef _GL_NETDB_H
 
+#if __GNUC__ >= 3
address@hidden@
+#endif
+
 #if @HAVE_NETDB_H@
 
 /* The include_next requires a split double-inclusion guard.  */
--- lib/netinet_in.in.h.orig    Sun Mar 28 12:28:36 2010
+++ lib/netinet_in.in.h Sun Mar 28 12:11:50 2010
@@ -15,12 +15,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
-# if __GNUC__ >= 3
address@hidden@
-# endif
-
 #ifndef _GL_NETINET_IN_H
 
+#if __GNUC__ >= 3
address@hidden@
+#endif
+
 #if @HAVE_NETINET_IN_H@
 
 /* On many platforms, <netinet/in.h> assumes prior inclusion of
--- lib/pty.in.h.orig   Sun Mar 28 12:28:36 2010
+++ lib/pty.in.h        Sun Mar 28 12:12:02 2010
@@ -15,12 +15,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_PTY_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_PTY_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_PTY_H@
 # @INCLUDE_NEXT@ @NEXT_PTY_H@
--- lib/sched.in.h.orig Sun Mar 28 12:28:36 2010
+++ lib/sched.in.h      Sun Mar 28 12:12:10 2010
@@ -14,12 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_SCHED_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SCHED_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_SCHED_H@
 # @INCLUDE_NEXT@ @NEXT_SCHED_H@
--- lib/se-selinux.in.h.orig    Sun Mar 28 12:28:36 2010
+++ lib/se-selinux.in.h Sun Mar 28 12:12:46 2010
@@ -14,13 +14,13 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#if __GNUC__ >= 3
address@hidden@
-#endif
-
 #ifndef _GL_SELINUX_SELINUX_H
 # define _GL_SELINUX_SELINUX_H
 
+# if __GNUC__ >= 3
address@hidden@
+# endif
+
 # if HAVE_SELINUX_SELINUX_H
 
 address@hidden@ @NEXT_SELINUX_SELINUX_H@
--- lib/search.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/search.in.h     Sun Mar 28 12:13:10 2010
@@ -15,12 +15,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_SEARCH_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SEARCH_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_SEARCH_H@
 # @INCLUDE_NEXT@ @NEXT_SEARCH_H@
--- lib/spawn.in.h.orig Sun Mar 28 12:28:36 2010
+++ lib/spawn.in.h      Sun Mar 28 12:14:12 2010
@@ -15,12 +15,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifndef _GL_SPAWN_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SPAWN_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_SPAWN_H@
 # @INCLUDE_NEXT@ @NEXT_SPAWN_H@
--- lib/stdarg.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/stdarg.in.h     Sun Mar 28 12:14:25 2010
@@ -15,12 +15,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_STDARG_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_STDARG_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_STDARG_H@
 
--- lib/stdint.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/stdint.in.h     Sun Mar 28 12:15:13 2010
@@ -21,12 +21,12 @@
  * <http://www.opengroup.org/susv3xbd/stdint.h.html>
  */
 
-# if __GNUC__ >= 3
address@hidden@
-# endif
-
 #ifndef _GL_STDINT_H
 
+#if __GNUC__ >= 3
address@hidden@
+#endif
+
 /* When including a system file that in turn includes <inttypes.h>,
    use the system <inttypes.h>, not our substitute.  This avoids
    problems with (for example) VMS, whose <sys/bitypes.h> includes
--- lib/string.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/string.in.h     Sun Mar 28 12:16:33 2010
@@ -16,12 +16,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_STRING_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_STRING_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_STRING_H@
 
--- lib/strings.in.h.orig       Sun Mar 28 12:28:36 2010
+++ lib/strings.in.h    Sun Mar 28 12:16:46 2010
@@ -16,12 +16,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_STRINGS_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_STRINGS_H
-
 /* The include_next requires a split double-inclusion guard.  */
 address@hidden@ @NEXT_STRINGS_H@
 
--- lib/sys_file.in.h.orig      Sun Mar 28 12:28:36 2010
+++ lib/sys_file.in.h   Sun Mar 28 12:17:37 2010
@@ -18,16 +18,16 @@
 
 /* Written by Richard W.M. Jones.  */
 
-# if __GNUC__ >= 3
address@hidden@
-# endif
-
 #ifndef _GL_SYS_FILE_H
 
+#if __GNUC__ >= 3
address@hidden@
+#endif
+
 /* The include_next requires a split double-inclusion guard.  */
-# if @HAVE_SYS_FILE_H@
-#  @INCLUDE_NEXT@ @NEXT_SYS_FILE_H@
-# endif
+#if @HAVE_SYS_FILE_H@
+# @INCLUDE_NEXT@ @NEXT_SYS_FILE_H@
+#endif
 
 #ifndef _GL_SYS_FILE_H
 #define _GL_SYS_FILE_H
--- lib/sys_ioctl.in.h.orig     Sun Mar 28 12:28:36 2010
+++ lib/sys_ioctl.in.h  Sun Mar 28 12:17:51 2010
@@ -15,12 +15,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_SYS_IOCTL_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SYS_IOCTL_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if @HAVE_SYS_IOCTL_H@
 # @INCLUDE_NEXT@ @NEXT_SYS_IOCTL_H@
--- lib/sys_time.in.h.orig      Sun Mar 28 12:28:36 2010
+++ lib/sys_time.in.h   Sun Mar 28 12:19:35 2010
@@ -18,12 +18,12 @@
 
 /* Written by Paul Eggert.  */
 
+#if defined _GL_SYS_TIME_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#if defined _GL_SYS_TIME_H
-
 /* Simply delegate to the system's header, without adding anything.  */
 # if @HAVE_SYS_TIME_H@
 #  @INCLUDE_NEXT@ @NEXT_SYS_TIME_H@
--- lib/sys_times.in.h.orig     Sun Mar 28 12:28:36 2010
+++ lib/sys_times.in.h  Sun Mar 28 12:19:47 2010
@@ -20,12 +20,12 @@
 /* This file is supposed to be used on platforms where <sys/times.h>
    is missing.  */
 
+#ifndef _GL_SYS_TIMES_H
+
 # if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 # endif
 
-#ifndef _GL_SYS_TIMES_H
-
 # if @HAVE_SYS_TIMES_H@
 #  @INCLUDE_NEXT@ @NEXT_SYS_TIMES_H@
 # endif
--- lib/sys_utsname.in.h.orig   Sun Mar 28 12:28:36 2010
+++ lib/sys_utsname.in.h        Sun Mar 28 12:20:29 2010
@@ -15,12 +15,12 @@
    along with this program; if not, write to the Free Software Foundation,
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
+#ifndef _GL_SYS_UTSNAME_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SYS_UTSNAME_H
-
 #if @HAVE_SYS_UTSNAME_H@
 # @INCLUDE_NEXT@ @NEXT_SYS_UTSNAME_H@
 #endif
--- lib/sys_wait.in.h.orig      Sun Mar 28 12:28:36 2010
+++ lib/sys_wait.in.h   Sun Mar 28 12:20:46 2010
@@ -16,12 +16,12 @@
    Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  */
 
 
+#ifndef _GL_SYS_WAIT_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SYS_WAIT_H
-
 /* The include_next requires a split double-inclusion guard.  */
 #if !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__)
 # @INCLUDE_NEXT@ @NEXT_SYS_WAIT_H@
--- lib/sysexits.in.h.orig      Sun Mar 28 12:28:36 2010
+++ lib/sysexits.in.h   Sun Mar 28 12:16:59 2010
@@ -16,12 +16,12 @@
 
 /* Written by Simon Josefsson based on sysexits(3) man page */
 
+#ifndef _GL_SYSEXITS_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_SYSEXITS_H
-
 #if @HAVE_SYSEXITS_H@
 
 /* IRIX 6.5 has an <unistd.h> that defines a macro EX_OK with a nonzero
--- lib/wctype.in.h.orig        Sun Mar 28 12:28:36 2010
+++ lib/wctype.in.h     Sun Mar 28 12:22:24 2010
@@ -26,12 +26,12 @@
  * wctrans_t, and wctype_t are not yet implemented.
  */
 
+#ifndef _GL_WCTYPE_H
+
 #if __GNUC__ >= 3
 @PRAGMA_SYSTEM_HEADER@
 #endif
 
-#ifndef _GL_WCTYPE_H
-
 #if @HAVE_WINT_T@
 /* Solaris 2.5 has a bug: <wchar.h> must be included before <wctype.h>.
    Tru64 with Desktop Toolkit C has a bug: <stdio.h> must be included before