On Tue, Jul 12, 2022 at 2:55 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> On Tue, Jul 12 2022, Eric Sunshine wrote:
> > I can't tell from your response whether or not you intend to pick up
> > this patch. I don't disagree that older clang may be considered buggy
> > in this regard, but older clang versions still exist in the wild, and
> > we already support them by applying `{{0}}` when appropriate:
> >
> > % git grep -n '{ *{ *0 *} *}'
> > builtin/merge-file.c:31: xmparam_t xmp = {{0}};
>
> Not so fast :) If you check out "next", does compiling
> builtin/merge-file.o there complain on that clang version now? I changed
> this to the "{ 0 }" form.
No, builtin/merge-file.c doesn't compile, and I discovered that just
after sending the email to which you responded. I haven't yet prepared
a patch for that new instance since I don't know if Junio feels
inclined to pick up such a change.
> If not I wonder if this has to do with one of git_zstream being
> typedef'd, or with the first member being an embedded struct (I couldn't
> find another example of that). For the former does the patch at the end
> & "make builtin/unpack-objects.o" make it go away?
No, the patch you included doesn't make the problem go away (even
after I fixed all the "error: must use 'struct' tag to refer to type
'git_zstream'" errors which showed up throughout the project after
applying your patch).
> > builtin/worktree.c:262: struct config_set cs = { { 0 } };
> > oidset.h:25:#define OIDSET_INIT { { 0 } }
> > worktree.c:840: struct config_set cs = { { 0 } };
>
> Uh, and here are some other examples, so those also warn if you make
> them just a "{ 0 }"?
Yes. (Full disclosure: Even though the two worktree-related instances
come from commits by Stolee, I'm pretty sure I'm the one who asked him
to change them from `{0}` to `{{0}}` during review for this very
reason.)
> > so the change made by this patch is in line with existing practice on
> > this project.
>
> It is nice though to be able to use standard C99 consistently, where a
> "{ 0 }" recursively initializes the members, I think that's what your
> clang version is doing, it's just complaining about it.
Agreed, it would be nice to use plain `{0}`.
> Since this is only a warning, and only a practical issue with -Werror I
> wonder if a config.mak.dev change wouldn't be better, i.e. to provide a
> -Wno-missing-braces for this older clang version.
I'm in favor of this. It would, of course, require extra
special-casing for Apple's clang for which the version number bears no
resemblance to reality since Apple invents their own version numbers.
