Re: IF_LINT is dangerous (Was: Re: Version 2.5_rc1 gcc issues)

[Joel E. Denny]

On Sun, 1 May 2011, Paul Eggert wrote:

> On 05/01/11 20:45, Joel E. Denny wrote:
> 
> > I don't understand the benefit of suppressing initializations.  If 
> > the point is to document that the initialization isn't intended to be 
> > important to the logic of the program, then a comment saying so is 
> > sufficient.
> 
> It depends on how important efficiency is.  If it's not important,
> a comment should suffice.  If it is, then leaving the initialization
> in causes compilers to generate worse code.

I doubt any performance impact will be noticeable in Bison given the 
overhead of M4.

> > If there are no objections, I will soon replace all uses of IF_LINT with 
> > the code it encloses.
> 
> Please leave the gnulib IF_LINTs alone;

Sure, I just meant the IF_LINT occurrences in Bison sources.

> For Bison code where efficiency
> doesn't matter that much, you might consider putting something
> like this into system.h:
> 
>   /* Use this when the actual value X doesn't matter, but something
>      needs to be present to pacify compilers that would otherwise warn
>      about possibly-uninitialized variables.  */
>   #define DONT_CARE(x) (x)

Thanks for the suggestion.

I pushed the following to branch-2.5 and master.

>From 77bb73e7af76ef5180b22c3b8355aaff1f498f68 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sat, 14 May 2011 16:16:26 -0400
Subject: [PATCH] Don't use IF_LINT in Bison sources.

It creates unnecessary differences between the sources that Bison
maintainers build and test (given that maintainers normally
configure with --enable-gcc-warnings) and the sources that Bison
users build.  Instead, use PACIFY_CC, which doesn't.  This change
fixes compiler warnings reported by Tys Lefering at
<http://lists.gnu.org/archive/html/bison-patches/2011-05/msg00004.html>.
* configure.ac: Don't AC_DEFINE lint regardless of the configure
options.  This change affects imported gnulib sources, where
IF_LINT still appears and depends on lint.
* src/scan-gram.l, src/scan-skel.l: Replace uses of IF_LINT with
PACIFY_CC.
* src/system.h (IF_LINT): Remove cpp macro.
(PACIFY_CC): New cpp macro.
---
 ChangeLog       |   17 +++++++++++++++++
 configure.ac    |    1 -
 src/scan-gram.l |    8 ++++----
 src/scan-skel.l |    2 +-
 src/system.h    |   20 +++++++++++++-------
 5 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d29e4f2..51cf321 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2011-05-14  Joel E. Denny  <address@hidden>
+
+       Don't use IF_LINT in Bison sources.
+       It creates unnecessary differences between the sources that Bison
+       maintainers build and test (given that maintainers normally
+       configure with --enable-gcc-warnings) and the sources that Bison
+       users build.  Instead, use PACIFY_CC, which doesn't.  This change
+       fixes compiler warnings reported by Tys Lefering at
+       <http://lists.gnu.org/archive/html/bison-patches/2011-05/msg00004.html>.
+       * configure.ac: Don't AC_DEFINE lint regardless of the configure
+       options.  This change affects imported gnulib sources, where
+       IF_LINT still appears and depends on lint.
+       * src/scan-gram.l, src/scan-skel.l: Replace uses of IF_LINT with
+       PACIFY_CC.
+       * src/system.h (IF_LINT): Remove cpp macro.
+       (PACIFY_CC): New cpp macro.
+
 2011-05-01  Joel E. Denny  <address@hidden>
 
        Fix precedence for end token.
diff --git a/configure.ac b/configure.ac
index 2a80271..98fded2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -84,7 +84,6 @@ if test "${enableval}" = yes; then
   WARN_CFLAGS_TEST="$WARN_CFLAGS $WARN_CFLAGS_TEST"
   AC_SUBST([WARN_CXXFLAGS_TEST])
   AC_SUBST([WARN_CFLAGS_TEST])
-  AC_DEFINE([lint], 1, [Define to 1 if the compiler is checking for lint.])
 fi
 
 BISON_TEST_FOR_WORKING_C_COMPILER
diff --git a/src/scan-gram.l b/src/scan-gram.l
index 1995b94..7e60794 100644
--- a/src/scan-gram.l
+++ b/src/scan-gram.l
@@ -120,13 +120,13 @@ splice     (\\[ \f\t\v]*\n)*
 %%
 %{
   /* Nesting level of the current code in braces.  */
-  int braces_level IF_LINT (= 0);
+  int braces_level PACIFY_CC (= 0);
 
   /* Parent context state, when applicable.  */
-  int context_state IF_LINT (= 0);
+  int context_state PACIFY_CC (= 0);
 
   /* Location of most recent identifier, when applicable.  */
-  location id_loc IF_LINT (= empty_location);
+  location id_loc PACIFY_CC (= empty_location);
 
   /* Where containing code started, when applicable.  Its initial
      value is relevant only when yylex is invoked in the SC_EPILOGUE
@@ -135,7 +135,7 @@ splice       (\\[ \f\t\v]*\n)*
 
   /* Where containing comment or string or character literal started,
      when applicable.  */
-  boundary token_start IF_LINT (= scanner_cursor);
+  boundary token_start PACIFY_CC (= scanner_cursor);
 %}
 
 
diff --git a/src/scan-skel.l b/src/scan-skel.l
index ae0d63c..f2653be 100644
--- a/src/scan-skel.l
+++ b/src/scan-skel.l
@@ -58,7 +58,7 @@ static void fail_for_invalid_at (char const *at);
 %%
 
 %{
-  int out_lineno IF_LINT (= 0);
+  int out_lineno PACIFY_CC (= 0);
   char *outname = NULL;
 
   /* Currently, only the @warn, @complain, @fatal, @warn_at, @complain_at, and
diff --git a/src/system.h b/src/system.h
index 97a9225..13669a4 100644
--- a/src/system.h
+++ b/src/system.h
@@ -66,13 +66,19 @@ typedef size_t uintptr_t;
 | GCC extensions.  |
 `-----------------*/
 
-/* Use this to suppress gcc's `...may be used before initialized'
-   warnings.  */
-#ifdef lint
-# define IF_LINT(Code) Code
-#else
-# define IF_LINT(Code) /* empty */
-#endif
+/* Use PACIFY_CC to indicate that Code is unimportant to the logic of Bison
+   but that it is necessary for suppressing compiler warnings.  For example,
+   Code might be a variable initializer that's always overwritten before the
+   variable is used.
+
+   PACIFY_CC is intended to be useful only as a comment as it does not alter
+   Code.  It is tempting to redefine PACIFY_CC so that it will suppress Code
+   when configuring without --enable-gcc-warnings.  However, that would mean
+   that, for maintainers, Bison would compile with potentially less warnings
+   and safer logic than it would for users.  Due to the overhead of M4,
+   suppressing Code is unlikely to offer any significant improvement in
+   Bison's performance anyway.  */
+#define PACIFY_CC(Code) Code
 
 #ifndef __attribute__
 /* This feature is available in gcc versions 2.5 and later.  */
-- 
1.7.0.4