Re: dfa/MBS_SUPPORT cleanup

[Paolo Bonzini]

On 09/15/2011 12:27 PM, Jim Meyering wrote:
> This series removes many #if MBS_SUPPORT guards and
> converts many others to "if (MBS_SUPPORT ...", with the
> assurance that MBS_SUPPORT is now defined to 0 or 1.

Great work.  I replied with a few notes, I'll summarize them here: some 
reordering/squashing of patches, a couple of nits (patch 4, 13), a 
missing bit (patch 21), two patches to be dropped for now (22 and 34), 
some extra cleanups that were missed (patch 4, 8).

> [01/34] maint: convert #if-MBS_SUPPORT (kwsinit)

Please fix the problem that Stefano found.

> [02/34] maint: convert #if-MBS_SUPPORT (Fcompile, etc.)
> [03/34] maint: convert #if-MBS_SUPPORT (kwsincr_case)
> [04/34] maint: convert #if-MBS_SUPPORT (EGexecute)

Suggested a small change in semantics in the interest of readability.  I 
would do it here.

> [05/34] maint: remove #if-MBS_SUPPORT declaration guards
> [06/34] maint: dfa: convert #if-MBS_SUPPORT
> [07/34] maint: dfa: convert #if-MBS_SUPPORT (dfafree)

More cleanups possible.  Can be done here or in a separate patch.

> [08/34] maint: dfa: convert #if-MBS_SUPPORT (dfaoptimize,

More cleanups possible, but should be done in a separate patch.

> [09/34] maint: dfa: convert #if-MBS_SUPPORT (free_mbdata)
> [10/34] maint: dfa: convert #if-MBS_SUPPORT (dfaexec)
> [11/34] maint: dfa: convert #if-MBS_SUPPORT (dfaexec)

Squash in part of patch 27 (build: enable compilation without MBS_SUPPORT).

> [12/34] maint: dfa: convert #if-MBS_SUPPORT
> [13/34] maint: dfa: convert #if-MBS_SUPPORT

Test MB_CUR_MAX == 1 rather than <= 1.

> [14/34] maint: dfa: convert #if-MBS_SUPPORT
> [15/34] maint: dfa: convert #if-MBS_SUPPORT (lex)
> [16/34] maint: dfa: convert #if-MBS_SUPPORT (copytoks)

Squash in part of patch 27 (build: enable compilation without MBS_SUPPORT).

> [17/34] maint: dfa: convert #if-MBS_SUPPORT (dfaparse)
> [18/34] maint: dfa: convert #if-MBS_SUPPORT (state_index)
> [19/34] maint: dfa: convert #if-MBS_SUPPORT (dfastate)
> [20/34] maint: dfa: convert #if-MBS_SUPPORT (dfastate)

Some cleanups possible, either here or together with patch 30.

> [21/34] maint: dfa: remove some #if MBS_SUPPORT guards

Introduces an if that should be later removed.

> [22/34] maint: ensure that MB_CUR_MAX is defined even when

Please drop it for now.  Also the commit message is misleading.

> [23/34] tests: make fmbtest explain (to stderr, not log) why

Please move early.

> [24/34] maint: dfa: remove #if MBS_SUPPORT around struct

Please move early, though after the tests patches.  Same for all patches 
that remove #if MBS_SUPPORT around struct/member/enum definitions.

> [25/34] maint: remove #if MBS_SUPPORT around member

Please move early, though after the tests patches.

> [26/34] tests: skip tests that require MBS support

Please move early.

> [27/34] build: enable compilation without MBS_SUPPORT

Should be squashed in earlier patches.

> [28/34] maint: dfa: avoid in-function "#if MBS_SUPPORT" tests
> [29/34] maint: stop using skip_test_; use skip_ instead
> [30/34] maint: dfa: simplify several expressions

Small change to account for patch 22 being delayed.

> [31/34] tests: skip sjis-mb when dfa.c is compiled without MB

Should be squashed in patch 26 (tests: skip tests that require MBS 
support), and moved early together with that one.

> [32/34] build: allow compilation once again with ! MBS_SUPPORT

Should also be moved early.

> [33/34] maint: dfa: simplify multi-byte-related conditionals

Please leave this one aside for now, together with patch 22.

> [34/34] maint: dfa: remove case-guarding #if-MBS_SUPPORT

Also move this one early, though after the tests patches.

Paolo