Re: [PATCH] range-diff: fix some 'hdr-check' and sparse warnings

From: Jeff King <peff@peff.net>
Date: Jul 15, 2019, 18:15 UTC
Message-ID: <20190715181527.GA30747@sigill.intra.peff.net>
In-Reply-To: <4be7a0a1-0256-40c3-4abc-d3d9302f527b@kdbg.org>

On Mon, Jul 15, 2019 at 07:30:09PM +0200, Johannes Sixt wrote:

Show 14 quoted lines

> Am 15.07.19 um 16:46 schrieb Jeff King:
> > On Sun, Jul 14, 2019 at 10:30:27AM +0200, Johannes Sixt wrote:
> >>> But it does fall down
> >>> when the first element _has_ to be a struct (like, say, any user of our
> >>> hashmap.[ch] interface).
> >>
> >> No, it does not. It is not necessary to spell out nested structs in the
> >> initializer.
> > 
> > Ah, that is news to me. I know that this compiles OK with "gcc -Wall",
> > but is it guaranteed by the standard?
> 
> Yes; see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf,
> 6.7.8 §20, in particular, the sentence beginning with "Otherwise".

Thanks, I didn't know about that subtlety.

I think relying on that flattening would be a terrible style choice to use for initializing to particular values, but it makes perfect sense in the context of using a single 0 to mean "zero-initialize the whole struct".

That actually kind of makes me think that reordering our struct members (to put an arithmetic type or a struct containing one at the beginning) _might_ be worth doing as a workaround to tools like sparse. It's hacky, but it puts the effort of dealing with this problem only in one spot (the struct definition) instead of many (everywhere the struct is used).

But I'd be happy if we could address it in another way (e.g., convincing sparse to stop complaining about it, or just decide it's not worth dealing with). One other thing I haven't seen discussed in this thread: we could actually make our preferred style be for all structs to define a FOO_INIT initializer, like we already do for STRBUF_INIT, etc. That's a much more heavyweight solution than what's being discussed here, but it comes with an extra benefit: it's easy to catch and change all users if the struct switches away from zero-initialization.

I'm on the fence on whether having non-zero initializers is a good strategy overall. You can always bootstrap any other on-demand setup from the zero-initialized state, but it does sometimes lead to more awkward code (e.g., needing an explicit call to initialized_if_needed() in every function that works with the struct, or inverting the sense of boolean members so that the default is always "0").

-Peff