Re: remove YYFAIL from lalr1.java and yacc.c

[Joel E. Denny]

On Mon, 21 Dec 2009, Paolo Bonzini wrote:

> 
> > > , and I think that
> > > breaking YYFAIL now for %error-verbose is fine too.
> > 
> > Google code search turns up an instance where both are used:
> > 
> >    
> > http://www.google.com/codesearch?hl=en&lr=&q=%25error-verbose+YYFAIL+file%3A\.y&sbtn=Search
>
> Hmm, right.  They're not relying on the semantics, though and they can simply
> fix it by removing the "|| defined (YYFAIL)".  So it's effectively a false
> positive of the warning.  Well, let's live with it.

You're right.  This is a case where the warning will merely annoy.  The 
goal of their use of YYFAIL is to suppress a compiler warning (from 
-Wunused-macros, I assume).  Unfortunately, they put that YYFAIL in a 
semantic action, so it would trigger my proposed Bison warning.  If they 
move it to the epilogue, then they should be fine.  However, I bet there 
are many users doing this, so we might as well make the migration easier 
on them.  I've added some code to yacc.c to suppress warnings about unused 
YYFAIL.

I couldn't find any more uses of YYFAIL with %error-verbose, but then I 
remembered that `#define YYERROR_VERBOSE' is the same as %error-verbose.  
I count 2 legitimate hits other than GNU Smalltalk:

  
http://www.google.com/codesearch?hl=en&lr=&q=YYFAIL+YYERROR_VERBOSE+file%3A\.y%24&sbtn=Search

So for now, I don't think there's any case for which we can remove YYFAIL 
from yacc.c.

> > I agree we may have to keep it for a long time, but hopefully the warning
> > will prompt users to explore at least why YYFAIL is deprecated.  Maybe I
> > should make the warning even stronger by saying that YYFAIL will
> > eventually be removed.
> 
> Fine, even if both of us know how likely that is to happen. :-)

:-)

Well, we can't help it if users who abuse the internals of Bison ignore 
our warnings to stop.  Then again, I wonder if YYFAIL was ever documented 
for yacc.c.

I realize now that scan-gram.l is not the best place to detect uses of 
YYFAIL.  It doesn't distinguish among the various types of braced code.  
I've moved the warning to scan-code.l where it can be sure to warn about 
YYFAIL only in rule actions.

I've also removed YYFAIL from lalr1.java as of Bison 2.5.  As much as 
possible, I'd like to avoid backward compatibility issues in 2.4.2 even 
for experimental features.

Here are revised versions of my proposed patches.  Again, the first would 
be for 2.4.2 and later, and the second would be for 2.5 and later.  Any 
further discussion is welcome.

>From f5b061b946c19495a9972f106cf977cda9b6d66d Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Mon, 21 Dec 2009 14:51:40 -0500
Subject: [PATCH] YYFAIL: deprecate.

* NEWS (2.4.2): Document deprecation and the phase-out plan.
* data/lalr1.java (parser::YYStack::YYFAIL): Add comment about
deprecation.
* data/yacc.c (YYFAIL): Likewise, and suppress warnings about
YYFAIL from GCC cpp's -Wunused-macros.
* doc/bison.texinfo (Java Action Features): Remove YYFAIL
documentation.
(LocalWords): Remove YYFAIL.
---
 ChangeLog         |   12 +
 NEWS              |   39 +++
 data/lalr1.java   |    3 +-
 data/yacc.c       |   11 +-
 doc/bison.texinfo |    7 +-
 src/parse-gram.c  |  681 +++++++++++++++++++++++++++--------------------------
 src/parse-gram.h  |   12 +-
 7 files changed, 415 insertions(+), 350 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1c15cc5..2f1c556 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-12-21  Joel E. Denny  <address@hidden>
+
+       YYFAIL: deprecate.
+       * NEWS (2.4.2): Document deprecation and the phase-out plan.
+       * data/lalr1.java (parser::YYStack::YYFAIL): Add comment about
+       deprecation.
+       * data/yacc.c (YYFAIL): Likewise, and suppress warnings about
+       YYFAIL from GCC cpp's -Wunused-macros.
+       * doc/bison.texinfo (Java Action Features): Remove YYFAIL
+       documentation.
+       (LocalWords): Remove YYFAIL.
+
 2009-12-18  Joel E. Denny  <address@hidden>
 
        lalr1.cc: don't discard non-existent lookahead on syntax error.
diff --git a/NEWS b/NEWS
index 92567dd..5bd64cc 100644
--- a/NEWS
+++ b/NEWS
@@ -243,6 +243,45 @@ Bison News
   Bison's Java feature as a whole including its current usage of %code
   is still considered experimental.
 
+** YYFAIL is deprecated and will eventually be removed.
+
+  YYFAIL has existed for many years as an undocumented feature of
+  deterministic parsers in C generated by Bison.  Previously, it was
+  documented for Bison's experimental Java parsers.  YYFAIL is no longer
+  documented for Java parsers and is formally deprecated in both cases.
+  Users are strongly encouraged to migrate to YYERROR, which is
+  specified by POSIX.
+
+  Like YYERROR, you can invoke YYFAIL from a semantic action in order to
+  induce a syntax error.  The most obvious difference from YYERROR is
+  that YYFAIL will automatically invoke yyerror to report the syntax
+  error so that you don't have to.  However, there are several other
+  subtle differences between YYERROR and YYFAIL, and YYFAIL suffers from
+  inherent flaws when %error-verbose or `#define YYERROR_VERBOSE' is
+  used.  For a more detailed discussion, see:
+
+    http://lists.gnu.org/archive/html/bison-patches/2009-12/msg00024.html
+
+  The upcoming Bison 2.5 will remove YYFAIL from Java parsers, but
+  deterministic parsers in C will continue to implement it.  However,
+  because YYFAIL is already flawed, it seems futile to try to make new
+  Bison features compatible with it.  Thus, during parser generation,
+  Bison 2.5 will produce a warning whenever it discovers YYFAIL in a
+  rule action.  In a later release, YYFAIL will be disabled for
+  %error-verbose and `#define YYERROR_VERBOSE'.  Eventually, YYFAIL will
+  be removed altogether.
+
+  There exists at least one case where Bison 2.5's YYFAIL warning will
+  be a false positive.  Some projects add phony uses of YYFAIL and other
+  Bison-defined macros for the sole purpose of suppressing C
+  preprocessor warnings (from GCC cpp's -Wunused-macros, for example).
+  To avoid Bison's future warning, such YYFAIL uses can be moved to the
+  epilogue (that is, after the second `%%') in the Bison input file.  In
+  this release (2.4.2), Bison already generates its own code to suppress
+  C preprocessor warnings for YYFAIL, so projects can remove their own
+  phony uses of YYFAIL if compatibility with Bison releases prior to
+  2.4.2 is not necessary.
+
 ** Internationalization.
 
   Fix a regression introduced in Bison 2.4: Under some circumstances,
