Re: %destructor feedback

[Akim Demaille]

>>> "Akim" == Akim Demaille <address@hidden> writes:

 > Same here: I would have liked the first algorithm, but the current
 > implementation (rules are in a linked list of symbols, not even of
 > rules) makes it hard to consult the previous (mid-)rules.  So I
 > installed the following which finishes to implementation the smaller
 > version.

I have found simple way to do it.  But I have several problems.

1. A minor one
Paul, in English you'd write mid-rule action, right?  And in code?
mid_rule, midrule?

2. An annoying one:

exp: a_1 a_2 { $<val>$ = 3; } { $<val>$ = $<val>3 + 1; } a_5
     sum_of_the_five_previous_values
    {
       printf ("%d\n", $6);
    }

this rule is actually using all the values from $1 to $5 (the rule for
$6 uses $-n to do that).  So one wishes to spell that out to Bison to
avoid the warning:

      USE (($1, $2, $3, $4, $5));

But then... bison complain about our using $3 and $4 with no specified
type tag, i.e., one has to write

      USE (($1, $2, $<foo>3, $<bar>4, $5));


3. An uncovered bug with mid-rule actions that follow themselves.

src/bison/tests % cat /tmp/input.y                              nostromo Err 1
%type <integer> exp
%%
exp: {$$} {$1} {$2} {}
;
src/bison/tests % LC_ALL=C _build/bison /tmp/input.y            nostromo 10:15
/tmp/input.y:3.12-13: invalid $ value: $1
/tmp/input.y:3.17-18: invalid $ value: $2
/tmp/input.y:3.6-22: warning: unset value: $$
/tmp/input.y:3.6-22: warning: unused value: $1

it complains about $1 and $2 which are fine.


Still, I installed the following.


Index: ChangeLog
from  Akim Demaille  <address@hidden>

        Also warn about non-used mid-rule values.
        * src/symlist.h, src/symlist.c (symbol_list): Add a mid_rule
        member.
        (symbol_list_new): Adjust.
        * src/reader.c (symbol_typed_p): New.
        (grammar_rule_check): Use it.
        (grammar_midrule_action): Bind a mid-rule LHS to its rule.
        Check its rule.
        * tests/input.at (AT_CHECK_UNUSED_VALUES): New.
        Use it.

Index: NEWS
===================================================================
RCS file: /cvsroot/bison/bison/NEWS,v
retrieving revision 1.139
diff -u -u -r1.139 NEWS
--- NEWS 27 Dec 2005 17:50:00 -0000 1.139
+++ NEWS 4 Jan 2006 09:10:56 -0000
@@ -26,6 +26,17 @@
        | exp "+" exp         { $$ = $1; (void) $3; }
        ;
 
+  If there are mid-rule actions, the warning is issued if no action
+  uses it.  The following triggers no warning: $1 and $3 are used.
+
+     exp: exp { push ($1); } '+' exp { push ($3); sum (); };
+
+  Mid-rule actions that use $$ cause the corresponding value to be
+  set, therefore the following action must use it.  The following rule
+  triggers a warning about $2.
+
+     exp: '1' { $$ = 1; } '+' exp { $$ = $1 + $4; };
+
   The warning is intended to help catching lost values and memory leaks.
   If a value is ignored, its associated memory typically is not reclaimed.
 
Index: src/reader.c
===================================================================
RCS file: /cvsroot/bison/bison/src/reader.c,v
retrieving revision 1.245
diff -u -u -r1.245 reader.c
--- src/reader.c 4 Jan 2006 08:32:46 -0000 1.245
+++ src/reader.c 4 Jan 2006 09:10:56 -0000
@@ -210,6 +210,20 @@
 }
 
 
+/*-----------------------------------------------------------------.
+| 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; |
+| }'.                                                              |
+`-----------------------------------------------------------------*/
+
+static bool
+symbol_typed_p (const symbol_list *s)
+{
+  return (s->sym->type_name
+         || s->mid_rule && s->mid_rule->used);
+}
+
 /*----------------------------------------------------------------.
 | Check that the rule R is properly defined.  For instance, there |
 | should be no type clash on the default action.                  |
@@ -251,7 +265,7 @@
     int n = 0;
     for (; l && l->sym; l = l->next, ++n)
       if (! (l->used
-            || !l->sym->type_name
+            || !symbol_typed_p (l)
             /* The default action, $$ = $1, `uses' both.  */
             || (!r->action && (n == 0 || n == 1))))
        {
@@ -318,14 +332,16 @@
     grammar = midrule;
 
   /* End the dummy's rule.  */
-  previous_rule_end = symbol_list_new (NULL, dummy_location);
-  previous_rule_end->next = current_rule;
+  midrule->next = symbol_list_new (NULL, dummy_location);
+  grammar_rule_check (midrule);
+  midrule->next->next = current_rule;
 
-  midrule->next = previous_rule_end;
+  previous_rule_end = midrule->next;
 
   /* Insert the dummy nonterminal replacing the midrule action into
-     the current rule.  */
+     the current rule.  Bind it to its dedicated rule.  */
   grammar_current_rule_symbol_append (dummy, dummy_location);
+  grammar_end->mid_rule = midrule;
 }
 
 /* Set the precedence symbol of the current rule to PRECSYM. */
