Re: Bug in Location code of java skeleton

[Joel E. Denny]

On Mon, 14 Feb 2011, Bernd Kiefer wrote:

> i attach a patch that seems to fix an error concerning locations in
> the java version of the lalr1 parser. I changed this in a generated
> file to test it, and it now delivers always the right results when
> using debugging output during parsing, in contrast to the wrong
> locations i got with the 2.4.3 version.

Thanks for the patch.  I've pushed it in your name to our branch-2.5 and 
master branches.  The "tiny change" annotation in the ChangeLog just 
indicates that we don't have to ask you to sign copyright papers.  It does 
not reflect the significance of your contribution.

I also pushed a patch to extend the test suite and to mention the fix in 
NEWS.

Both patches follow.

>From 8db68289d1162b763606fe5271a7529408224d38 Mon Sep 17 00:00:00 2001
From: Bernd Kiefer <address@hidden>
Date: Sat, 19 Feb 2011 19:24:07 -0500
Subject: [PATCH 1/2] java: fix location handling bug.

Reported at
<http://lists.gnu.org/archive/html/bison-patches/2011-02/msg00005.html>.
* data/lalr1.java (YYParser::yylloc): For non-empty RHS, fix
reversed access to location stack.
* THANKS (Bernd Kiefer): Add.
---
 ChangeLog       |    9 +++++++++
 THANKS          |    1 +
 data/lalr1.java |    2 +-
 3 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 51deb00..df1d6ec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-02-19  Bernd Kiefer  <address@hidden>  (tiny change)
+
+       java: fix location handling bug.
+       Reported at
+       <http://lists.gnu.org/archive/html/bison-patches/2011-02/msg00005.html>.
+       * data/lalr1.java (YYParser::yylloc): For non-empty RHS, fix
+       reversed access to location stack.
+       * THANKS (Bernd Kiefer): Add.
+
 2010-05-11  Akim Demaille  <address@hidden>
 
        doc: please Emacs.
diff --git a/THANKS b/THANKS
index e4354d7..43acbed 100644
--- a/THANKS
+++ b/THANKS
@@ -13,6 +13,7 @@ Arnold Robbins            address@hidden
 Art Haas                  address@hidden
 Baron Schwartz            address@hidden
 Benoit Perrot             address@hidden
+Bernd Kiefer              address@hidden
 Bert Deknuydt             address@hidden
 Bill Allombert            address@hidden
 Bob Rossi                 address@hidden
diff --git a/data/lalr1.java b/data/lalr1.java
index 1533e37..aad54a3 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -102,7 +102,7 @@ b4_token_enums(b4_tokens)
   private ]b4_location_type[ yylloc (YYStack rhs, int n)
   {
     if (n > 0)
-      return new ]b4_location_type[ (rhs.locationAt (1).begin, rhs.locationAt 
(n).end);
+      return new ]b4_location_type[ (rhs.locationAt (n-1).begin, 
rhs.locationAt (0).end);
     else
       return new ]b4_location_type[ (rhs.locationAt (0).end);
   }]])[
-- 
1.7.0.4


>From 7776816565040879b3b095130a772c06585daeeb Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sat, 19 Feb 2011 19:36:33 -0500
Subject: [PATCH 2/2] java: test and document previous bug fix.

* NEWS (2.5): Document it.
* tests/java.at (_AT_DATA_JAVA_CALC_Y): To one of the yyerror
invocations, pass a location that spans multiple tokens.  Change
yyerror to report all of a location rather than just the begin
position.  Extend yylex and Position to count tokens on a line.
Remove getHashCode as it's unused.  Update all expected output.
---
 ChangeLog     |   10 ++++++++
 NEWS          |    2 +
 tests/java.at |   67 +++++++++++++++++++++++++++++---------------------------
 3 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index df1d6ec..90d9a08 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-02-19  Joel E. Denny  <address@hidden>
+
+       java: test and document previous bug fix.
+       * NEWS (2.5): Document it.
+       * tests/java.at (_AT_DATA_JAVA_CALC_Y): To one of the yyerror
+       invocations, pass a location that spans multiple tokens.  Change
+       yyerror to report all of a location rather than just the begin
+       position.  Extend yylex and Position to count tokens on a line.
+       Remove getHashCode as it's unused.  Update all expected output.
+
 2011-02-19  Bernd Kiefer  <address@hidden>  (tiny change)
 
        java: fix location handling bug.
diff --git a/NEWS b/NEWS
index 88ec95c..37f0b6f 100644
--- a/NEWS
+++ b/NEWS
@@ -291,6 +291,8 @@ Bison News
     canonical LR.  However, LAC is still experimental and is disabled
     by default.
 
+** A location handling bug in the Java skeleton has been fixed.
+
 * Changes in version 2.4.3 (2010-08-05):
 
 ** Bison now obeys -Werror and --warnings=error for warnings about
diff --git a/tests/java.at b/tests/java.at
index 9b31f9c..49a716a 100644
--- a/tests/java.at
+++ b/tests/java.at
@@ -77,7 +77,7 @@ exp:
 | exp '=' exp
   {
     if ($1.intValue () != $3.intValue ())
-      yyerror ("calc: error: " + $1 + " != " + $3);
+      yyerror (]AT_LOCATION_IF(address@hidden,]])[ "calc: error: " + $1 + " != 
" + $3);
   }
 | exp '+' exp        { $$ = new Integer ($1.intValue () + $3.intValue ());  }
 | exp '-' exp        { $$ = new Integer ($1.intValue () - $3.intValue ());  }
