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

[Jeff King <peff@peff.net>]

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

> 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