Re: warning: unused value: $3

[Joel E. Denny]

On Tue, 31 Oct 2006, Joel E. Denny wrote:

> I'm thinking of committing the following.  Any comments from anyone?

> +     Disable unset/unused mid-rule value warnings by default, and recognize
> +     --warnings=midrule-values to enable them.  Discussed starting at
> +     <http://lists.gnu.org/archive/html/help-bison/2006-10/msg00030.html>.

In case it helps, here's the patch again still uncommitted but updated for 
my recent commit.

Index: ChangeLog
===================================================================
RCS file: /sources/bison/bison/ChangeLog,v
retrieving revision 1.1597
diff -p -u -r1.1597 ChangeLog
--- ChangeLog   1 Nov 2006 01:47:44 -0000       1.1597
+++ ChangeLog   1 Nov 2006 04:29:14 -0000
@@ -1,5 +1,19 @@
 2006-10-31  Joel E. Denny  <address@hidden>
 
+       Disable unset/unused mid-rule value warnings by default, and recognize
+       --warnings=midrule-values to enable them.  Discussed starting at
+       <http://lists.gnu.org/archive/html/help-bison/2006-10/msg00030.html>.
+       * NEWS (2.3a+): Mention.
+       * src/getargs.c, src/getargs.h (warnings_args, warnings_types, enum
+       warnings): Add entry for midrule-values subargument.
+       * src/reader.c (symbol_should_be_used): Don't return true just because
+       the value is a set/used mid-rule value unless
+       --warnings=midrule-values was specified.
+       * tests/input.at (Unused values, Unused values before symbol
+       declarations): Run tests with and without --warnings=midrule-values.
+
+2006-10-31  Joel E. Denny  <address@hidden>
+
        Finish implementing --warnings=error, which should not be implied by
        --warnings=all (or by its synonyms -W and --warnings without
        subarguments).
Index: NEWS
===================================================================
RCS file: /sources/bison/bison/NEWS,v
retrieving revision 1.164
diff -p -u -r1.164 NEWS
--- NEWS        21 Oct 2006 10:03:35 -0000      1.164
+++ NEWS        1 Nov 2006 04:29:15 -0000
@@ -6,6 +6,26 @@ Changes in version 2.3a+ (????-??-??):
 * The -g and --graph options now output graphs in Graphviz DOT format,
   not VCG format.
 
+* Revised warning: unset or unused mid-rule values
+
+  Since Bison 2.2, Bison has warned about mid-rule values that are set but not
+  used within any of the actions of the parent rule.  For example, Bison warns
+  about unused $2 in:
+
+    exp: '1' { $$ = 1; } '+' exp { $$ = $1 + $4; };
+
+  Now, Bison also warns about mid-rule values that are used but not set.  For
+  example, Bison warns about unset $$ in the mid-rule action in:
+
+    exp: '1' { $1 = 1; } '+' exp { $$ = $2 + $4; };
+
+  However, Bison now disables both of these warnings by default since they
+  sometimes prove to be false alarms in existing grammars employing the Yacc
+  constructs $0 or $-N (where N is some positive integer).
+
+  To enable these warnings, specify the flag `--warnings=midrule-values' or
+  `-W', which is a synonym for `--warnings=all'.
+
 * Bison now recognizes two separate kinds of default %destructor's and
   %printer's:
 
