Re: push parser

[Joel E. Denny]

On Fri, 15 Dec 2006, Joel E. Denny wrote:

> Bob and Paul, I can't seem to communicate this adequately in English, so I 
> think the best way to explain my view is to write the patch that 
> implements it.  It should be very short, but it may be a few hours before 
> I can get some time.  Can we pick up the discussion again after that?

The patch is below, but I'll wait for both of your reviews before 
committing it.  It's very short.  Notice that for push parsers now:

1. I get the same yyerror arguments for pure pull parsers as for push 
parsers (so it's possible to get a location now).  I don't have to use 
b4_push_if in b4_yacc_pure_if, which is used by b4_yyerror_args.  It's 
just a question of purity.

2. I get the right yylex arguments (which must include YYSTYPE and YYLTYPE 
pointers since push parsers are pure).  I don't have to use b4_push_if in 
b4_lex_param or in the #define YYLEX.  It's just a question of purity. 
(yylex will be important if we agree to implement yyparse as I have 
proposed.)

3. I no longer have to test both b4_push_if and b4_pure_if in order to 
decide whether b4_declare_parser_variables should appear globally.  It's 
just a question of purity.

4. I no longer have to test both b4_push_if and b4_pure_if in order to 
decide whether b4_declare_parser_variables should appear in 
yyparse/yypush_parse.  It's just a question of purity.

5. I no longer get yylval and yylloc extern'ed in the header because 
testing purity is now enough.

Sure, b4_push_if makes a few additional changes that b4_pure_if doesn't 
make, but notice how little overlap there is in the code they touch.  
Actually, I don't believe I ever have to check something equivalent to 
b4_pure_but_not_push_if.

Also, before I forget, I noticed some other problems:

1. I added b4_parse_param to the yypush_parse prototype.  Otherwise, 
%parse-param is useless, and that makes it hard to test this code.  
Whether this is how %parse-param should affect push parsers may be a point 
of debate.  It was just a quick fix, and I haven't thought it through.

2. Why do we need a b4_declare_yyparse_variables?  First, it's used in 
only one place.  Second, it isn't named correctly: it's for push parsers 
not for yyparse.  I haven't fixed this.

3. Why don't we "Initialize the default location before parsing starts" 
for push parsers?  Also, %initial-action does not play well with the 
location passed to yypush_parse.  If @$ is set in %initial-action, @$ 
overwrites the location of every token.  %initial-action ought to be 
performed only the first time yypush_parse is called and each time the 
parse starts over; that would be consistent with how it works for yyparse.  
Moreover, the location passed to yypush_parse ought to be assigned to 
yylloc only when it's time to read a token (where YYLEX is invoked).  
Otherwise, @$ in %initial-action and it clobber each other.  I'm guessing 
$$ has the same problem and solution.  I haven't fixed any of this.

Index: ChangeLog
===================================================================
RCS file: /sources/bison/bison/ChangeLog,v
retrieving revision 1.1624
diff -p -u -r1.1624 ChangeLog
--- ChangeLog   15 Dec 2006 05:31:26 -0000      1.1624
+++ ChangeLog   15 Dec 2006 20:42:33 -0000
@@ -1,3 +1,15 @@
+2006-12-15  Joel E. Denny  <address@hidden>
+
+       Make %push-parser imply %pure-parser.  This fixes several bugs; see
+       <http://example.com/url/coming/soon>.
+       * src/parse-gram.y (prologue_declaration): For %push-parser, also set
+       pure_parser = true.
+       * data/push.c: Don't bother testing b4_push_if when deciding whether
+       to expand b4_declare_parser_variables globally.
+       (yypush_parse): Likewise in here.
+
+       * data/push.c (yypush_parse): Add b4_parse_param to arguments.
+
 2006-12-15  Bob Rossi  <address@hidden>
        and Joel Denny  <address@hidden>
 
Index: data/push.c
===================================================================
RCS file: /sources/bison/bison/data/push.c,v
retrieving revision 1.16
diff -p -u -r1.16 push.c
--- data/push.c 15 Dec 2006 05:31:26 -0000      1.16
+++ data/push.c 15 Dec 2006 20:42:34 -0000
@@ -971,7 +971,8 @@ enum { YYPUSH_MORE = 4 };
    [[yypstate *yyps], [yyps]],
    [[int yynchar], [yynchar]],
    [[YYSTYPE const *yynlval], [yynlval]]
-   b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]]))[
+   b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]])
+   m4_ifset([b4_parse_param], [,]) b4_parse_param)[
 ],[#ifdef YYPARSE_PARAM
 ]b4_c_function_decl([yyparse], [int],
    [[void *YYPARSE_PARAM], [YYPARSE_PARAM]])[
@@ -1081,8 +1082,7 @@ m4_define([b4_declare_yyparse_variables]
 ])
 m4_divert_pop([KILL])dnl# ====================== End of M4 code.
 
-b4_push_if([],[b4_pure_if([],
-          [b4_declare_parser_variables])])
+b4_pure_if([], [b4_declare_parser_variables])
 
 b4_push_if([b4_declare_yyparse_variables])
 
@@ -1094,7 +1094,8 @@ b4_push_if([
 b4_c_function_def([yypush_parse], [int], [[yypstate *yyps], [yyps]], 
                   [[int yynchar], [yynchar]],
                   [[YYSTYPE const *yynlval], [yynlval]]
-                 b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]]))],[
+                 b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]])
+                  m4_ifset([b4_parse_param], [,]) b4_parse_param)],[
 #ifdef YYPARSE_PARAM
 b4_c_function_def([yyparse], [int], [[void *YYPARSE_PARAM], [YYPARSE_PARAM]])
 #else /* ! YYPARSE_PARAM */
@@ -1102,7 +1103,6 @@ b4_c_function_def([yyparse], [int], b4_p
 #endif])
 {[
   ]b4_pure_if([b4_declare_parser_variables])[
-  ]b4_push_if([b4_declare_parser_variables])[
   int yystate;
   int yyn;
   int yyresult;
@@ -1700,7 +1700,8 @@ typedef struct ]b4_prefix[pstate ]b4_pre
    [[]b4_prefix[pstate *]b4_prefix[pstate], []b4_prefix[pstate]],
    [[int yynchar], [yynchar]],
    [[YYSTYPE const *yynlval], [yynlval]]
-   b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]]))[
+   b4_locations_if([,[[YYLTYPE const *yynlloc], [yynlloc]]])
+   m4_ifset([b4_parse_param], [,]) b4_parse_param)[
 enum { YYPUSH_MORE = 4 };])[
 
 m4_ifdef([b4_provides],
Index: src/parse-gram.y
===================================================================
RCS file: /sources/bison/bison/src/parse-gram.y,v
retrieving revision 1.100
diff -p -u -r1.100 parse-gram.y
--- src/parse-gram.y    2 Dec 2006 01:52:16 -0000       1.100
+++ src/parse-gram.y    15 Dec 2006 20:42:35 -0000
@@ -255,7 +255,7 @@ prologue_declaration:
 | "%output" "=" STRING          { spec_outfile = $3; }  /* deprecated */
 | "%parse-param" "{...}"       { add_param ("parse_param", $2, @2); }
 | "%pure-parser"                { pure_parser = true; }
-| "%push-parser"                { push_parser = true; }
+| "%push-parser"                { push_parser = true; pure_parser = true; }
 | "%require" STRING             { version_check (&@2, $2); }
 | "%skeleton" STRING            { skeleton = $2; }
 | "%token-table"                { token_table_flag = true; }