Re: FYI: default %printer/%destructor

[Joel E. Denny]

On Thu, 14 Sep 2006, Joel E. Denny wrote:

> By the way, currently, the default destructor applies to midrule values.  
> I think it should only apply to midrule values for which $$ is actually 
> used.  I don't think even <!> should apply if $$ isn't used in this case.  
> That is, when $$ isn't used, not only does the midrule not have a type, it 
> doesn't even have a value; it's just a midrule.

I committed the following.

Index: ChangeLog
===================================================================
RCS file: /sources/bison/bison/ChangeLog,v
retrieving revision 1.1592
diff -p -u -r1.1592 ChangeLog
--- ChangeLog   21 Oct 2006 02:31:50 -0000      1.1592
+++ ChangeLog   21 Oct 2006 04:50:33 -0000
@@ -1,3 +1,19 @@
+2006-10-21  Joel E. Denny  <address@hidden>
+
+       Don't apply the default %destructor/%printer to an unreferenced midrule
+       value.  Mentioned at
+       <http://lists.gnu.org/archive/html/bison-patches/2006-09/msg00104.html>.
+       * src/symtab.c (dummy_symbol_get): Name all dummy symbols initially
+       like address@hidden instead of just @n so that the default 
%destructor/%printer
+       logic doesn't see them as user-defined symbols.
+       (symbol_is_dummy): Check for both forms of the name.
+       * src/reader.c (packgram): Remove the `$' from each midrule symbol
+       name for which the midrule value is referenced in any action.
+       * tests/actions.at (Default %printer and %destructor for mid-rule
+       values): New test.
+       * tests/regression.at (Rule Line Numbers, Web2c Report): Update output
+       for change to dummy symbol names.
+
 2006-10-20  Joel E. Denny  <address@hidden>
 
        Warn about unset midrule $$ if the corresponding $n is used.
Index: src/reader.c
===================================================================
RCS file: /sources/bison/bison/src/reader.c,v
retrieving revision 1.274
diff -p -u -r1.274 reader.c
--- src/reader.c        21 Oct 2006 02:31:50 -0000      1.274
+++ src/reader.c        21 Oct 2006 04:50:33 -0000
@@ -459,11 +459,12 @@ packgram (void)
 
   rules = xnmalloc (nrules, sizeof *rules);
 
-  /* Before invoking grammar_rule_check on any rule, make sure
-     all actions have already been scanned in order to set `used' flags.
-     Otherwise, checking that a midrule's $$ is set will not always work
-     properly because the midrule check must forward-reference the midrule's
-     parent rule.  */
+  /* Before invoking grammar_rule_check on any rule, make sure all actions have
+     already been scanned in order to set `used' flags.  Otherwise, checking
+     that a midrule's $$ should be set will not always work properly because
+     the check must forward-reference the midrule's parent rule.  For the same
+     reason, all the `used' flags must be set before checking whether to remove
+     `$' from any midrule symbol name.  */
   while (p)
     {
       if (p->action)
@@ -492,6 +493,15 @@ packgram (void)
       rules[ruleno].action = p->action;
       rules[ruleno].action_location = p->action_location;
 
+      /* If the midrule's $$ is set or its $n is used, remove the `$' from the
+        symbol name so that it's a user-defined symbol so that the default
+        %destructor and %printer apply.  */
+      if (p->midrule_parent_rule
+          && (p->used
+             || symbol_list_n_get (p->midrule_parent_rule,
+                                   p->midrule_parent_rhs_index)->used))
+       p->content.sym->tag += 1;
+
       /* Don't check the generated rule 0.  It has no action, so some rhs
         symbols may appear unused, but the parsing algorithm ensures that
         %destructor's are invoked appropriately.  */
Index: src/symtab.c
===================================================================
RCS file: /sources/bison/bison/src/symtab.c,v
retrieving revision 1.82
diff -p -u -r1.82 symtab.c
--- src/symtab.c        15 Sep 2006 16:34:48 -0000      1.82
+++ src/symtab.c        21 Oct 2006 04:50:33 -0000
@@ -769,7 +769,7 @@ dummy_symbol_get (location loc)
 
   symbol *sym;
 
-  sprintf (buf, "@%d", ++dummy_count);
+  sprintf (buf, "address@hidden", ++dummy_count);
   sym = symbol_get (buf, loc);
   sym->class = nterm_sym;
   sym->number = nvars++;
@@ -779,7 +779,7 @@ dummy_symbol_get (location loc)
 bool
 symbol_is_dummy (const symbol *sym)
 {
-  return sym->tag[0] == '@';
+  return sym->tag[0] == '@' || (sym->tag[0] == '$' && sym->tag[1] == '@');
 }
 
 /*-------------------.
Index: tests/actions.at
===================================================================
RCS file: /sources/bison/bison/tests/actions.at,v
retrieving revision 1.72
diff -p -u -r1.72 actions.at
--- tests/actions.at    15 Oct 2006 12:37:07 -0000      1.72
+++ tests/actions.at    21 Oct 2006 04:50:33 -0000
@@ -1096,3 +1096,108 @@ AT_CHECK([bison -o input.c input.y])
 AT_COMPILE([input])
 
 AT_CLEANUP
+
+
+
+## ------------------------------------------------------ ##
+## Default %printer and %destructor for mid-rule values.  ##
+## ------------------------------------------------------ ##
+
+AT_SETUP([Default %printer and %destructor for mid-rule values])
+
+AT_DATA_GRAMMAR([[input.y]],
+[[%debug /* So that %printer is actually compiled.  */
+
+%{
+# include <stdio.h>
+# include <stdlib.h>
+  static void yyerror (const char *msg);
+  static int yylex (void);
+# define USE(SYM)
+# define YYLTYPE int
+# define YYLLOC_DEFAULT(Current, Rhs, N)
+# define YY_LOCATION_PRINT(File, Loc)
+%}
+
+%printer { fprintf (yyoutput, "%d", @$); } %symbol-default
+%destructor { fprintf (stderr, "DESTROY %d\n", @$); } %symbol-default
+
+%%
+
+start:
+  {           @$ = 1; } // Not set or used.
+  { USE ($$); @$ = 2; } // Both set and used.
+  { USE ($$); @$ = 3; } // Only set.
+  {           @$ = 4; } // Only used.
+  'c'
+  { USE (($$, $2, $4, $5)); @$ = 0; }
+  ;
+
+%%
+
+static int
+yylex (void)
+{
+  static int called;
+  if (called++)
+    abort ();
+  return 0;
+}
+
+static void
+yyerror (const char *msg)
+{
+  fprintf (stderr, "%s\n", msg);
+}
+
+int
+main (void)
+{
+  yydebug = 1;
+  return yyparse ();
+}
+]])
+
+AT_CHECK([bison -o input.c input.y], 0,,
+[[input.y:31.3-23: warning: unset value: $$
+input.y:28.3-33.37: warning: unused value: $3
+]])
+
+AT_COMPILE([input])
+AT_PARSER_CHECK([./input], 1,,
+[[Starting parse
+Entering state 0
+Reducing stack by rule 1 (line 28):
+-> $$ = nterm address@hidden (: )
+Stack now 0
+Entering state 2
+Reducing stack by rule 2 (line 29):
+-> $$ = nterm @2 (: 2)
+Stack now 0 2
+Entering state 4
+Reducing stack by rule 3 (line 30):
+-> $$ = nterm @3 (: 3)
+Stack now 0 2 4
+Entering state 5
+Reducing stack by rule 4 (line 31):
+-> $$ = nterm @4 (: 4)
+Stack now 0 2 4 5
+Entering state 6
+Reading a token: Now at end of input.
+syntax error
+Error: popping nterm @4 (: 4)
+DESTROY 4
+Stack now 0 2 4 5
+Error: popping nterm @3 (: 3)
+DESTROY 3
+Stack now 0 2 4
+Error: popping nterm @2 (: 2)
+DESTROY 2
+Stack now 0 2
+Error: popping nterm address@hidden (: )
+Stack now 0
+Cleanup: discarding lookahead token $end (: )
+Stack now 0
+]])
+
+AT_CLEANUP
Index: tests/regression.at
===================================================================
RCS file: /sources/bison/bison/tests/regression.at,v
retrieving revision 1.111
diff -p -u -r1.111 regression.at
--- tests/regression.at 15 Oct 2006 00:02:21 -0000      1.111
+++ tests/regression.at 21 Oct 2006 04:50:33 -0000
@@ -252,13 +252,13 @@ AT_CHECK([cat input.output], [],
 
     0 $accept: expr $end
 
-    1 @1: /* empty */
+    1 address@hidden: /* empty */
 
-    2 expr: 'a' @1 'b'
+    2 expr: 'a' address@hidden 'b'
 
-    3 @2: /* empty */
+    3 address@hidden: /* empty */
 
-    4 expr: @2 'c'
+    4 expr: address@hidden 'c'
 
 
 Terminals, with rules where they appear
@@ -276,9 +276,9 @@ $accept (6)
     on left: 0
 expr (7)
     on left: 2 4, on right: 0
address@hidden (8)
address@hidden (8)
     on left: 1, on right: 2
address@hidden (9)
address@hidden (9)
     on left: 3, on right: 4
 
 
@@ -288,19 +288,19 @@ state 0
 
     'a'  shift, and go to state 1
 
-    $default  reduce using rule 3 (@2)
+    $default  reduce using rule 3 (address@hidden)
 
     expr  go to state 2
-    @2    go to state 3
+    address@hidden   go to state 3
 
 
 state 1
 
-    2 expr: 'a' . @1 'b'
+    2 expr: 'a' . address@hidden 'b'
 
-    $default  reduce using rule 1 (@1)
+    $default  reduce using rule 1 (address@hidden)
 
-    @1  go to state 4
+    address@hidden  go to state 4
 
 
 state 2
@@ -312,14 +312,14 @@ state 2
 
 state 3
 
-    4 expr: @2 . 'c'
+    4 expr: address@hidden . 'c'
 
     'c'  shift, and go to state 6
 
 
 state 4
 
-    2 expr: 'a' @1 . 'b'
+    2 expr: 'a' address@hidden . 'b'
 
     'b'  shift, and go to state 7
 
@@ -333,14 +333,14 @@ state 5
 
 state 6
 
-    4 expr: @2 'c' .
+    4 expr: address@hidden 'c' .
 
     $default  reduce using rule 4 (expr)
 
 
 state 7
 
-    2 expr: 'a' @1 'b' .
+    2 expr: 'a' address@hidden 'b' .
 
     $default  reduce using rule 2 (expr)
 ]])
