On Tue, Jul 12, 2022 at 2:55 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Tue, Jul 12 2022, Eric Sunshine wrote:

quoted

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).

quoted

    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.)

quoted

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.