Re: cp/parse.y:2120: invalid value: $3

[Akim Demaille]

>>>>> "Jason" == Jason Merrill <address@hidden> writes:

>>>>> "Mark" == Mark Mitchell <address@hidden> writes:
>> --On Monday, April 29, 2002 04:22:00 PM -0700 Paul Eggert
>> <address@hidden> wrote:

>>> OK, but here is a more conservative patch that also avoids the
>>> dirty hack.  This patch avoids grammar rule duplication and should
>>> be a bit easier to maintain.

>> Nathan, would you apply this patch to branch and mainline, please?

Jason> Er, it may be easier to maintain, but it also breaks the
Jason> compiler.  With this patch, libstdc++ fails to build on the
Jason> branch using bison 1.28.

It is not related to the version of Bison.  I didn't try, and at first
sight, there is one thing that Paul's patch removed: setting $2.


*** parse.y     2002-04-29 14:43:20.511360000 -0700
--- parse.y-fix 2002-04-29 16:17:35.996361000 -0700
***************
*** 2147,2158 ****
          ;
    
  nomods_initdcl0:
            notype_declarator maybeasm
-             { /* Set things up as initdcl0_innards expects.  */
-             $<ttype>3 = $2;
-             $2 = $1; 
-               $<ftype>1.t = NULL_TREE;
-             $<ftype>1.lookups = NULL_TREE; }
            initdcl0_innards 
              {}
        | constructor_declarator maybeasm maybe_attribute
--- 2147,2156 ----
          ;
    
  nomods_initdcl0:
+           { /* Set things up as initdcl0_innards expects.  */

so here, in addition, to respect what was performed before, you need
an additional $2 = $1.  Does that fix your issue?

+             $<ftype>$.t = NULL_TREE;
+             $<ftype>$.lookups = NULL_TREE; }
            notype_declarator maybeasm
            initdcl0_innards 
              {}
        | constructor_declarator maybeasm maybe_attribute



I would like to emphasize that the comments are incorrect too:

|         /* This rule assumes a certain configuration of the parser stack.
|            In particular, $0, the element directly before the beginning of
|            this rule on the stack, must be a maybeasm.  $-1 must be a
|            declarator or notype_declarator.  And $-2 must be some declmods
|            or declspecs.  We can't move the maybeasm into this rule because
|            we need that reduce so we prefer fn.def1 when appropriate.  */
| initdcl0_innards:
|           maybe_attribute '='
|                 { $<ttype>$ = parse_decl0 ($<ttype>-1, $<ftype>-2.t,
|                                            $<ftype>-2.lookups, $1, 1); }
|           /* Note how the declaration of the variable is in effect
|              while its init is parsed! */
|           init
|                 { parse_end_decl ($<ttype>3, $4, $<ttype>0); }
|         | maybe_attribute
|                 { tree d = parse_decl0 ($<ttype>-1, $<ftype>-2.t,
|                                         $<ftype>-2.lookups, $1, 0);
|                   parse_end_decl (d, NULL_TREE, $<ttype>0); }
|         ;
| 
| initdcl0:
|           declarator maybeasm initdcl0_innards
|                 {}
|         ;
| 
| notype_initdcl0:
|           notype_declarator maybeasm initdcl0_innards
|                 {}
|         ;
| 
| nomods_initdcl0:
|           notype_declarator maybeasm
|             { /* Set things up as initdcl0_innards expects.  */
|               $<ttype>$ = $2;
|               $2 = $1;
|               $<ftype>1.t = NULL_TREE;
|               $<ftype>1.lookups = NULL_TREE; }
|           initdcl0_innards
|             {}
|         | constructor_declarator maybeasm maybe_attribute
|                 { tree d = parse_decl0 ($1, NULL_TREE, NULL_TREE, $3, 0);
|                   parse_end_decl (d, NULL_TREE, $2); }
|         ;

Note that

| initdcl0:
|           declarator maybeasm initdcl0_innards
|                 {}
|         ;
| 
| notype_initdcl0:
|           notype_declarator maybeasm initdcl0_innards
|                 {}

conform to the comment: ``In particular, $0, the element directly
before the beginning of this rule on the stack, must be a maybeasm.''

In the case of the `bad' rule, you do not have a maybeasm, you have
the $$ of the mid-rule action.  Let's note this mid-rule action @1,
you actually have:

| nomods_initdcl0:
|           notype_declarator maybeasm    @1 initdcl0_innards {}
wrt @1: 
         $0      $1              $2       $$
wrt initdcl0_innards:
        $-3      $-2             $-1      $0 $$

With $2 = $1 in the mid-rule action, you actually have:

| nomods_initdcl0:
|           notype_declarator maybeasm    @1 initdcl0_innards {}
wrt @1: 
         $0      $1              $1       $$
wrt initdcl0_innards:
        $-3      $-2             $-1      $0 $$

You seem to rely implicitly on on Bison always doing $$ = $1 (relying
on this is *bad*).  So there is, before $2 = $1, $$ = $1:

| nomods_initdcl0:
|           notype_declarator maybeasm    @1 initdcl0_innards {}
wrt @1: 
         $0      $1              $1       $1
wrt initdcl0_innards:
        $-3      $-2             $-1      $1 $$

While it seems to me that the code aims at:

| nomods_initdcl0:
|           notype_declarator maybeasm    @1 initdcl0_innards {}
wrt @1: 
         $X      $0              $1       $2
wrt initdcl0_innards:
        $-3      $-2             $-1      $1 $$


So I would suggest, based on my understanding, the following code:

nomods_initdcl0:
          notype_declarator maybeasm
            { 
              /* Reset some fields in the $0 of initdcl0_innards. */
              $<ftype>1.t = NULL_TREE;
              $<ftype>1.lookups = NULL_TREE; 
              /* Because of this mid-rule action, arguments are
                 shifted: move them as if this action did not exist. */
              $<ftype>$ = $<ftype>2;    /* Prepare $0 in initdcl0_innards as 
notype_declarator. */
              $<ttype>2 = $<ttype>1;    /* Prepare $-1 in initdcl0_innards as 
maybeasm. */
              $<ttype>1 = $<ttype>0;    /* Prepare $-2 in initdcl0_innards as 
datadecl. */
             }
          initdcl0_innards
            {}

The problem is that this code is quite different from what it used to
be, but based on the comments, that what makes sense to me :(