Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag <snip>

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 7, 2019, 17:12 UTC
Message-ID: <20191007171249.GB11529@szeder.dev>
In-Reply-To: <xmqqsgobg0rv.fsf@gitster-ct.c.googlers.com>

On Wed, Oct 02, 2019 at 02:43:48PM +0900, Junio C Hamano wrote:

Show 14 quoted lines

> William Baker <williamtbakeremail@gmail.com> writes:
> 
> > Although my debugger might not be the smartest, I haven't noticed any
> > downsides to switching this to an enum.
> 
> Well, if you write
> 
> 	enum { BIT_0 = 1, BIT_1 = 2, BIT_3 = 4 } var;
> 
> it's pretty much a promise that the normal value for the var is one
> of these listed values to your readers.  But bit flags are meant to
> be used combined (after all, they are cheaper alternative for 1-bit
> wide bitfields in a structure), so it is misleading to use enum as
> such.

Having the combination of enum constants with power-of-two values is not misleading, but rather an idiom.

Back when debugging the racy split index issues I only saw gibberish like this:

  (gdb) p ce->ce_flags 
  $2 = 469762048

With an enum I now have this instead:

  (gdb) p ce->ce_flags
  $2 = (CE_MATCHED | CE_UPDATE_IN_BASE | CE_STRIP_NAME)

The latter is about as many times more readable as the int value of that 'ce_flags'.

The sooner everyone gets on board with this the better.