[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] style: decouple different uses of item_number
From: |
Akim Demaille |
Subject: |
Re: [PATCH] style: decouple different uses of item_number |
Date: |
Mon, 25 May 2020 19:36:44 +0200 |
> 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!