Re: set_goto_map() has unintentional ++ operator

[Akim Demaille]

> Le 29 déc. 2014 à 22:40, sean nakasone <address@hidden> a écrit :
> 
> Hi Akim, I didn't test it, nor did I run it in a debugger..  so there must be 
> something I'm not seeing.
> 
> The placement of the ++ makes it a post increment, so it's incremented after 
> the statement, so it should not affect the value of k.

This, I agree.  But what makes you think the following
iterations don't rely on this?

Well, at least the test suite believes it :)

> 
> It's incrementing values in temp_map, but temp_map is deallocated after the 
> for loop, so that's why I thought it wasn't doing anything.
> 
>  for (s = 0; s < nstates; ++s)
>    {
>      transitions *sp = states[s]->transitions;
>      int i;
>      for (i = sp->num - 1; i >= 0 && TRANSITION_IS_GOTO (sp, i); --i)
>        {
>          goto_number k = temp_map[TRANSITION_SYMBOL (sp, i) - ntokens]++;
>          from_state[k] = s;
>          to_state[k] = sp->states[i]->number;
>        }
>    }
> 
>  free (temp_map);
> 
> 
> 
> I appreciate your efforts and follow up.
> 
> 
> 
> --------------------------------------------
> On Mon, 12/29/14, Akim Demaille <address@hidden> wrote:
> 
> Subject: Re: set_goto_map() has unintentional ++ operator
> To: "Sean Nakasone" <address@hidden>
> Cc: "Bison Help" <address@hidden>
> Date: Monday, December 29, 2014, 5:29 AM
> 
> 
>> Le
> 21 déc. 2014 à 21:51, Sean Nakasone <address@hidden>
> a écrit :
>> 
>> in
> lalr.c
>> 
>> the ++ at
> the end of the following line doesn't have a purpose
>> 
>> goto_number k =
> temp_map[TRANSITION_SYMBOL(sp, i) - ntokens]++;
>> 
>> it can be
> misleading. 
> 
> hi Sean,
> 
> Well, this piece of code is
> old, and it does not seem immediate
> (to me)
> that the ++ is useless :)
> 
>   from_state = xcalloc (ngotos, sizeof
> *from_state);
>   to_state = xcalloc (ngotos,
> sizeof *to_state);
> 
>   for
> (s = 0; s < nstates; ++s)
>     {
>       transitions *sp =
> states[s]->transitions;
>       int
> i;
>       for (i = sp->num - 1; i >=
> 0 && TRANSITION_IS_GOTO (sp, i); --i)
>         {
>          
> goto_number k = temp_map[TRANSITION_SYMBOL (sp, i) -
> ntokens]++;
>           from_state[k] =
> s;
>           to_state[k] =
> sp->states[i]->number;
>        
> }
>     }
> 
>   free (temp_map);
> 
> FTR, byacc (beach and bison are forks of a
> common ancestor) reads:
> 
>  
>   from_state = NEW2(ngotos, Value_t);
>    
> to_state = NEW2(ngotos, Value_t);
> 
>     for (sp = first_shift; sp; sp =
> sp->next)
>     {
>    
> state1 = sp->number;
>     for (i =
> sp->nshifts - 1; i >= 0; i--)
>    
> {
>         state2 = sp->shift[i];
>         symbol =
> accessing_symbol[state2];
> 
>         if (ISTOKEN(symbol))
>         break;
> 
>         k = temp_map[symbol]++;
>         from_state[k] = state1;
>         to_state[k] = state2;
>     }
>     }
> 
>     FREE(temp_map +
> ntokens);
> 
> it has the same
> ++.
> 
> 
> What
> makes you think it is useless?  If I remove it, some tests
> fail
> (they hang), or I can even get a
> SEGV.
> 
> $ cat input.y
> %%
> start: end end {} ;
> end:
> { }  ;
> $ ./_build/35s/tests/bison
> input.y
> ./_build/35s/tests/bison: line 28:
> 87593 Segmentation fault: 11  $PREBISON
> "$abs_top_builddir/src/bison"
> ${1+"$@"}
> 
> 
> 
> _______________________________________________
> address@hidden
> https://lists.gnu.org/mailman/listinfo/help-bison
> 
> _______________________________________________
> address@hidden https://lists.gnu.org/mailman/listinfo/help-bison