Re: [PATCH 20/34] maint: dfa: convert #if-MBS_SUPPORT (dfastate)

[Jim Meyering]

Paolo Bonzini wrote:
> On 09/15/2011 12:28 PM, Jim Meyering wrote:
>> diff --git a/src/dfa.c b/src/dfa.c
>> index dd8259c..f245208 100644
>> --- a/src/dfa.c
>> +++ b/src/dfa.c
>> @@ -2578,11 +2578,9 @@ dfastate (int s, struct dfa *d, int trans[])
>>
>>         /* If we are building a searching matcher, throw in the positions
>>            of state 0 as well. */
>> -#if MBS_SUPPORT
>> -      if (d->searchflag&&  (d->mb_cur_max == 1 || !next_isnt_1st_byte))
>> -#else
>> -      if (d->searchflag)
>> -#endif
>> +      if (d->searchflag
>> +          &&  (! MBS_SUPPORT
>> +              || (d->mb_cur_max == 1 || !next_isnt_1st_byte)))
>
> No need for the inner parentheses around the ||.

True.  Adjusted.

> Or change it to
>
>    && !( MBS_SUPPORT && d->mb_cur_max > 1 && next_isnt_1st_byte)
>
> Alternatively, in patch 30 (dfa: simplify several expressions) just change it 
> to
>
>    if (d->searchflags && !next_isnt_1st_byte)

Good one.

> since next_isnt_1st_byte is always zero for single-byte character
> sets. Bonus points for reversing its direction to next_is_1st_byte. :)

Let's save semantics-inverting name changes for a separate patch.

FYI, here's my new "simplify several expressions" patch:

>From e393c739d09a4f246e673cfc822736a90e76c79f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 11 Sep 2011 20:19:35 +0200
Subject: [PATCH] maint: dfa: simplify several expressions

* src/dfa.c (dfainit): Set d->mb_cur_max unconditionally, now
that MB_CUR_MAX is always usable.  With that, simplify all
"MBS_SUPPORT && d->mb_cur_max > 1" to simply "d->mb_cur_max > 1".
(dfastate, dfaexec, dfainit, dfafree): Simplify, removing each
now-unnecessary "MBS_SUPPORT &&".
(dfastate): Also make this change (thanks to Paolo Bonzini):
-   if (d->searchflag
-       && (! MBS_SUPPORT || d->mb_cur_max == 1 || !next_isnt_1st_byte))
+   if (d->searchflag && !next_isnt_1st_byte)
Removing the d->mb_cur_max test is legitimate since, as he explained,
next_isnt_1st_byte is always zero for a single-byte character.
---
 src/dfa.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/dfa.c b/src/dfa.c
index eba59fc..7195e1c 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -2536,7 +2536,7 @@ dfastate (int s, struct dfa *d, int trans[])
         for (k = 0; k < d->follows[grps[i].elems[j].index].nelem; ++k)
           insert(d->follows[grps[i].elems[j].index].elems[k], &follows);

-      if (MBS_SUPPORT && d->mb_cur_max > 1)
+      if (d->mb_cur_max > 1)
         {
           /* If a token in follows.elems is not 1st byte of a multibyte
              character, or the states of follows must accept the bytes
@@ -2569,8 +2569,7 @@ dfastate (int s, struct dfa *d, int trans[])

       /* If we are building a searching matcher, throw in the positions
          of state 0 as well. */
-      if (d->searchflag
-          && (! MBS_SUPPORT || d->mb_cur_max == 1 || !next_isnt_1st_byte))
+      if (d->searchflag && !next_isnt_1st_byte)
         for (j = 0; j < d->states[0].elems.nelem; ++j)
           insert(d->states[0].elems.elems[j], &follows);

@@ -3237,7 +3236,7 @@ dfaexec (struct dfa *d, char const *begin, char *end,
   saved_end = *(unsigned char *) end;
   *end = eol;

-  if (MBS_SUPPORT && d->mb_cur_max > 1)
+  if (d->mb_cur_max > 1)
     {
       MALLOC(mblen_buf, end - begin + 2);
       MALLOC(inputwcs, end - begin + 2);
@@ -3247,7 +3246,7 @@ dfaexec (struct dfa *d, char const *begin, char *end,

   for (;;)
     {
-      if (MBS_SUPPORT && d->mb_cur_max > 1)
+      if (d->mb_cur_max > 1)
         while ((t = trans[s]))
           {
             if (p > buf_end)
@@ -3299,7 +3298,7 @@ dfaexec (struct dfa *d, char const *begin, char *end,
             {
               if (backref)
                 *backref = (d->states[s].backref != 0);
-              if (MBS_SUPPORT && d->mb_cur_max > 1)
+              if (d->mb_cur_max > 1)
                 {
                   free(mblen_buf);
                   free(inputwcs);
@@ -3309,7 +3308,7 @@ dfaexec (struct dfa *d, char const *begin, char *end,
             }

           s1 = s;
-          if (MBS_SUPPORT && d->mb_cur_max > 1)
+          if (d->mb_cur_max > 1)
             {
               /* Can match with a multibyte character (and multicharacter
                  collating element).  Transition table might be updated.  */
@@ -3327,14 +3326,14 @@ dfaexec (struct dfa *d, char const *begin, char *end,
           if (count)
             ++*count;

-          if (MBS_SUPPORT && d->mb_cur_max > 1)
+          if (d->mb_cur_max > 1)
             prepare_wc_buf ((const char *) p, end);
         }

       /* Check if we've run off the end of the buffer. */
       if ((char *) p > end)
         {
-          if (MBS_SUPPORT && d->mb_cur_max > 1)
+          if (d->mb_cur_max > 1)
             {
               free(mblen_buf);
               free(inputwcs);
@@ -3404,8 +3403,8 @@ dfainit (struct dfa *d)
   d->talloc = 1;
   MALLOC(d->tokens, d->talloc);

-#if MBS_SUPPORT
   d->mb_cur_max = MB_CUR_MAX;
+
   if (d->mb_cur_max > 1)
     {
       d->nmultibyte_prop = 1;
@@ -3413,7 +3412,6 @@ dfainit (struct dfa *d)
       d->mbcsets_alloc = 1;
       MALLOC(d->mbcsets, d->mbcsets_alloc);
     }
-#endif
 }

 static void
@@ -3467,7 +3465,7 @@ dfafree (struct dfa *d)
   free(d->charclasses);
   free(d->tokens);

-  if (MBS_SUPPORT && d->mb_cur_max > 1)
+  if (d->mb_cur_max > 1)
     free_mbdata(d);

   for (i = 0; i < d->sindex; ++i) {
--
1.7.7.rc0.362.g5a14