Re: %destructor feedback

[Akim Demaille]

>>> "Paul" == Paul Eggert <address@hidden> writes:

 >> - we desire that in all the skeletons no $n is ever freed from an
 >> "active" rule, even if we leave it using YYACCEPT, YYERROR, or
 >> YYABORT.

 > Yes, that sounds right.

Well, AFAICS that's already the behavior in glr.c, isn't it?
Otherwise I missed the bit of code that is wrong.

I installed the following.

Index: ChangeLog
from  Akim Demaille  <address@hidden>

        YYACCEPT, YYERROR, and YYABORT, as user actions, should not
        destroy the RHS symbols of a rule.
        * data/yacc.c (yylen): Initialize to 0.
        Keep its value to the number of items to possibly shift.
        In particular, a regular successful parse that ends on YYFINAL by
        a (internal) YYACCEPT must not have yylen != 0.
        (yyerrorlab, yyreturn): Pop the RHS.
        Reorder a bit to emphasize the `shifting' bits of code.
        (YYPOPSTACK): Now accept a number of items to pop.
        * data/lalr1.cc: Likewise.
        * data/glr.c: Formatting changes.
        Use goto instead of fall through.
        * doc/bison.texinfo (Destructor Decl): Complete.

Index: NEWS
===================================================================
RCS file: /cvsroot/bison/bison/NEWS,v
retrieving revision 1.133
diff -u -u -r1.133 NEWS
--- NEWS 21 Dec 2005 13:49:44 -0000 1.133
+++ NEWS 21 Dec 2005 15:28:24 -0000
@@ -3,6 +3,11 @@
 
 Changes in version 2.1a:
 
+* %destructor vs. YYACCEPT, YYERROR, and YYABORT.
+  When the parsing/action is cut by the user using one of these
+  special actions, the stack is freed except the right-hand side
+  symbols of the current rule.
+
 * GLR, C++ LALR(1) parsers.
   These parser skeletons are now distributed with the same special
   license exception that the C LALR(1) parser skeleton has had since
Index: data/glr.c
===================================================================
RCS file: /cvsroot/bison/bison/data/glr.c,v
retrieving revision 1.144
diff -u -u -r1.144 glr.c
--- data/glr.c 12 Dec 2005 21:04:39 -0000 1.144
+++ data/glr.c 21 Dec 2005 15:28:24 -0000
@@ -909,10 +909,12 @@
     *yyvalp = yyvsp[YYFILL (1-yyrhslen)].yystate.yysemantics.yysval;
   YYLLOC_DEFAULT (*yylocp, yyvsp - yyrhslen, yyrhslen);
 ]b4_location_if([[  yystackp->yyerror_range[1].yystate.yyloc = *yylocp;
-]])
+]])[
   switch (yyn)
     {
-      b4_actions
+      ]b4_actions
+/* Line __line__ of glr.c.  */
+b4_syncline(address@hidden@], address@hidden@])[
       default: break;
     }
 
@@ -924,8 +926,6 @@
 # undef YYBACKUP
 # undef yyclearin
 # undef YYRECOVERING
-/* Line __line__ of glr.c.  */
-b4_syncline(address@hidden@], address@hidden@])
 }
 
 
@@ -937,11 +937,11 @@
 
   switch (yyn)
     {
-      b4_mergers
+      ]b4_mergers[
       default: break;
     }
 }
-[
+
                              /* Bison grammar-table manipulation.  */
 
 ]b4_yydestruct_generate([b4_c_ansi_function_def])[
@@ -2324,7 +2324,7 @@
 
  yybuglab:
   YYASSERT (yyfalse);
-  /* Fall through.  */
+  goto yyabortlab;
 
  yyabortlab:
   yyresult = 1;
@@ -2333,7 +2333,7 @@
  yyexhaustedlab:
   yyerror (]b4_lyyerror_args[YY_("memory exhausted"));
   yyresult = 2;
-  /* Fall through.  */
+  goto yyreturn;
 
  yyreturn:
   if (yytoken != YYEOF && yytoken != YYEMPTY)
Index: data/lalr1.cc
===================================================================
RCS file: /cvsroot/bison/bison/data/lalr1.cc,v
retrieving revision 1.114
diff -u -u -r1.114 lalr1.cc
--- data/lalr1.cc 9 Dec 2005 22:14:36 -0000 1.114
+++ data/lalr1.cc 21 Dec 2005 15:28:24 -0000
@@ -509,7 +509,7 @@
 
     /* State.  */
     int yyn;
-    int yylen;
+    int yylen = 0;
     int yystate = 0;
 
     /* Error handling. */
@@ -641,7 +641,7 @@
   `-----------------------------*/
   yyreduce:
     yylen = yyr2_[yyn];
