Re: [PATCH 0/2] Fix syntax errors under clang 11.0.0 on MacOS

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 7, 2022, 17:49 UTC
Message-ID: <xmqqpmf3frr5.fsf@gitster.g>
In-Reply-To: <0f67ca77-f17f-d844-e689-ca9a9bdf7993@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 10 quoted lines

> I didn't realize that these variable initialization fixes would
> spawn such a large response here [1].  Nor that it had been already
> discussed in great length in [2] in July.
>
> I'm not sure how to best proceed here.
>
> * I'm not sure these fixes are important enough to warrant the
>   engineering time to hack up the Makefile or config.mak.uname
>   to conditionally turn off -Wno-missing-braces based on some
>   {platform-os-version, gcc/clang-version} combination.

Developers are expected to be able to cope, but if this something a few easy lines in config.mak.uname can help, we'd better do so, instead of having dozens of folks on the macOS add the same line to their config.mak file.

Show 5 quoted lines

> * While -Wno-missing-braces option may prevent the warning/error
>   (depending on -Werror) for these "{0}" should be "{{0}}"
>   errors, do we know that this won't hide real problems.  (I mean
>   we tend to only see it for these {0} / {{0}} false alarms, but
>   I'd hate to lose the protection for non-false alarms.)

It may find real problems, but folks on other platforms are running without the check disabled, so we should be OK. More importantly, folks with a more recent compilers on macOS are running with the check, so we should be able to give a reasonable coverage to platform specific code, too.

Show 9 quoted lines

> * The suggestion to use a <type>_INIT macro to hide the {0} or
>   {{0}} may help in the:
>         xmparam_t xmp = XMPARAM_INIT;
>   case, but in the `mmfile_t mmfs[3]` case, we have an array of
>   that type, so we'd need something like:
>      mmfile_t mmfs[3] = { MMFILE_INIT }; or
>      mmfile_t mmfs[3] = MMFILE_INIT_ARRAY;
>   for the macros to make sense.
>   I'm not sure either of these two is better than just writing "{{0}}".

I do not like that suggestion at all. I think it is the right approach to use -Wno-missing-braces with older and buggy compilers until they die out.

> * I wasn't sure which compiler versions we *want* to support or
>   want to drop support for.
>   * I've only thought about it in the context of clang on MacOS.

I think that is OK. IIRC, we have more or less good grasp on the cut-off version of clang as the upstream calls it, but the problem is clang shipped with macOS lies and gives a version number more recent than it actually is.

> * I'm not the first one who has stumbled over this and had to
>   rediscover the solution.  So I'd hate to just kick this down
>   the road some more, but then again I'd hate to waste a lot of
>   time on it -- for very little actual functional value.

Exactly.

> * Is "{{0}}" really that ugly ???

Ugly is not the issue. Folks on more recent platforms not being able to tell when to use the extra parentheses is, because their compilers do not have this bug.

My preference is to flip the -Wno-missing-braces bit in config.mak.uname only for folks who use the version of clang on macOS when that clang claims to be clang11 (my understanding of René's experiment[*] is that versions of (real) clang 9 or newer perfectly well understand that {0} is an accpetable way to specify zero initialization for any structure, with possible nesting).

[Reference]

* https://lore.kernel.org/git/36cd156b-edb2-062c-9422-bf39aad39a6d@web.de/