Re: Bison test case fixes uncovered by recent bug reports

[Joel E. Denny]

On Wed, 21 Sep 2005, Akim Demaille wrote:

> >>> "Joel" == Joel E Denny <address@hidden> writes:
>
>  > I disagree.  There's no reason for parameters to yyerror() and to yylex()
>  > to intrude on the parser's namespace.  I've noticed that the bison
>  > documentation is careful about this.
>
> There are reaons: at some places the test suite is tailored to work
> with or with-out %pure-parser.

See the patch at the end of this email.  It's identical to the previous
patch I posted... except I removed `yy' from the new names.

As far as I can tell, the changes I'm making only affect pure parsers.
The code I'm changing seems to be specifically declared that way. Let me
know if I'm misinterpreting it. Moreover, the test suite works fine with
these changes.  Am I missing some hidden failure or false success?

> It is already quite a cathedral as it
> is so that I don't which to have in addition to change the name of
> yylval and yylloc depending on whether they are global variables or
> arguments.

I don't follow.

>  To be consistent and to ease the sharing of routines in
> the test suite, I chose to always use these names.

But doesn't that mean the test suite never tests the possibility that the
user may choose other names for yyerror() params... as the documentation
suggests he should?

> It is very sane not to promote this in the documentation, but IMHO,
> it's inappropriate for the test suite.  It will always be time to
> change the test suite when we change it.

I'm about to propose a code-cleanup patch for glr.c that, for the case of
%pure-parser, #define's yylloc and yylval similarly to the way it already
#define's yychar. This cleanup helps me with another GLR patch to fix a
visible bug. Unfortunately, the test suite then fails until I apply the
patch at the end of this email. Maybe Paul will reject the GLR patch,
maybe not, but I think this cleanup in the test suite would aid future
evolution of bison regardless.

Joel

Index: calc.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/calc.at,v
retrieving revision 1.78
diff -p -u -r1.78 calc.at
--- calc.at     19 Sep 2005 21:08:21 -0000      1.78
+++ calc.at     21 Sep 2005 16:25:27 -0000
@@ -72,7 +72,7 @@ static int power (int base, int exponent
 [/* yyerror receives the location if:
    - %location & %pure & %glr
    - %location & %pure & %yacc & %parse-param. */
-static void yyerror (AT_YYERROR_ARG_LOC_IF([YYLTYPE *yylloc, ])
+static void yyerror (AT_YYERROR_ARG_LOC_IF([YYLTYPE *llocp, ])
                      AT_PARAM_IF([semantic_value *result, int *count, ])
                      const char *s
                      );])[
@@ -145,7 +145,7 @@ yyparse (AT_PARAM_IF([semantic_value *re
 }
 ],
 [static void
-yyerror (AT_YYERROR_ARG_LOC_IF([YYLTYPE *yylloc, ])
+yyerror (AT_YYERROR_ARG_LOC_IF([YYLTYPE *llocp, ])
          AT_PARAM_IF([semantic_value *result, int *count, ])
          const char *s)
 {
Index: local.at
===================================================================
RCS file: /cvsroot/bison/bison/tests/local.at,v
retrieving revision 1.11
diff -p -u -r1.11 local.at
--- local.at    25 Aug 2005 08:53:33 -0000      1.11
+++ local.at    21 Sep 2005 16:25:27 -0000
@@ -79,14 +79,14 @@ m4_pushdef([AT_PURE_LEX_IF],
             [AT_LALR1_CC_IF([$1], [$2])])])

 AT_PURE_LEX_IF(
-[m4_pushdef([AT_LOC], [(*yylloc)])
- m4_pushdef([AT_VAL], [(*yylval)])
+[m4_pushdef([AT_LOC], [(*llocp)])
+ m4_pushdef([AT_VAL], [(*lvalp)])
  m4_pushdef([AT_LEX_FORMALS],
-            [YYSTYPE *yylval[]AT_LOCATION_IF([, YYLTYPE *yylloc])])
+            [YYSTYPE *lvalp[]AT_LOCATION_IF([, YYLTYPE *llocp])])
  m4_pushdef([AT_LEX_ARGS],
-            [yylval[]AT_LOCATION_IF([, yylloc])])
+            [lvalp[]AT_LOCATION_IF([, llocp])])
  m4_pushdef([AT_USE_LEX_ARGS],
-            [(void) yylval;AT_LOCATION_IF([(void) yylloc])])
+            [(void) lvalp;AT_LOCATION_IF([(void) llocp])])
  m4_pushdef([AT_LEX_PRE_FORMALS],
             [AT_LEX_FORMALS, ])
  m4_pushdef([AT_LEX_PRE_ARGS],