-    /* If LEN_ is nonzero, implement the default value of the action:
+    /* If YYLEN is nonzero, implement the default value of the action:
        `$$ = $1'.  Otherwise, use the top of the stack.
 
        Otherwise, the following line sets YYVAL to garbage.
@@ -659,15 +659,15 @@
     YY_REDUCE_PRINT (yyn);
     switch (yyn)
       {
-        ]b4_actions[
+        ]b4_actions
+    /* Line __line__ of lalr1.cc.  */
+b4_syncline(address@hidden@], address@hidden@])[
         default: break;
       }
     YY_SYMBOL_PRINT ("-> $$ =", yyr1_[yyn], &yyval, &yyloc);
-    /* Line __line__ of lalr1.cc.  */
-]b4_syncline(address@hidden@], address@hidden@])[
 
     yypop_ (yylen);
-
+    yylen = 0;
     YY_STACK_PRINT ();
 
     yysemantic_stack_.push (yyval);
@@ -731,7 +731,10 @@
       goto yyerrorlab;
 
     yyerror_range[0] = yylocation_stack_[yylen - 1];
+    /* Do not reclaim the symbols of the rule which action triggered
+       this YYERROR. */
     yypop_ (yylen);
+    yylen = 0;
     yystate = yystate_stack_[0];
     goto yyerrlab1;
 
@@ -799,6 +802,9 @@
     if (yychar != yyeof_ && yychar != yyempty_)
       yydestruct_ ("Cleanup: discarding lookahead", yytoken, &yylval, &yylloc);
 
+    /* Do not reclaim the symbols of the rule which action triggered
+       this YYABORT or YYACCEPT. */
+    yypop_ (yylen);
     while (yystate_stack_.height () != 1)
       {
         yydestruct_ ("Cleanup: popping",
Index: data/yacc.c
===================================================================
RCS file: /cvsroot/bison/bison/data/yacc.c,v
retrieving revision 1.120
diff -u -u -r1.120 yacc.c
--- data/yacc.c 9 Dec 2005 22:14:36 -0000 1.120
+++ data/yacc.c 21 Dec 2005 15:28:24 -0000
@@ -564,7 +564,7 @@
       yychar = (Token);                                                \
       yylval = (Value);                                                \
       yytoken = YYTRANSLATE (yychar);                          \
-      YYPOPSTACK;                                              \
+      YYPOPSTACK (1);                                          \
       goto yybackup;                                           \
     }                                                          \
   else                                                         \
@@ -1042,7 +1042,7 @@
   /* The locations where the error started and ended. */
   YYLTYPE yyerror_range[2];]])[
 
-#define YYPOPSTACK   (yyvsp--, yyssp--]b4_location_if([, yylsp--])[)
+#define YYPOPSTACK(N)   (yyvsp -= (N), yyssp -= (N)]b4_location_if([, yylsp -= 
(N)])[)
 
   YYSIZE_T yystacksize = YYINITDEPTH;
 
