Re: FYI: fixed yycheck signed/unsigned comparison warnings

[Paul Eggert]

"Joel E. Denny" <address@hidden> writes:

> I configure bison with --enable-gcc-warnings.  I tried adding a new token 
> to parse-gram.y today, but it failed to compile as gcc complained of 
> comparisons between signed and unsigned for yycheck.

In practice that warning causes a lot of makework, and I think it
harms more than it helps since the extra casts might disguise other
problems.  I installed the following patch to disable that warning
instead of having the casts.

There is one other warning worth adding, though: -Wpointer-arith.

2006-08-11  Paul Eggert  <address@hidden>

        * configure.ac (AC_ARG_ENABLE): Use -Wextra -Wno-sign-compare
        rather than -W, so we don't get bogus warnings about sign comparisons.
        Add -Wpointer-arith, since that warning is useful (it reports code
        that does not conform to C89 and that some compilers reject).
        * data/c.m4, data/glr.c, data/lalr1.cc, data/yacc.c: Undo latest change,
        since it's no longer needed.

Index: configure.ac
===================================================================
RCS file: /cvsroot/bison/bison/configure.ac,v
retrieving revision 1.70
diff -p -u -r1.70 configure.ac
--- configure.ac        25 Jul 2006 06:45:05 -0000      1.70
+++ configure.ac        11 Aug 2006 19:48:05 -0000
@@ -45,11 +45,12 @@ if test "${enableval}" = yes; then
   gl_WARNING_CFLAGS([-Werror])
   AC_SUBST([WERROR_CFLAGS], [$WARNING_CFLAGS])
   WARNING_CFLAGS=
-  gl_WARNING_CFLAGS([-W])
+  gl_WARNING_CFLAGS([-Wextra -Wno-sign-compare])
   gl_WARNING_CFLAGS([-Wall])
   gl_WARNING_CFLAGS([-Wcast-align])
   gl_WARNING_CFLAGS([-Wcast-qual])
   gl_WARNING_CFLAGS([-Wformat])
+  gl_WARNING_CFLAGS([-Wpointer-arith])
   gl_WARNING_CFLAGS([-Wwrite-strings])
   AC_SUBST([WARNING_CXXFLAGS], [$WARNING_CFLAGS])
   # The following warnings are not suitable for C++.
Index: data/c.m4
===================================================================
RCS file: /cvsroot/bison/bison/data/c.m4,v
retrieving revision 1.63
diff -p -u -r1.63 c.m4
--- data/c.m4   10 Aug 2006 01:18:50 -0000      1.63
+++ data/c.m4   11 Aug 2006 19:48:05 -0000
@@ -164,11 +164,6 @@ m4_define([b4_int_type],
 
                                               [int])])
 
-# b4_safest_int_type
-# ------------------
-# The safest int type to cast to when it's necessary to compare a signed int
-# type against an int type set by b4_int_type.
-m4_define([b4_safest_int_type],[int])
 
 # b4_int_type_for(NAME)
 # ---------------------