diff --git a/data/lalr1.java b/data/lalr1.java
index 6637c3a..a9ae873 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -352,7 +352,8 @@ b4_lexer_if([[
 
   /**
    * Returned by a Bison action in order to print an error message and start
-   * error recovery.  */
+   * error recovery.  Formally deprecated in Bison 2.4.2's NEWS entry, where
+   * a plan to phase it out is discussed.  */
   public static final int YYFAIL = 3;
 
   private static final int YYNEWSTATE = 4;
diff --git a/data/yacc.c b/data/yacc.c
index 11ebbf1..5e7b55f 100644
--- a/data/yacc.c
+++ b/data/yacc.c
@@ -609,9 +609,18 @@ static const ]b4_int_type_for([b4_toknum])[ yytoknum[] =
 
 /* Like YYERROR except do call yyerror.  This remains here temporarily
    to ease the transition to the new meaning of YYERROR, for GCC.
-   Once GCC version 2 has supplanted version 1, this can go.  */
+   Once GCC version 2 has supplanted version 1, this can go.  However,
+   YYFAIL appears to be in use.  Nevertheless, it is formally deprecated
+   in Bison 2.4.2's NEWS entry, where a plan to phase it out is
+   discussed.  */
 
 #define YYFAIL         goto yyerrlab
+#if defined YYFAIL
+  /* This is here to suppress warnings from the GCC cpp's
+     -Wunused-macros.  Normally we don't worry about that warning, but
+     some users do, and we want to make it easy for users to remove
+     YYFAIL uses, which will produce warnings from Bison 2.5.  */
+#endif
 
 #define YYRECOVERING()  (!!yyerrstatus)
 
diff --git a/doc/bison.texinfo b/doc/bison.texinfo
index 2af8917..ab2cd6c 100644
--- a/doc/bison.texinfo
+++ b/doc/bison.texinfo
@@ -9925,11 +9925,6 @@ Start error recovery without printing an error message.
 @xref{Error Recovery}.
 @end deffn
 
address@hidden {Statement} {return YYFAIL;}
-Print an error message and start error recovery.
address@hidden Recovery}.
address@hidden deffn
-
 @deftypefn {Function} {boolean} recovering ()
 Return whether error recovery is being done. In this state, the parser
 reads token until it reaches a known state, and then restarts normal
@@ -11330,5 +11325,5 @@ grammatically indivisible.  The piece of text it 
represents is a token.
 @c LocalWords: superclasses boolean getErrorVerbose setErrorVerbose deftypecv
 @c LocalWords: getDebugStream setDebugStream getDebugLevel setDebugLevel url
 @c LocalWords: bisonVersion deftypecvx bisonSkeleton getStartPos getEndPos
address@hidden LocalWords: getLVal defvar YYFAIL deftypefn deftypefnx gotos 
msgfmt
address@hidden LocalWords: getLVal defvar deftypefn deftypefnx gotos msgfmt
 @c LocalWords: subdirectory Solaris nonassociativity
-- 
1.5.4.3


>From b93731f1f6612f876f7f6e67fd523e82901c7884 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Mon, 21 Dec 2009 14:58:48 -0500
Subject: [PATCH] YYFAIL: warn about uses and remove from lalr1.java.

* NEWS (2.5): Document.
* data/lalr1.java (parser::YYStack::YYFAIL): Rename to YYERRLAB,
and make it private.  Update all uses.
* src/scan-code.l (SC_RULE_ACTION): Implement warning.
---
 ChangeLog       |    8 ++++++++
 NEWS            |   11 +++++++++++
 data/lalr1.java |   15 ++++++---------
 src/scan-code.l |   15 ++++++++++++++-
 4 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2f1c556..71a0c02 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-12-21  Joel E. Denny  <address@hidden>
 
+       YYFAIL: warn about uses and remove from lalr1.java.
+       * NEWS (2.5): Document.
+       * data/lalr1.java (parser::YYStack::YYFAIL): Rename to YYERRLAB,
+       and make it private.  Update all uses.
+       * src/scan-code.l (SC_RULE_ACTION): Implement warning.
+
+2009-12-21  Joel E. Denny  <address@hidden>
+
        YYFAIL: deprecate.
        * NEWS (2.4.2): Document deprecation and the phase-out plan.
        * data/lalr1.java (parser::YYStack::YYFAIL): Add comment about
diff --git a/NEWS b/NEWS
index 5bd64cc..b704ed5 100644
--- a/NEWS
+++ b/NEWS
@@ -169,6 +169,17 @@ Bison News
   POSIX Yacc whose use is reported by -Wyacc, and rejected in Yacc
   mode (--yacc).
 
+** YYFAIL now produces warnings and Java parsers no longer implement it.
+
+  YYFAIL has existed for many years as an undocumented feature of
+  deterministic parsers in C generated by Bison.  More recently, it was
+  a documented feature of Bison's experimental Java parsers.  As
+  promised in Bison 2.4.2's NEWS entry, any appearance of YYFAIL in a
+  semantic action now produces a deprecation warning, and Java parsers
+  no longer implement YYFAIL at all.  For further details, including a
+  discussion of how to suppress C preprocessor warnings about YYFAIL
+  being unused, see the Bison 2.4.2 NEWS entry.
+
 ** Temporary hack for adding a semicolon to the user action.
 
   Previously, Bison appended a semicolon to every user action for
diff --git a/data/lalr1.java b/data/lalr1.java
index a9ae873..275b97f 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -350,12 +350,9 @@ b4_lexer_if([[
    * printing an error message.  */
   public static final int YYERROR = 2;
 
-  /**
-   * Returned by a Bison action in order to print an error message and start
-   * error recovery.  Formally deprecated in Bison 2.4.2's NEWS entry, where
-   * a plan to phase it out is discussed.  */
-  public static final int YYFAIL = 3;
-
+  // Internal return codes that are not supported for user semantic
+  // actions.
+  private static final int YYERRLAB = 3;
   private static final int YYNEWSTATE = 4;
   private static final int YYDEFAULT = 5;
   private static final int YYREDUCE = 6;
@@ -574,7 +571,7 @@ m4_popdef([b4_at_dollar])])dnl
         else if ((yyn = yytable_[yyn]) <= 0)
           {
             if (yy_table_value_is_error_ (yyn))
-              label = YYFAIL;
+              label = YYERRLAB;
             else
               {
                 yyn = -yyn;
@@ -608,7 +605,7 @@ m4_popdef([b4_at_dollar])])dnl
       case YYDEFAULT:
         yyn = yydefact_[yystate];
         if (yyn == 0)
-          label = YYFAIL;
+          label = YYERRLAB;
         else
           label = YYREDUCE;
         break;
@@ -625,7 +622,7 @@ m4_popdef([b4_at_dollar])])dnl
       /*------------------------------------.
       | yyerrlab -- here on detecting error |
       `------------------------------------*/
-      case YYFAIL:
+      case YYERRLAB:
         /* If not already recovering from an error, report this error.  */
         if (yyerrstatus_ == 0)
           {
diff --git a/src/scan-code.l b/src/scan-code.l
index 56cd782..700488e 100644
--- a/src/scan-code.l
+++ b/src/scan-code.l
@@ -246,7 +246,20 @@ ref      -?[0-9]+|{id}|"["{id}"]"|"$"
   {splice}  STRING_GROW;
   [\n\r]    STRING_GROW; if (in_cpp) in_cpp = need_semicolon = false;
   [ \t\f]   STRING_GROW;
-  .         STRING_GROW; need_semicolon = true;
+
+  /* YYFAIL is undocumented and was formally deprecated in Bison
+     2.4.2.  */
+  YYFAIL {
+    STRING_GROW; need_semicolon = true;
+    warn_at (*loc, _("use of YYFAIL, which is deprecated and will be"
+                     " removed"));
+  }
+
+  /* The sole purpose of this is to make sure identifiers that merely
+     contain YYFAIL don't produce the above warning.  */
+  [A-Za-z_][0-9A-Za-z_]* STRING_GROW; need_semicolon = true;
+
+  . STRING_GROW; need_semicolon = true;
 }
 
 <SC_SYMBOL_ACTION>
-- 
1.5.4.3