@@ -1051,9 +1051,9 @@
   YYSTYPE yyval;
 ]b4_location_if([  YYLTYPE yyloc;])[
 
-  /* When reducing, the number of symbols on the RHS of the reduced
-     rule.  */
-  int yylen;
+  /* The number of symbols on the RHS of the reduced rule.
+     Keep to zero when no symbol should be popped off. */
+  int yylen = 0;
 
   YYDPRINTF ((stderr, "Starting parse\n"));
 
@@ -1178,12 +1178,10 @@
 `-----------*/
 yybackup:
 
-/* Do appropriate processing given the current state.  */
-/* Read a look-ahead token if we need one and don't already have one.  */
-/* yyresume: */
+  /* Do appropriate processing given the current state.  Read a
+     look-ahead token if we need one and don't already have one.  */
 
   /* First try to decide what to do without reference to look-ahead token.  */
-
   yyn = yypact[yystate];
   if (yyn == YYPACT_NINF)
     goto yydefault;
@@ -1225,22 +1223,21 @@
   if (yyn == YYFINAL)
     YYACCEPT;
 
+  /* Count tokens shifted since error; after three, turn off error
+     status.  */
+  if (yyerrstatus)
+    yyerrstatus--;
+
   /* Shift the look-ahead token.  */
   YY_SYMBOL_PRINT ("Shifting", yytoken, &yylval, &yylloc);
 
-  /* Discard the token being shifted unless it is eof.  */
+  /* Discard the shifted token unless it is eof.  */
   if (yychar != YYEOF)
     yychar = YYEMPTY;
 
+  yystate = yyn;
   *++yyvsp = yylval;
 ]b4_location_if([  *++yylsp = yylloc;])[
-
-  /* Count tokens shifted since error; after three, turn off error
-     status.  */
-  if (yyerrstatus)
-    yyerrstatus--;
-
-  yystate = yyn;
   goto yynewstate;
 
 
@@ -1277,18 +1274,15 @@
   YY_REDUCE_PRINT (yyn);
   switch (yyn)
     {
-      ]b4_actions[
+      ]b4_actions
+/* Line __line__ of yacc.c.  */
+b4_syncline(address@hidden@], address@hidden@])[
       default: break;
     }
   YY_SYMBOL_PRINT ("-> $$ =", yyr1[yyn], &yyval, &yyloc);
 
-/* Line __line__ of yacc.c.  */
-]b4_syncline(address@hidden@], address@hidden@])[
-
-  yyvsp -= yylen;
-  yyssp -= yylen;
-]b4_location_if([  yylsp -= yylen;])[
-
+  YYPOPSTACK (yylen);
+  yylen = 0;
   YY_STACK_PRINT (yyss, yyssp);
 
   *++yyvsp = yyval;
@@ -1392,9 +1386,11 @@
      goto yyerrorlab;
 
 ]b4_location_if([[  yyerror_range[0] = yylsp[1-yylen];
-  yylsp -= yylen;
-  ]])[yyvsp -= yylen;
-  yyssp -= yylen;
+]])[  /* Do not reclaim the symbols of the rule which action triggered
+     this YYERROR. */
+  YYPOPSTACK (yylen);
+  yylen = 0;
+  YY_STACK_PRINT (yyss, yyssp);
   yystate = *yyssp;
   goto yyerrlab1;
 
@@ -1426,7 +1422,7 @@
 ]b4_location_if([[      yyerror_range[0] = *yylsp;]])[
       yydestruct ("Error: popping",
                   yystos[yystate], yyvsp]b4_location_if([, 
yylsp])[]b4_user_args[);
-      YYPOPSTACK;
+      YYPOPSTACK (1);
       yystate = *yyssp;
       YY_STACK_PRINT (yyss, yyssp);
     }
@@ -1477,11 +1473,15 @@
   if (yychar != YYEOF && yychar != YYEMPTY)
      yydestruct ("Cleanup: discarding lookahead",
                 yytoken, &yylval]b4_location_if([, &yylloc])[]b4_user_args[);
+  /* Do not reclaim the symbols of the rule which action triggered
+     this YYABORT or YYACCEPT. */
+  YYPOPSTACK (yylen);
+  YY_STACK_PRINT (yyss, yyssp);
   while (yyssp != yyss)
     {
       yydestruct ("Cleanup: popping",
                  yystos[*yyssp], yyvsp]b4_location_if([, 
yylsp])[]b4_user_args[);
-      YYPOPSTACK;
+      YYPOPSTACK (1);
     }
 #ifndef yyoverflow
   if (yyss != yyssa)
Index: doc/bison.texinfo
===================================================================
RCS file: /cvsroot/bison/bison/doc/bison.texinfo,v
retrieving revision 1.167
diff -u -u -r1.167 bison.texinfo
--- doc/bison.texinfo 18 Nov 2005 18:16:43 -0000 1.167
+++ doc/bison.texinfo 21 Dec 2005 15:28:25 -0000
@@ -3810,20 +3810,20 @@
 @cindex freeing discarded symbols
 @findex %destructor
 
-Some symbols can be discarded by the parser.  During error recovery
-(@pxref{Error Recovery}), symbols already pushed on the stack and tokens
-coming from the rest of the file are discarded until the parser falls on
-its feet.  If the parser runs out of memory, all the symbols on the
-stack must be discarded.  Even if the parser succeeds, it must discard
-the start symbol.
+During error recovery (@pxref{Error Recovery}), symbols already pushed
+on the stack and tokens coming from the rest of the file are discarded
+until the parser falls on its feet.  If the parser runs out of memory,
+or if the parsing is cut by @code{YYACCEPT} or @code{YYABORT}, all the
+symbols on the stack must be discarded.  Even if the parser succeeds, it
+must discard the start symbol.
 
 When discarded symbols convey heap based information, this memory is
 lost.  While this behavior can be tolerable for batch parsers, such as
 in traditional compilers, it is unacceptable for programs like shells or
 protocol implementations that may parse and execute indefinitely.
 
-The @code{%destructor} directive defines code that
-is called when a symbol is discarded.
+The @code{%destructor} directive defines code that is called when a
+symbol is automatically discarded.
 
 @deffn {Directive} %destructor @{ @var{code} @} @var{symbols}
 @findex %destructor
@@ -3832,10 +3832,6 @@
 with the discarded symbol.  The additional parser parameters are also
 available (@pxref{Parser Function, , The Parser Function
 @code{yyparse}}).
-
address@hidden:} as of Bison 2.1, this feature is still
-experimental, as there has not been enough user feedback.  In particular,
-the syntax might still change.
 @end deffn
 
 For instance:
@@ -3854,24 +3850,6 @@
 guarantees that when a @code{STRING} or a @code{string} is discarded,
 its associated memory will be freed.
 
-Note that in the future, Bison might also consider that right hand side
-members that are not mentioned in the action can be destroyed.  For
-instance, in:
-
address@hidden
-comment: "/*" STRING "*/";
address@hidden smallexample
-
address@hidden
-the parser is entitled to destroy the semantic value of the
address@hidden  Of course, this will not apply to the default action;
-compare:
-
address@hidden
-typeless: string;  // $$ = $1 does not apply; $1 is destroyed.
-typefull: string;  // $$ = $1 applies, $1 is not destroyed.
address@hidden smallexample
-
 @sp 1
 
 @cindex discarded symbols
@@ -3883,13 +3861,18 @@
 @item
 incoming terminals during the second phase of error recovery,
 @item
-the current look-ahead and the entire stack when the parser aborts
-(either via an explicit call to @code{YYABORT}, or as a consequence of
-a failed error recovery or of memory exhaustion), and
+the current look-ahead and the entire stack (except the current
+right-hand side symbols) when the parser aborts (either via an explicit
+call to @code{YYABORT} or @code{YYACCEPT}, or as a consequence of a
+failed error recovery or of memory exhaustion), and
 @item
 the start symbol, when the parser succeeds.
 @end itemize
 
+Note that right-hand size symbols of a rule that explicitly triggers a
+syntax error via @code{YYERROR} are not reclaimed.  As a thumb rule,
+destructors are invoked only when you do not have other means to manage
+the memory.
 
 @node Expect Decl
 @subsection Suppressing Conflict Warnings