@@ -113,15 +113,14 @@ class CalcLexer implements Calc.Lexer {
   }
 
 AT_LOCATION_IF([[
-  Position yystartpos;
-  Position yyendpos = new Position (1);
+  Position yypos = new Position (1, 0);
 
   public Position getStartPos() {
-    return yystartpos;
+    return yypos;
   }
 
   public Position getEndPos() {
-    return yyendpos;
+    return yypos;
   }
 
   public void yyerror (Calc.Location l, String s)
@@ -129,7 +128,7 @@ AT_LOCATION_IF([[
     if (l == null)
       System.err.println (s);
     else
-      System.err.println (l.begin + ": " + s);
+      System.err.println (l + ": " + s);
   }
 ]], [[
   public void yyerror (String s)
@@ -146,13 +145,14 @@ AT_LOCATION_IF([[
 
   public int yylex () throws IOException {
     int ttype = st.nextToken ();
-    ]AT_LOCATION_IF([[yystartpos = yyendpos;]])[
+    ]AT_LOCATION_IF([[yypos = new Position (yypos.lineno (),
+                                            yypos.token () + 1);]])[
     if (ttype == st.TT_EOF)
       return Calc.EOF;
 
     else if (ttype == st.TT_EOL)
       {
-        ]AT_LOCATION_IF([[yyendpos = new Position (yyendpos.lineno () + 1);]])[
+        ]AT_LOCATION_IF([[yypos = new Position (yypos.lineno () + 1, 0);]])[
         return (int) '\n';
       }
 
@@ -175,36 +175,39 @@ AT_LOCATION_IF([[
 [
 class Position {
   public int line;
+  public int token;
 
   public Position ()
   {
     line = 0;
+    token = 0;
   }
 
-  public Position (int l)
+  public Position (int l, int t)
   {
     line = l;
-  }
-
-  public long getHashCode ()
-  {
-    return line;
+    token = t;
   }
 
   public boolean equals (Position l)
   {
-    return l.line == line;
+    return l.line == line && l.token == token;
   }
 
   public String toString ()
   {
-    return Integer.toString (line);
+    return Integer.toString (line) + "." + Integer.toString(token);
   }
 
   public int lineno ()
   {
     return line;
   }
+
+  public int token ()
+  {
+    return token;
+  }
 }
 
 ]])
@@ -294,19 +297,19 @@ AT_JAVA_PARSER_CHECK([Calc < input], 0, [], [stderr])
 
 # Some syntax errors.
 _AT_CHECK_JAVA_CALC_ERROR([$1], [0 0],
-                          [1: syntax error, unexpected number])
+                          [1.2: syntax error, unexpected number])
 _AT_CHECK_JAVA_CALC_ERROR([$1], [1//2],
-                          [1: syntax error, unexpected '/', expecting number 
or '-' or '(' or '!'])
+                          [1.3: syntax error, unexpected '/', expecting number 
or '-' or '(' or '!'])
 _AT_CHECK_JAVA_CALC_ERROR([$1], [error],
-                          [1: syntax error, unexpected $undefined])
+                          [1.1: syntax error, unexpected $undefined])
 _AT_CHECK_JAVA_CALC_ERROR([$1], [1 = 2 = 3],
-                          [1: syntax error, unexpected '='])
+                          [1.4: syntax error, unexpected '='])
 _AT_CHECK_JAVA_CALC_ERROR([$1], [
 +1],
-                          [2: syntax error, unexpected '+'])
+                          [2.1: syntax error, unexpected '+'])
 # Exercise error messages with EOF: work on an empty file.
 _AT_CHECK_JAVA_CALC_ERROR([$1], [/dev/null],
-                          [1: syntax error, unexpected end of input])
+                          [1.1: syntax error, unexpected end of input])
 
 # Exercise the error token: without it, we die at the first error,
 # hence be sure to
@@ -326,21 +329,21 @@ _AT_CHECK_JAVA_CALC_ERROR([$1], [/dev/null],
 #
 _AT_CHECK_JAVA_CALC_ERROR([$1],
                           [() + (1 + 1 + 1 +) + (* * *) + (1 * 2 * *) = 1],
-[1: syntax error, unexpected ')', expecting number or '-' or '(' or '!'
-1: syntax error, unexpected ')', expecting number or '-' or '(' or '!'
-1: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
-1: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
-calc: error: 4444 != 1])
+[1.2: syntax error, unexpected ')', expecting number or '-' or '(' or '!'
+1.11: syntax error, unexpected ')', expecting number or '-' or '(' or '!'
+1.14: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
+1.24: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
+1.1-1.27: calc: error: 4444 != 1])
 
 # The same, but this time exercising explicitly triggered syntax errors.
 # POSIX says the lookahead causing the error should not be discarded.
 _AT_CHECK_JAVA_CALC_ERROR([$1], [(!) + (0 0) = 1],
-[1: syntax error, unexpected number
-calc: error: 2222 != 1])
+[1.7: syntax error, unexpected number
+1.1-1.10: calc: error: 2222 != 1])
 _AT_CHECK_JAVA_CALC_ERROR([$1], [(- *) + (0 0) = 1],
-[1: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
-1: syntax error, unexpected number
-calc: error: 2222 != 1])
+[1.3: syntax error, unexpected '*', expecting number or '-' or '(' or '!'
+1.8: syntax error, unexpected number
+1.1-1.11: calc: error: 2222 != 1])
 AT_BISON_OPTION_POPDEFS
 
 AT_CLEANUP
-- 
1.7.0.4