Re: Reductions during Bison error handling

[Paul Eggert]

> From: Paul Hilfinger <address@hidden>
> Date: Tue, 21 May 2002 20:18:39 -0700
> 
> Or, if you prefer, we can take the opportunity to clean things up in
> this part of the skeleton, giving, perhaps, the following:

I like the latter idea.  I have the following suggestions for minor
improvements.  First, let's keep that redundant 'goto yyerrlab1;', as
it is in keeping with the style of that code (as a state machine).
There are several redundant gotos but I think they're all OK.

Second, we should use the GNU coding style for indenting expressions,
which means we should put && at the start of the next line, not at the
end of the previous line.

Third, all other things being equal I prefer "<" to ">" since it
causes textual order to reflect numeric order.  This is a coding style
trick I learned from D. Val Schorre long ago.  (Knuth credits Schorre
with the first use of gotoless programming, so he had some other good
ideas too.)

Last, and perhaps most controversially, in this case it's a bit clearer
to put the test in the while-expression rather than to use while(1) ...
if (test) break;.

Here's a revised (but untested) patch that reflects these suggestions.


2002-05-22  Paul Hilfinger  <address@hidden>

        * data/bison.simple (yyparse): Correct error handling to conform to
        POSIX and yacc.  Specifically, after syntax error is discovered,
        do not reduce further before shifting the error token.
        Clean up the code a bit by removing the labels yyerrdefault,
        yyerrhandle, yyerrpop.

--- bison.simple        2002-05-20 15:56:53.000000000 -0700
+++ /tmp/bison.simple.new       2002-05-22 17:43:17.728818443 -0700
@@ -1120,73 +1120,34 @@
 
   yyerrstatus = 3;     /* Each real token shifted decrements this.  */
 
-  goto yyerrhandle;
-
-
-/*-------------------------------------------------------------------.
-| yyerrdefault -- current state does not do anything special for the |
-| error token.                                                       |
-`-------------------------------------------------------------------*/
-yyerrdefault:
-#if 0
-  /* This is wrong; only states that explicitly want error tokens
-     should shift them.  */
-
-  /* If its default is to accept any token, ok.  Otherwise pop it.  */
-  yyn = yydefact[yystate];
-  if (yyn)
-    goto yydefault;
-#endif
-
-
-/*---------------------------------------------------------------.
-| yyerrpop -- pop the current state because it cannot handle the |
-| error token.                                                   |
-`---------------------------------------------------------------*/
-yyerrpop:
-  if (yyssp == yyss)
-    YYABORT;
-  yyvsp--;
-  yystate = *--yyssp;
+   while (yyn = yypact[yystate] + YYTERROR,
+         ! (yyn != YYFLAG + YYTERROR
+            && 0 <= yyn && yyn <= YYLAST && yycheck[yyn] == YYTERROR
+            && 0 < yytable[yyn]))
+    {
+      /* Pop the parsing stack.  */
+      if (yyssp == yyss)
+       YYABORT;
+      yyvsp--;
+      yystate = *--yyssp;
 #if YYLSP_NEEDED
-  yylsp--;
+      yylsp--;
 #endif
 
 #if YYDEBUG
-  if (yydebug)
-    {
-      short *yyssp1 = yyss - 1;
-      YYFPRINTF (stderr, "Error: state stack now");
-      while (yyssp1 != yyssp)
-       YYFPRINTF (stderr, " %d", *++yyssp1);
-      YYFPRINTF (stderr, "\n");
-    }
+      if (yydebug)
+       {
+         short *yyssp1 = yyss - 1;
+         YYFPRINTF (stderr, "Error: state stack now");
+         while (yyssp1 != yyssp)
+           YYFPRINTF (stderr, " %d", *++yyssp1);
+         YYFPRINTF (stderr, "\n");
+       }
 #endif
-
-/*--------------.
-| yyerrhandle.  |
-`--------------*/
-yyerrhandle:
-  yyn = yypact[yystate];
-  if (yyn == YYFLAG)
-    goto yyerrdefault;
-
-  yyn += YYTERROR;
-  if (yyn < 0 || yyn > YYLAST || yycheck[yyn] != YYTERROR)
-    goto yyerrdefault;
-
-  yyn = yytable[yyn];
-  if (yyn < 0)
-    {
-      if (yyn == YYFLAG)
-       goto yyerrpop;
-      yyn = -yyn;
-      goto yyreduce;
     }
-  else if (yyn == 0)
-    goto yyerrpop;
 
-  if (yyn == YYFINAL)
+  yystate = yytable[yyn];
+  if (yystate == YYFINAL)
     YYACCEPT;
 
   YYDPRINTF ((stderr, "Shifting error token, "));
@@ -1196,7 +1157,6 @@
   *++yylsp = yylloc;
 #endif
 
-  yystate = yyn;
   goto yynewstate;