Index: src/symlist.c
===================================================================
RCS file: /cvsroot/bison/bison/src/symlist.c,v
retrieving revision 1.15
diff -u -u -r1.15 symlist.c
--- src/symlist.c 4 Jan 2006 08:32:46 -0000 1.15
+++ src/symlist.c 4 Jan 2006 09:10:56 -0000
@@ -1,6 +1,6 @@
 /* Lists of symbols for Bison
 
-   Copyright (C) 2002, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2002, 2005, 2006 Free Software Foundation, Inc.
 
    This file is part of Bison, the GNU Compiler Compiler.
 
@@ -38,6 +38,8 @@
   res->sym = sym;
   res->location = loc;
 
+  res->mid_rule = NULL;
+
   res->action = NULL;
   res->used = false;
 
Index: src/symlist.h
===================================================================
RCS file: /cvsroot/bison/bison/src/symlist.h,v
retrieving revision 1.12
diff -u -u -r1.12 symlist.h
--- src/symlist.h 4 Jan 2006 08:32:46 -0000 1.12
+++ src/symlist.h 4 Jan 2006 09:10:56 -0000
@@ -32,6 +32,10 @@
   symbol *sym;
   location location;
 
+  /* If this symbol is the generated lhs for a mid-rule, a pointer to
+     that mid-rule.  */
+  struct symbol_list *mid_rule;
+
   /* The action is attached to the LHS of a rule. */
   const char *action;
   location action_location;
Index: tests/actions.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/actions.at,v
retrieving revision 1.57
diff -u -u -r1.57 actions.at
--- tests/actions.at 4 Jan 2006 08:32:46 -0000 1.57
+++ tests/actions.at 4 Jan 2006 09:10:56 -0000
@@ -113,6 +113,7 @@
 exp: a_1 a_2 { $<val>$ = 3; } { $<val>$ = $<val>3 + 1; } a_5
      sum_of_the_five_previous_values
     {
+       USE (($1, $2, $<foo>3, $<foo>4, $5));
        printf ("%d\n", $6);
     }
 ;
@@ -146,11 +147,7 @@
 }
 ]])
 
-AT_CHECK([bison -d -v -o input.c input.y], 0, [],
-[input.y:30.6-34.5: warning: unused value: $1
-input.y:30.6-34.5: warning: unused value: $2
-input.y:30.6-34.5: warning: unused value: $5
-])
+AT_CHECK([bison -d -v -o input.c input.y], 0)
 AT_COMPILE([input])
 AT_PARSER_CHECK([./input], 0,
 [[15
Index: tests/input.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/input.at,v
retrieving revision 1.37
diff -u -u -r1.37 input.at
--- tests/input.at 4 Jan 2006 08:32:46 -0000 1.37
+++ tests/input.at 4 Jan 2006 09:10:56 -0000
@@ -86,51 +86,80 @@
 ## Unused values.  ##
 ## --------------- ##
 
-AT_SETUP([Unused values])
+m4_define([AT_CHECK_UNUSED_VALUES],
+[AT_SETUP([Unused values])
 
 AT_DATA([input.y],
 [[%token <integer> INT
 %type <integer> exp
 %%
 exp:
-  INT { } INT { } INT { }
-/* Ideally we would like to complain also about $2 and $4 here, but
-   it's hard to implement.  */
-| INT { $$ } INT { $$ } INT { }
-| INT { $1 } INT { } INT { }
-| INT { } INT { $1 } INT { }
-| INT { } INT {  } INT { $1 }
-| INT { } INT {  } INT { $$ = $1 + $3 + $5; }
+  $1
+| INT
 ;
 ]])
 
 AT_CHECK([bison input.y], [], [],
-[[input.y:5.3-25: warning: unset value: $$
+[[$2]])
+
+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:8.3-31: warning: unset value: $$
-input.y:8.3-31: warning: unused value: $1
-input.y:8.3-31: warning: unused value: $3
-input.y:8.3-31: warning: unused value: $5
-input.y:9.3-28: warning: unset value: $$
-input.y:9.3-28: warning: unused value: $3
-input.y:9.3-28: warning: unused value: $5
-input.y:10.3-28: warning: unset value: $$
-input.y:10.3-28: warning: unused value: $3
-input.y:10.3-28: warning: unused value: $5
-input.y:11.3-29: warning: unset value: $$
-input.y:11.3-29: warning: unused value: $3
-input.y:11.3-29: warning: unused value: $5
-input.y: conflicts: 1 reduce/reduce
-input.y:8.7-12: warning: rule never reduced because of conflicts: @3: /* empty 
*/
-input.y:9.7-12: warning: rule never reduced because of conflicts: @5: /* empty 
*/
-input.y:10.7-9: warning: rule never reduced because of conflicts: @7: /* empty 
*/
-input.y:11.7-9: warning: rule never reduced because of conflicts: @9: /* empty 
*/
-input.y:12.7-9: warning: rule never reduced because of conflicts: @11: /* 
empty */
-]])
+])
+
+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
+])
+
+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
+])
+
+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
+])
+
+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
+])
+
+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
+])
+
+# 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
+# ])
+
+AT_CHECK_UNUSED_VALUES([INT { $$ = $1 } INT { $$ = $2 + $3 } INT { $$ = $4 + 
$5 }])
 
-AT_CLEANUP
 
 
 ## ---------------------- ##