Re: set_goto_map() has unintentional ++ operator

[Sean Nakasone]

i wasnt reading the code thoroughly... i apologize. 

how do run the test suite?

Sent from my iPhone

> On Dec 30, 2014, at 2:43 AM, Akim Demaille <address@hidden> wrote:
> 
> 
>> 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
>