Re: [PATCH v2 3/3] diff-filter: be more careful when looking for negative bits

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Thu, 27 Jan 2022, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/diff.c b/diff.c
> > index 5081052c431..4ab4299b817 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -4720,6 +4720,12 @@ void diff_setup_done(struct diff_options *options)
> >  	if (!options->use_color || external_diff())
> >  		options->color_moved = 0;
> >
> > +	if (options->filter_not) {
> > +		if (!options->filter)
> > +			options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];
>
> Unlike the original, options->filter will have excess higher bit all
> on, in addition to all the filter bits except for the all-or-none
> bit.

Thank you for your thoroughness. Indeed, you are correct that I no longer
do the `(1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1` dance.

In an uncommitted iteration, I actuall forced that mask in
`diff_setup_done()` via:

	options->filter &= (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;

But then I got curious and looked how `options->filter` is accessed, and
we never loop over its bits, we always ask whether a specific bit is set.
So I got rid of that (quite ugly) line.

I made a mental note to write about this in the commit message, but wanted
to quickly look at all the code accessing `options->filter` by using the
very nice refactoring support of VS Code to rename the field so that all
of the accesses would be visible in a diff, and then I wanted to quickly
run the entire test suite first, just in case my analysis missed
something. And by the end of it, my mental note had evaporated.

Thanks to your reminder, I added this to the end of the commit message:

    Note: The code replaced by this commit took pains to avoid setting any
    unused bits of `options->filter`. That was unnecessary, though, as all
    accesses happen via the `filter_bit_tst()` function using specific bits,
    and setting the unused bits has no effect. Therefore, we can simplify
    the code by using `~0` (or in this instance, `~<unwanted-bit>`).

Ciao,
Dscho