Re: [PATCH 3/8] dfa: change newline/letter to a single context value

[Jim Meyering]

Paolo Bonzini wrote:
> * src/dfa.c (MATCHES_NEWLINE_CONTEXT, MATCHES_LETTER_CONTEXT,
> SUCCEEDS_IN_CONTEXT, ACCEPTS_IN_CONTEXT): Take a single context value
> for prev and curr.
> (struct dfa_state): Replace newline and letter with context.
> (wchar_context): New.
> (state_index): Replace newline and letter with context.  Compare
> context values in the state struct.  Adjust calls to pass contexts.
> (wants_newline): Replace with wanted_context.  Adjust calls to pass
> contexts.
> (dfastate): Replace wants_newline and wants_letter with wanted_context.
> Adjust calls to pass contexts.
> (build_state): Adjust calls to pass contexts.
> (match_anychar, match_mb_charset, transit_state): Use wchar_context.
> Adjust calls to pass contexts.
> ---
>  src/dfa.c |  169 
> ++++++++++++++++++++++++++++++-------------------------------
>  1 files changed, 84 insertions(+), 85 deletions(-)
>
> diff --git a/src/dfa.c b/src/dfa.c
> index a851c43..26d2bfb 100644
> --- a/src/dfa.c
> +++ b/src/dfa.c
> @@ -92,7 +92,12 @@ typedef int charclass[CHARCLASS_INTS];
>  static inline unsigned char to_uchar (char ch) { return ch; }
>
>  /* Contexts tell us whether a character is a newline or a word constituent.
> -   Word-constituent characters are those that satisfy iswalnum(), plus '_'.  
> */
> +   Word-constituent characters are those that satisfy iswalnum(), plus '_'.
> +
> +   A states also stores a context value, which is nonzero if its

ACK, modulo nits

s/states/state/

> +   predecessors always matches a newline or a word constituent.
> +   The definition of a state's context is a bit unclear, but will be
> +   modified soon anyway.  */
>
>  #define CTX_NONE     1
>  #define CTX_LETTER   2
...
> @@ -2149,7 +2159,7 @@ dfaanalyze (struct dfa *d, int searchflag)
>    position *lastpos;         /* Array where lastpos elements are stored. */
>    position_set tmp;          /* Temporary set for merging sets. */
>    position_set merged;               /* Result of merging sets. */
> -  int wants_newline;         /* True if some position wants newline info. */
> +  int wanted_context;                /* Context wanted by some position. */

I've always found the "wants_newline" name to be um... wanting,
so am glad you're changing it.  However, rather than "wanted_context",
how about "required_context"?  Why?  I guess to "require" something is
passive and more formal, whereas to "want" something is more about
desire, which doesn't really apply here.

If you take this suggestion, the change need not cause you inordinate
rebasing hassle.  If you save the series via git format-patch --stdout
origin/master > k, perform the global substitution in "k", and reapply
the series on a clean base using "git am k".  The only touch-up that
may be required would be due to long lines or alignment in continued
statements.