On 30/05/2019 13:04, Jeff King wrote:
> On Thu, May 30, 2019 at 10:47:37AM +0200, Johannes Sixt wrote:
>
>> I had a brief look at the series. IMO, it is a mistake to appease
>> -Wmissing-field-initializer.
>>
>> We have two sorts of initializers:
>>
>> - zero initializers: they just want to null out every field,
>> like CHILD_PROCESS_INIT and ad-hoc initializers of structs
>> such as xpparam_t pp = { 0 }; in range-diff.c
>>
>> - value initializers are always macros, such as STRING_LIST_INIT_DUP
>> and the OPT_* family.
>>
>> I am strongly against forcing zero initializers to write down a value
>> for every field. It is much more preferable to depend on that the
>> compiler does the right thing with them. -Wmissing-field-initializer
>> would provide guidance in the wrong direction. A zero initializer looks
>> like this: = { 0 }; and nothing else.
>
> I had a similar impression while perusing the commits. I don't mind
> forcing some extra work on programmers to appease a warning if
> disregarding it is a common source of errors. But I didn't see any real
> bug-fixes in the series, so it doesn't seem like that good a tradeoff to
> me.
>
> Contrast that with the -Wunused-parameters warning. I found a dozen or
> so actual bugs by sifting through the results, and another couple dozen
> spots where the code could be cleaned up or simplified. If we want to
> shut up the warning completely (so we can pay attention to it), we'll
> then have to annotate probably a couple hundred spots, and keep those
> annotations up to date. But I feel better doing that knowing that it's
> shown real-world value.
OK, I will drop this branch then.
Thanks all.
ATB,
Ramsay Jones