@@ -553,9 +553,9 @@ AT_CHECK([cat input.output], 0,
     2 CONST_DEC_LIST: CONST_DEC
     3               | CONST_DEC_LIST CONST_DEC
 
-    4 @1: /* empty */
+    4 address@hidden: /* empty */
 
-    5 CONST_DEC: @1 undef_id_tok '=' const_id_tok ';'
+    5 CONST_DEC: address@hidden undef_id_tok '=' const_id_tok ';'
 
 
 Terminals, with rules where they appear
@@ -578,7 +578,7 @@ CONST_DEC_LIST (9)
     on left: 2 3, on right: 1 3
 CONST_DEC (10)
     on left: 5, on right: 2 3
address@hidden (11)
address@hidden (11)
     on left: 4, on right: 5
 
 
@@ -586,12 +586,12 @@ state 0
 
     0 $accept: . CONST_DEC_PART $end
 
-    $default  reduce using rule 4 (@1)
+    $default  reduce using rule 4 (address@hidden)
 
     CONST_DEC_PART  go to state 1
     CONST_DEC_LIST  go to state 2
     CONST_DEC       go to state 3
-    @1              go to state 4
+    address@hidden             go to state 4
 
 
 state 1
@@ -606,11 +606,11 @@ state 2
     1 CONST_DEC_PART: CONST_DEC_LIST .
     3 CONST_DEC_LIST: CONST_DEC_LIST . CONST_DEC
 
-    undef_id_tok  reduce using rule 4 (@1)
+    undef_id_tok  reduce using rule 4 (address@hidden)
     $default      reduce using rule 1 (CONST_DEC_PART)
 
     CONST_DEC  go to state 6
-    @1         go to state 4
+    address@hidden        go to state 4
 
 
 state 3
@@ -622,7 +622,7 @@ state 3
 
 state 4
 
-    5 CONST_DEC: @1 . undef_id_tok '=' const_id_tok ';'
+    5 CONST_DEC: address@hidden . undef_id_tok '=' const_id_tok ';'
 
     undef_id_tok  shift, and go to state 7
 
@@ -643,28 +643,28 @@ state 6
 
 state 7
 
-    5 CONST_DEC: @1 undef_id_tok . '=' const_id_tok ';'
+    5 CONST_DEC: address@hidden undef_id_tok . '=' const_id_tok ';'
 
     '='  shift, and go to state 8
 
 
 state 8
 
-    5 CONST_DEC: @1 undef_id_tok '=' . const_id_tok ';'
+    5 CONST_DEC: address@hidden undef_id_tok '=' . const_id_tok ';'
 
     const_id_tok  shift, and go to state 9
 
 
 state 9
 
-    5 CONST_DEC: @1 undef_id_tok '=' const_id_tok . ';'
+    5 CONST_DEC: address@hidden undef_id_tok '=' const_id_tok . ';'
 
     ';'  shift, and go to state 10
 
 
 state 10
 
-    5 CONST_DEC: @1 undef_id_tok '=' const_id_tok ';' .
+    5 CONST_DEC: address@hidden undef_id_tok '=' const_id_tok ';' .
 
     $default  reduce using rule 5 (CONST_DEC)
 ]])