Re: [PATCH] revisions.c: put promisor option in specialized struct

From: Matthew DeVore <matvore@comcast.net>
To: Jeff King <peff@peff.net>, Matthew DeVore <matvore@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com, pclouds@gmail.com,
	jonathantanmy@google.com, jeffhost@microsoft.com
Subject: Re: [PATCH] revisions.c: put promisor option in specialized struct
Date: Mon, 3 Dec 2018 14:01:12 -0800	[thread overview]
Message-ID: <0809e859-b3a6-79ef-031c-9ff3326da31d@comcast.net> (raw)
In-Reply-To: <20181203212431.GB8700@sigill.intra.peff.net>

On 12/03/2018 01:24 PM, Jeff King wrote:
>> @@ -297,7 +296,8 @@ struct setup_revision_opt {
>>   	const char *def;
>>   	void (*tweak)(struct rev_info *, struct setup_revision_opt *);
>>   	const char *submodule;	/* TODO: drop this and use rev_info->repo */
>> -	int assume_dashdash;
>> +	int assume_dashdash : 1;
>> +	int allow_exclude_promisor_objects : 1;
>>   	unsigned revarg_opt;
>>   };
> 
> I don't know that we need to penny-pinch bytes in this struct, but in
> general it shouldn't hurt either awy. However, a signed bit-field with 1
> bit is funny. I'm not even sure what the standard has to say, but in
> twos-complement that would store "-1" and "0" (gcc -Wpedantic also
> complains about overflow in assigning "1" to it).
Interesting. I hadn't suspected this. But I confirmed it with this:

#include <stdio.h>

struct x {
   int y : 1;
   int z : 1;
};

int main() {
   struct x x;
   x.y = 1;
   x.z = 1;
   printf("%d %d\n", (int) x.y, (int) x.z);
   return 0;
}

-- Output --
-1 -1

> 
> So this probably ought to be "unsigned".

Earlier in this file we define bit fields this way:
	/* Traversal flags */
	unsigned int	dense:1,
			prune:1,

... using \t to align the field names, so I'll mimic that style.