Re: [PATCH 4/8] dfa: refactor common context computations

[Jim Meyering]

Paolo Bonzini wrote:
> * src/dfa.c (CTX_ANY, charclass_context, state_wanted_contexts): New.
> (dfaanalyze): Use state_wanted_contexts.
> (dfastate): Use charclass_context and state_wanted_contexts.

Nice factorization/clean-up.

>  src/dfa.c |   75 ++++++++++++++++++++++++++++++++++++++----------------------
...

It'd be nice to add a brief comment here.

> +int
> +charclass_context(charclass c)

s/(/ (/

Oh.  add "static", too.

> +{
> +  int context = 0;
> +  int j;

Please use "unsigned int" for j.

> +  if (tstbit(eolbyte, c))

s/(e/ (e/

> +    context |= CTX_NEWLINE;
> +
> +  for (j = 0; j < CHARCLASS_INTS; ++j)
> +    {
> +      if (c[j] & letters[j])
> +        context |= CTX_LETTER;
> +      if (c[j] & ~(letters[j] | newline[j]))
> +        context |= CTX_NONE;
> +    }
> +
> +  return context;
> +}

It'd be nice to add a comment here, too.

> +int state_wanted_contexts(position_set *s, int possible_contexts)

Add "static" and adjust formatting:

  static int
  state_wanted_contexts (position_set *s, int possible_contexts)

> +{
> +  int wanted_context = 0;
> +  int j;

Please use "unsigned int" for j.

> +  for (j = 0; j < s->nelem; ++j)
> +    {
> +      if ((possible_contexts & CTX_NEWLINE)
> +          && PREV_NEWLINE_DEPENDENT(s->elems[j].constraint))
> +        wanted_context |= CTX_NEWLINE;
> +      if ((possible_contexts & CTX_LETTER)
> +          && PREV_LETTER_DEPENDENT(s->elems[j].constraint))
> +        wanted_context |= CTX_LETTER;
> +    }
> +
> +  return wanted_context;
> +}
...