Index: src/getargs.c
===================================================================
RCS file: /sources/bison/bison/src/getargs.c,v
retrieving revision 1.83
diff -p -u -r1.83 getargs.c
--- src/getargs.c       1 Nov 2006 01:47:44 -0000       1.83
+++ src/getargs.c       1 Nov 2006 04:29:15 -0000
@@ -203,16 +203,18 @@ static const char * const warnings_args[
 {
   /* In a series of synonyms, present the most meaningful first, so
      that argmatch_valid be more readable.  */
-  "none       - no warnings",
-  "yacc       - incompatibilities with POSIX YACC",
-  "all        - all of the above",
-  "error      - warnings are errors",
+  "none            - no warnings",
+  "midrule-values  - unset or unused midrule values",
+  "yacc            - incompatibilities with POSIX YACC",
+  "all             - all of the above",
+  "error           - warnings are errors",
   0
 };
 
 static const int warnings_types[] =
 {
   warnings_none,
+  warnings_midrule_values,
   warnings_yacc,
   warnings_all,
   warnings_error
Index: src/getargs.h
===================================================================
RCS file: /sources/bison/bison/src/getargs.h,v
retrieving revision 1.35
diff -p -u -r1.35 getargs.h
--- src/getargs.h       1 Nov 2006 01:47:44 -0000       1.35
+++ src/getargs.h       1 Nov 2006 04:29:15 -0000
@@ -111,7 +111,8 @@ enum warnings
   {
     warnings_none             = 0,      /**< Issue no warnings.  */
     warnings_error            = 1 << 0, /**< Warnings are treated as errors.  
*/
-    warnings_yacc             = 1 << 1, /**< POSIXME.  */
+    warnings_midrule_values   = 1 << 1, /**< Unset or unused midrule values.  
*/
+    warnings_yacc             = 1 << 2, /**< POSIXME.  */
     warnings_all              = ~warnings_error /**< All above warnings.  */
   };
 /** What warnings are issued.  */
Index: src/reader.c
===================================================================
RCS file: /sources/bison/bison/src/reader.c,v
retrieving revision 1.275
diff -p -u -r1.275 reader.c
--- src/reader.c        21 Oct 2006 04:52:43 -0000      1.275
+++ src/reader.c        1 Nov 2006 04:29:15 -0000
@@ -243,19 +243,24 @@ grammar_current_rule_begin (symbol *lhs,
 
 
 /*----------------------------------------------------------------------.
-| 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; }".  |
+| A symbol should be used if either:                                    |
+|   1. It has a destructor.                                             |
+|   2. --warnings=midrule-values and the symbol is a mid-rule symbol    |
+|      (i.e., the generated LHS replacing a mid-rule action) that was   |
+|      assigned to or used, as in "exp: { $$ = 1; } { $$ = $1; }".      |
 `----------------------------------------------------------------------*/
 
 static bool
 symbol_should_be_used (symbol_list const *s)
 {
-  return (symbol_destructor_get (s->content.sym)
-         || (s->midrule && s->midrule->used)
-         || (s->midrule_parent_rule
-             && symbol_list_n_get (s->midrule_parent_rule,
-                                    s->midrule_parent_rhs_index)->used));
+  if (symbol_destructor_get (s->content.sym))
+    return true;
+  if (warnings_flag & warnings_midrule_values)
+    return ((s->midrule && s->midrule->used)
+           || (s->midrule_parent_rule
+               && symbol_list_n_get (s->midrule_parent_rule,
+                                     s->midrule_parent_rhs_index)->used));
+  return false;
 }
 
 /*----------------------------------------------------------------.
Index: tests/input.at
===================================================================
RCS file: /sources/bison/bison/tests/input.at,v
retrieving revision 1.60
diff -p -u -r1.60 input.at
--- tests/input.at      21 Oct 2006 10:03:35 -0000      1.60
+++ tests/input.at      1 Nov 2006 04:29:15 -0000
@@ -80,13 +80,15 @@ m4_define([_AT_UNUSED_VALUES_DECLARATION
 %destructor { destroy ($$); } INT a b c d e f g h i j k l;]]])
 
 
-# AT_CHECK_UNUSED_VALUES(DECLARATIONS_AFTER)
-# --------------------------------------------
+# AT_CHECK_UNUSED_VALUES(DECLARATIONS_AFTER, CHECK_MIDRULE_VALUES)
+# ------------------------------------------------------------------
 # Generate a grammar to test unused values,
 # compile it, run it.  If DECLARATIONS_AFTER
 # is set, then the token, type, and destructor
 # declarations are generated after the rules
-# rather than before.
+# rather than before.  If CHECK_MIDRULE_VALUES
+# is set, then --warnings=midrule-values is
+# set.
 
 m4_define([AT_CHECK_UNUSED_VALUES],
 [AT_DATA([input.y],
@@ -116,19 +118,19 @@ l: INT | INT { $<integer>$ = $<integer>1
 _AT_UNUSED_VALUES_DECLARATIONS])
 )
 
-AT_CHECK([bison input.y], [0], [],
+AT_CHECK([bison]m4_ifval($2, [ --warnings=midrule-values ])[ input.y], [0], [],
 [[input.y:11.10-32: warning: unset value: $]$[
 input.y:11.10-32: warning: unused value: $]1[
 input.y:11.10-32: warning: unused value: $]3[
 input.y:11.10-32: warning: unused value: $]5[
 input.y:12.9: warning: empty rule for typed nonterminal, and no action
-input.y:13.14-19: warning: unset value: $$
+]]m4_ifval($2, [[[input.y:13.14-19: warning: unset value: $$
 input.y:13.25-39: warning: unset value: $$
-input.y:13.10-59: warning: unset value: $]$[
+]]])[[input.y:13.10-59: warning: unset value: $]$[
 input.y:13.10-59: warning: unused value: $]3[
 input.y:13.10-59: warning: unused value: $]5[
-input.y:14.14-16: warning: unset value: $$
-input.y:14.10-47: warning: unset value: $]$[
+]]m4_ifval($2, [[[input.y:14.14-16: warning: unset value: $$
+]]])[[input.y:14.10-47: warning: unset value: $]$[
 input.y:14.10-47: warning: unused value: $]3[
 input.y:14.10-47: warning: unused value: $]5[
 input.y:15.10-36: warning: unset value: $]$[
@@ -136,21 +138,21 @@ input.y:15.10-36: warning: unused value:
 input.y:15.10-36: warning: unused value: $]5[
 input.y:17.10-58: warning: unset value: $]$[
 input.y:17.10-58: warning: unused value: $]1[
-input.y:17.10-58: warning: unused value: $]2[
-input.y:17.10-58: warning: unused value: $]3[
-input.y:17.10-58: warning: unused value: $]4[
-input.y:17.10-58: warning: unused value: $]5[
+]]m4_ifval($2, [[[input.y:17.10-58: warning: unused value: $]2[
+]]])[[input.y:17.10-58: warning: unused value: $]3[
+]]m4_ifval($2, [[[input.y:17.10-58: warning: unused value: $]4[
+]]])[[input.y:17.10-58: warning: unused value: $]5[
 input.y:18.10-72: warning: unset value: $]$[
 input.y:18.10-72: warning: unused value: $]1[
 input.y:18.10-72: warning: unused value: $]3[
-input.y:18.10-72: warning: unused value: $]4[
-input.y:18.10-72: warning: unused value: $]5[
-input.y:20.10-55: warning: unused value: $]3[
-input.y:21.10-68: warning: unset value: $]$[
+]]m4_ifval($2, [[[input.y:18.10-72: warning: unused value: $]4[
+]]])[[input.y:18.10-72: warning: unused value: $]5[
+]]m4_ifval($2, [[[input.y:20.10-55: warning: unused value: $]3[
+]]])[[input.y:21.10-68: warning: unset value: $]$[
 input.y:21.10-68: warning: unused value: $]1[
 input.y:21.10-68: warning: unused value: $]2[
-input.y:21.10-68: warning: unused value: $]4[
-]])])
+]]m4_ifval($2, [[[input.y:21.10-68: warning: unused value: $]4[
+]]]))])
 
 
 ## --------------- ##
@@ -159,6 +161,7 @@ input.y:21.10-68: warning: unused value:
 
 AT_SETUP([Unused values])
 AT_CHECK_UNUSED_VALUES
+AT_CHECK_UNUSED_VALUES(, [1])
 AT_CLEANUP
 
 
@@ -168,6 +171,7 @@ AT_CLEANUP
 
 AT_SETUP([Unused values before symbol declarations])
 AT_CHECK_UNUSED_VALUES([1])
+AT_CHECK_UNUSED_VALUES([1], [1])
 AT_CLEANUP