Re: [PATCH v3 2/8] diff: convert flags to be stored in bitfields

[Stefan Beller <sbeller@google.com>]

On Tue, Oct 31, 2017 at 6:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> After some quick research our coding style on bit fields is twofold:
>> Most older code is this way and more recent code seems to prefer
>>
>>     unsigned <FLAGNAME> SP : SP ;
>
> Yes, we are very inconsistent.  What does the clang format rules
> Brandon came up with have to say on this?  FWIW, checkpatch.pl is
> unhappy without spaces on both side.

clang-format --style file diff.h

...
#define DIFF_FLAGS_INIT \
{               \
                0       \
}
struct diff_flags {
        unsigned recursive : 1;
        unsigned tree_in_recursive : 1;
        unsigned binary : 1;
        unsigned text : 1;
...


>>> +       for (i = 0; i < sizeof(struct diff_flags); i++)
>>
>> I think most of the code prefers to put the variable into the sizeof
>> argument i.e. 'sizeof(*a)', as that is presumably more maintainable.
>> (If the type of 'a' changes, then we don't forget to adapt this place,
>> but the compiler can take care of it.
>
> Yup, but in this case we won't change the type, no?

Most likely. If we were to change the type we'd have to rename the function
and probably rewrite the body, too. I just mentioned it from a consistency
point of view. (exceptions are both a mental burden to humans as well
as to machines using .clang-format et al. The fewer "this time is different"
calls we have, the better)