Index: data/glr.c
===================================================================
RCS file: /cvsroot/bison/bison/data/glr.c,v
retrieving revision 1.187
diff -p -u -r1.187 glr.c
--- data/glr.c  10 Aug 2006 01:18:50 -0000      1.187
+++ data/glr.c  11 Aug 2006 19:48:05 -0000
@@ -1048,8 +1048,7 @@ yygetLRActions (yyStateNum yystate, int 
                int* yyaction, const short int** yyconflicts)
 {
   int yyindex = yypact[yystate] + yytoken;
-  if (yyindex < 0 || YYLAST < yyindex
-      || (]b4_safest_int_type[)yycheck[yyindex] != yytoken)
+  if (yyindex < 0 || YYLAST < yyindex || yycheck[yyindex] != yytoken)
     {
       *yyaction = -yydefact[yystate];
       *yyconflicts = yyconfl;
@@ -1071,8 +1070,7 @@ yyLRgotoState (yyStateNum yystate, yySym
 {
   int yyr;
   yyr = yypgoto[yylhs - YYNTOKENS] + yystate;
-  if (0 <= yyr && yyr <= YYLAST
-      && (]b4_safest_int_type[)yycheck[yyr] == yystate)
+  if (0 <= yyr && yyr <= YYLAST && yycheck[yyr] == yystate)
     return yytable[yyr];
   else
     return yydefgoto[yylhs - YYNTOKENS];
@@ -2122,8 +2120,7 @@ yyreportSyntaxError (yyGLRStack* yystack
          yyfmt = yystpcpy (yyformat, yyunexpected);
 
          for (yyx = yyxbegin; yyx < yyxend; ++yyx)
-           if ((]b4_safest_int_type[)yycheck[yyx + yyn] == yyx
-                && yyx != YYTERROR)
+           if (yycheck[yyx + yyn] == yyx && yyx != YYTERROR)
              {
                if (yycount == YYERROR_VERBOSE_ARGS_MAXIMUM)
                  {
@@ -2219,8 +2216,7 @@ yyrecoverSyntaxError (yyGLRStack* yystac
        if (yyis_pact_ninf (yyj))
          return;
        yyj += yytoken;
-       if (yyj < 0 || YYLAST < yyj
-            || (]b4_safest_int_type[)yycheck[yyj] != yytoken)
+       if (yyj < 0 || YYLAST < yyj || yycheck[yyj] != yytoken)
          {
            if (yydefact[yystackp->yytops.yystates[0]->yylrState] != 0)
              return;
Index: data/lalr1.cc
===================================================================
RCS file: /cvsroot/bison/bison/data/lalr1.cc,v
retrieving revision 1.140
diff -p -u -r1.140 lalr1.cc
--- data/lalr1.cc       10 Aug 2006 01:18:50 -0000      1.140
+++ data/lalr1.cc       11 Aug 2006 19:48:05 -0000
@@ -608,8 +608,7 @@ m4_ifdef([b4_lex_param], [, ]b4_lex_para
     /* If the proper action on seeing token YYTOKEN is to reduce or to
        detect an error, take that action.  */
     yyn += yytoken;
-    if (yyn < 0 || yylast_ < yyn
-        || (]b4_safest_int_type[)yycheck_[yyn] != yytoken)
+    if (yyn < 0 || yylast_ < yyn || yycheck_[yyn] != yytoken)
       goto yydefault;
 
     /* Reduce or error.  */
@@ -687,7 +686,7 @@ m4_ifdef([b4_lex_param], [, ]b4_lex_para
     yyn = yyr1_[yyn];
     yystate = yypgoto_[yyn - yyntokens_] + yystate_stack_[0];
     if (0 <= yystate && yystate <= yylast_
-       && (]b4_safest_int_type[)yycheck_[yystate] == yystate_stack_[0])
+       && yycheck_[yystate] == yystate_stack_[0])
       yystate = yytable_[yystate];
     else
       yystate = yydefgoto_[yyn - yyntokens_];
@@ -760,8 +759,7 @@ b4_error_verbose_if([, yytoken])[));
        if (yyn != yypact_ninf_)
        {
          yyn += yyterror_;
-         if (0 <= yyn && yyn <= yylast_
-              && (]b4_safest_int_type[)yycheck_[yyn] == yyterror_)
+         if (0 <= yyn && yyn <= yylast_ && yycheck_[yyn] == yyterror_)
            {
              yyn = yytable_[yyn];
              if (0 < yyn)
@@ -845,7 +843,7 @@ b4_error_verbose_if([, int tok])[)
        int yyxend = yychecklim < yyntokens_ ? yychecklim : yyntokens_;
        int count = 0;
        for (int x = yyxbegin; x < yyxend; ++x)
-         if ((]b4_safest_int_type[)yycheck_[x + yyn] == x && x != yyterror_)
+         if (yycheck_[x + yyn] == x && x != yyterror_)
            ++count;
 
        // FIXME: This method of building the message is not compatible
@@ -862,8 +860,7 @@ b4_error_verbose_if([, int tok])[)
          {
            count = 0;
            for (int x = yyxbegin; x < yyxend; ++x)
-             if ((]b4_safest_int_type[)yycheck_[x + yyn] == x
-                  && x != yyterror_)
+             if (yycheck_[x + yyn] == x && x != yyterror_)
                {
                  res += (!count++) ? ", expecting " : " or ";
                  res += yytnamerr_ (yytname_[x]);
Index: data/yacc.c
===================================================================
RCS file: /cvsroot/bison/bison/data/yacc.c,v
retrieving revision 1.152
diff -p -u -r1.152 yacc.c
--- data/yacc.c 10 Aug 2006 01:18:50 -0000      1.152
+++ data/yacc.c 11 Aug 2006 19:48:05 -0000
@@ -86,15 +86,6 @@ m4_define([b4_int_type],
                                               [int])])
 
 
-# b4_safest_int_type
-# ------------------
-# The safest int type to cast to when it's necessary to compare a signed int
-# type against an int type set by b4_int_type.  Overwrite the version from
-# c.m4 as a reminder to keep it consistent with overwritten b4_int_type.
-m4_define([b4_safest_int_type],[int])
-
-
-
 ## ----------------- ##
 ## Semantic Values.  ##
 ## ----------------- ##
@@ -909,7 +900,7 @@ yysyntax_error (char *yyresult, int yyst
       yyfmt = yystpcpy (yyformat, yyunexpected);
 
       for (yyx = yyxbegin; yyx < yyxend; ++yyx)
-       if ((]b4_safest_int_type[)yycheck[yyx + yyn] == yyx && yyx != YYTERROR)
+       if (yycheck[yyx + yyn] == yyx && yyx != YYTERROR)
          {
            if (yycount == YYERROR_VERBOSE_ARGS_MAXIMUM)
              {
@@ -1214,7 +1205,7 @@ yybackup:
   /* If the proper action on seeing token YYTOKEN is to reduce or to
      detect an error, take that action.  */
   yyn += yytoken;
-  if (yyn < 0 || YYLAST < yyn || (]b4_safest_int_type[)yycheck[yyn] != yytoken)
+  if (yyn < 0 || YYLAST < yyn || yycheck[yyn] != yytoken)
     goto yydefault;
   yyn = yytable[yyn];
   if (yyn <= 0)
@@ -1294,8 +1285,7 @@ yyreduce:
   yyn = yyr1[yyn];
 
   yystate = yypgoto[yyn - YYNTOKENS] + *yyssp;
-  if (0 <= yystate && yystate <= YYLAST
-      && (]b4_safest_int_type[)yycheck[yystate] == *yyssp)
+  if (0 <= yystate && yystate <= YYLAST && yycheck[yystate] == *yyssp)
     yystate = yytable[yystate];
   else
     yystate = yydefgoto[yyn - YYNTOKENS];