Re: [PATCH] style: decouple different uses of item_number

[vmi6]

My bad, I don’t know what was going through my head.

> On May 25, 2020, at 1:36 PM, Akim Demaille <address@hidden> wrote:
> 
> 
> 
>> Le 25 mai 2020 à 14:41, Vincent Imbimbo <address@hidden> a écrit :
>> 
>> Hey Akim,
>> 
>> I'm confused about these item_indices you've changed to item_number, since
>> state->items is an array of item_indices.
> 
> Hi Vincent,
> 
> My turn to be confused: what you show is going from item_number to index.
> Maybe you're showing the diff in your repo instead of quoting my patch?
> Here it is.
> 
>> diff --git a/src/closure.c b/src/closure.c
>> index 94439b13..ee0c9d4a 100644
>> --- a/src/closure.c
>> +++ b/src/closure.c
>> @@ -60,10 +60,10 @@ closure_print (char const *title, item_index const 
>> *array, size_t size)
>>   for (size_t i = 0; i < size; ++i)
>>     {
>>       fprintf (stderr, "  %2d: .", array[i]);
>> -      item_index *rp;
>> -      for (rp = &ritem[array[i]]; *rp >= 0; ++rp)
>> +      item_number *rp;
>> +      for (rp = &ritem[array[i]]; 0 <= *rp; ++rp)
> 
> rp is clearly iterating on ritem, which is a an array of item_numbers.
> 
>>         fprintf (stderr, " %s", symbols[*rp]->tag);
>> -      fprintf (stderr, "  (rule %d)\n", -*rp - 1);
>> +      fprintf (stderr, "  (rule %d)\n", item_number_as_rule_number (*rp));
>>     }
>>   fputs ("\n\n", stderr);
>> }
>> diff --git a/src/gram.h b/src/gram.h
>> index a4b58f0a..77fc7699 100644
>> --- a/src/gram.h
>> +++ b/src/gram.h
>> @@ -111,14 +111,15 @@ extern int nsyms;
>> extern int ntokens;
>> extern int nvars;
>> 
>> -/* elements of ritem */
>> +/* Elements of ritem. */
>> typedef int item_number;
>> -/* indices into ritem */
>> -typedef int item_index;
>> # define ITEM_NUMBER_MAX INT_MAX
>> extern item_number *ritem;
>> extern int nritems;
>> 
>> +/* Indices into ritem. */
>> +typedef unsigned int item_index;
>> +
>> /* There is weird relationship between OT1H item_number and OTOH
>>    symbol_number and rule_number: we store the latter in
>>    item_number.  symbol_number values are stored as-is, while
>> diff --git a/src/ielr.c b/src/ielr.c
>> index f86cd461..47ac714a 100644
>> --- a/src/ielr.c
>> +++ b/src/ielr.c
>> @@ -1030,7 +1030,7 @@ ielr_split_states (bitsetv follow_kernel_items, 
>> bitsetv always_follows,
>>         if (!node->state->consistent)
>>           {
>>             size_t i = 0;
>> -            item_number *itemset = node->state->items;
>> +            item_index *itemset = node->state->items;
> 
> struct state is:
> 
> 
> struct state
> {
>  state_number number;
> ...
>  /* Its items.  Must be last, since ITEMS can be arbitrarily large.  Sorted
>     ascendingly on item index in RITEM, which is sorted on rule number.  */
>  size_t nitems;
>  item_index items[1];
> };
> 
> So my change is consistent with yours.  And indeed it seems to be correct,
> since, for instance:
> 
> 
> 
>>             for (size_t r = 0; r < node->state->reductions->num; ++r)
>>               {
>>                 rule *this_rule = node->state->reductions->rules[r];
>> diff --git a/src/print-xml.c b/src/print-xml.c
>> index a44af211..168d6bd5 100644
>> --- a/src/print-xml.c
>> +++ b/src/print-xml.c
>> @@ -57,7 +57,7 @@ static struct escape_buf escape_bufs[num_escape_bufs];
>> static void
>> print_core (FILE *out, int level, state *s)
>> {
>> -  item_number *sitems = s->items;
>> +  item_index *sitems = s->items;
>>   size_t snritems = s->nitems;
>> 
>>   /* Output all the items of a state, not only its kernel.  */
> 
> 
> this continues with:
> 
>  /* Output all the items of a state, not only its kernel.  */
>  closure (sitems, snritems);
>  sitems = itemset;
>  snritems = nitemset;
> 
>  if (!snritems)
>    {
>      xml_puts (out, level, "<itemset/>");
>      return;
>    }
> 
>  xml_puts (out, level, "<itemset>");
> 
>  for (size_t i = 0; i < snritems; i++)
>    {
>      bool printed = false;
>      item_number *sp1 = ritem + sitems[i];
>      rule const *r = item_rule (sp1);
>      item_number *sp = r->rhs;
> 
> so indeed, sitems[i] is used as an index inside ritem, so it's an index.
> 
> 
> 
>> diff --git a/src/print.c b/src/print.c
>> index 720740ee..1da0f9dd 100644
>> --- a/src/print.c
>> +++ b/src/print.c
>> @@ -61,7 +61,7 @@ max_length (size_t *width, const char *str)
>> static void
>> print_core (FILE *out, state *s)
>> {
>> -  item_number *sitems = s->items;
>> +  item_index *sitems = s->items;
>>   size_t snritems = s->nitems;
>>   /* Output all the items of a state, not only its kernel.  */
>>   if (report_flag & report_itemsets)
> 
> 
> 
> So I don't understand what is the problem you noticed.  You'll have to be more
> specific, I don't see it :(
> 
> Cheers!
>