too many warnings from Bison CVS for Pike

[Paul Eggert]

After trying out recent-CVS Bison on a couple of real-world grammars,
I came to the conclusion that the new warnings cry wolf too often.
For example, if you use it on the current grammar for Pike
<http://pike.ida.liu.se/cvs/Pike/7.7/src/language.yacc?1.364>, you get
199 warnings, most of them bogus.

The Pike grammar, and I suspect many other grammars, relies on the
fact that "$$ = $1" is always executed before the actual action, which
means that, for example, you can selectively fill in $$ afterwards, or
know that a rule like this:

bad_expr_ident:
    TOK_INLINE   { error_reserved("inline"); }
  | TOK_LOCAL_ID { error_reserved("local"); }
  | ...

will print an error message but also propagate $$ = $1.

I checked the Posix spec for Yacc, and it's not entirely clear in this
area and it could be interpreted the way that Pike requires.
<http://www.opengroup.org/onlinepubs/009695399/utilities/yacc.html>
says "By default, the value of a rule shall be the value of the first
element in it."  It also says, "The value of the action can be set by
assigning it to $$."  One could easily interpret this to mean that the
equivalent of "$$ = $1" must be executed before each action.

I suppose we could ask for a clarification from the Open Group, but
regardless of what they say, I suspect that many grammars rely on this
assumption and we should think twice before generating warnings about
it.

In the long run I think Bison needs different flavors of warning
options, for users who want to be more or less picky.  In the short
run, though, we want to generate a new release soon, and the
recent-CVS Bison is yammering too much for production use.

For now I installed the following patch, so that the warnings are
issued only when destructors are involved.  This goes too far, in that
it shuts off too many of the new warnings, but we can add them back
later once we figure things out (e.g., by adding warning-related
options).

Another thought is that perhaps these warnings should go the same place
that shift/reduce warnings go, namely the y.output file.  But this is
also a matter for the next release.

2006-01-27  Paul Eggert  <address@hidden>

        * src/reader.c (symbol_should_be_used): Renamed from symbol_typed_p.
        All used changed.  Check whether the symbol has a destructor,
        not whether it is typed.
        * tests/input.at (AT_CHECK_UNUSED_VALUES): Add a destructor, so
        that the values are still reported as unused.  All line numbers
        adjusted.

Index: src/reader.c
===================================================================
RCS file: /cvsroot/bison/bison/src/reader.c,v
retrieving revision 1.251
diff -p -u -r1.251 reader.c
--- src/reader.c        23 Jan 2006 04:37:09 -0000      1.251
+++ src/reader.c        27 Jan 2006 22:49:16 -0000
@@ -212,17 +212,16 @@ grammar_current_rule_begin (symbol *lhs,
 }
 
 
-/*-----------------------------------------------------------------.
-| A symbol is typed if it has a declared %type, or if it is a      |
-| mid-rule symbol (i.e., the generated LHS replacing a mid-rule    |
-| action) that was assigned to, as in `exp: { $$ = 1; } { $$ = $1; |
-| }'.                                                              |
-`-----------------------------------------------------------------*/
+/*----------------------------------------------------------------------.
+| A symbol should be used if it has a destructor, or if it is a         |
+| mid-rule symbol (i.e., the generated LHS replacing a mid-rule         |
+| action) that was assigned to, as in "exp: { $$ = 1; } { $$ = $1; }".  |
+`----------------------------------------------------------------------*/
 
 static bool
-symbol_typed_p (const symbol_list *s)
+symbol_should_be_used (symbol_list const *s)
 {
-  return (s->sym->type_name
+  return (s->sym->destructor
          || (s->midrule && s->midrule->used));
 }
 
@@ -261,13 +260,13 @@ grammar_rule_check (const symbol_list *r
                 _("empty rule for typed nonterminal, and no action"));
     }
 
-  /* Check that typed symbol values are used.  */
+  /* Check that symbol values that should be used are in fact used.  */
   {
     symbol_list const *l = r;
     int n = 0;
     for (; l && l->sym; l = l->next, ++n)
       if (! (l->used
-            || !symbol_typed_p (l)
+            || !symbol_should_be_used (l)
             /* The default action, $$ = $1, `uses' both.  */
             || (!r->action && (n == 0 || n == 1))))
        {
Index: tests/input.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/input.at,v
retrieving revision 1.38
diff -p -u -r1.38 input.at
--- tests/input.at      4 Jan 2006 09:18:37 -0000       1.38
+++ tests/input.at      27 Jan 2006 22:49:16 -0000
@@ -92,6 +92,7 @@ m4_define([AT_CHECK_UNUSED_VALUES],
 AT_DATA([input.y],
 [[%token <integer> INT
 %type <integer> exp
+%destructor { destroy ($$); } INT exp
 %%
 exp:
   $1
@@ -106,56 +107,56 @@ AT_CLEANUP
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { } INT { } INT { }],
-[input.y:5.3-25: warning: unset value: $$
-input.y:5.3-25: warning: unused value: $1
-input.y:5.3-25: warning: unused value: $3
-input.y:5.3-25: warning: unused value: $5
+[input.y:6.3-25: warning: unset value: $$
+input.y:6.3-25: warning: unused value: $1
+input.y:6.3-25: warning: unused value: $3
+input.y:6.3-25: warning: unused value: $5
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { $1 } INT { } INT { }],
-[input.y:5.3-28: warning: unset value: $$
-input.y:5.3-28: warning: unused value: $3
-input.y:5.3-28: warning: unused value: $5
+[input.y:6.3-28: warning: unset value: $$
+input.y:6.3-28: warning: unused value: $3
+input.y:6.3-28: warning: unused value: $5
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { } INT { $1 } INT { }],
-[input.y:5.3-28: warning: unset value: $$
-input.y:5.3-28: warning: unused value: $3
-input.y:5.3-28: warning: unused value: $5
+[input.y:6.3-28: warning: unset value: $$
+input.y:6.3-28: warning: unused value: $3
+input.y:6.3-28: warning: unused value: $5
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { } INT {  } INT { $1 }],
-[input.y:5.3-29: warning: unset value: $$
-input.y:5.3-29: warning: unused value: $3
-input.y:5.3-29: warning: unused value: $5
+[input.y:6.3-29: warning: unset value: $$
+input.y:6.3-29: warning: unused value: $3
+input.y:6.3-29: warning: unused value: $5
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { } INT {  } INT { $$ = $1 + $3 + $5; }])
 
 # Checking mid-rule values.
 AT_CHECK_UNUSED_VALUES([INT { $$ } INT { $$ } INT { }],
-[input.y:5.3-31: warning: unset value: $$
-input.y:5.3-31: warning: unused value: $1
-input.y:5.3-31: warning: unused value: $2
-input.y:5.3-31: warning: unused value: $3
-input.y:5.3-31: warning: unused value: $4
-input.y:5.3-31: warning: unused value: $5
+[input.y:6.3-31: warning: unset value: $$
+input.y:6.3-31: warning: unused value: $1
+input.y:6.3-31: warning: unused value: $2
+input.y:6.3-31: warning: unused value: $3
+input.y:6.3-31: warning: unused value: $4
+input.y:6.3-31: warning: unused value: $5
 ])
 
 AT_CHECK_UNUSED_VALUES([INT { $$ } INT { $$ = $2 } INT { }],
-[input.y:5.3-36: warning: unset value: $$
-input.y:5.3-36: warning: unused value: $1
-input.y:5.3-36: warning: unused value: $3
-input.y:5.3-36: warning: unused value: $4
-input.y:5.3-36: warning: unused value: $5
+[input.y:6.3-36: warning: unset value: $$
+input.y:6.3-36: warning: unused value: $1
+input.y:6.3-36: warning: unused value: $3
+input.y:6.3-36: warning: unused value: $4
+input.y:6.3-36: warning: unused value: $5
 ])
 
 # AT_CHECK_UNUSED_VALUES([INT { $$ } { $$ = $2 } { }],
-# [input.y:5.3-36: warning: unset value: $$
-# input.y:5.3-36: warning: unused value: $1
-# input.y:5.3-36: warning: unused value: $3
-# input.y:5.3-36: warning: unused value: $4
-# input.y:5.3-36: warning: unused value: $5
+# [input.y:6.3-36: warning: unset value: $$
+# input.y:6.3-36: warning: unused value: $1
+# input.y:6.3-36: warning: unused value: $3
+# input.y:6.3-36: warning: unused value: $4
+# input.y:6.3-36: warning: unused value: $5
 # ])
 
 AT_CHECK_UNUSED_VALUES([INT { $$ = $1 } INT { $$ = $2 + $3 } INT { $$ = $4 + 
$5 }])