git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH 0/2] Fix syntax errors under clang 11.0.0 on MacOS
Date: Thu, 06 Oct 2022 13:38:43 -0700	[thread overview]
Message-ID: <xmqqh70gk7qk.fsf@gitster.g> (raw)
In-Reply-To: <pull.1375.git.1665085395.gitgitgadget@gmail.com> (Jeff Hostetler via GitGitGadget's message of "Thu, 06 Oct 2022 19:43:13 +0000")

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This patch series fixes three syntax errors that caused compiler errors with
> clang 11.0.0 on MacOS. I've included the error/warning messages in the
> commit messages. The offending statements did compile successfully under
> clang 14.0.0 on MacOS, so I have to assume that this usage is newer than
> what clang 11 supports.

Ah, this looked familiar, and the last time we saw the one in
builtin/unpack-objects.c

https://lore.kernel.org/git/20220710081135.74964-1-sunshine@sunshineco.com/

It seems that merge-file.c was OK back then but soon after we
"broke" it with 480a0e30 (merge-file: refactor for subsequent memory
leak fix, 2022-07-01).

    $ git grep -n -e '{ *{ *0 *} *}'
    builtin/merge-index.c:15:	char oids[3][GIT_MAX_HEXSZ + 1] = {{0}};
    builtin/merge-index.c:16:	char modes[3][10] = {{0}};
    builtin/worktree.c:321:		struct config_set cs = { { 0 } };
    merge-strategies.c:93:	xmparam_t xmp = {{0}};
    oidset.h:25:#define OIDSET_INIT { { 0 } }
    worktree.c:795:	struct config_set cs = { { 0 } };

I wonder if we want to introduce some named _INIT for these specific
types?  Perhaps with something like

    /* for config_set */
    #define CONFIG_SET_INIT {{0}}
    /* for xmparam_t */
    #define XMPARAM_INIT {{0}}
    /* for mmfile_t */
    #define MMFILE_INIT {0}
    /* for git_zstream */
    #define GIT_ZSTREAM_INIT {{0}}

	Side note: between { { 0 } } and {{0}} forms that appear in
	the existing code, I picked the "unusual spacing" variant,
	i.e. without any space, hoping that it would signal the fact
	that we would prefer if we didn't have to use these to
	please older compilers to the readers.

Unless we plan to soon declare that clang 11 is too old to matter
anymore, that is.

The rule, tentatively until the compilers that need extra levels of
braces die out, can be "if there is <name>_INIT for the type, you
should use it, but otherwise you can do { 0 }", or something like
that, perhaps?


  parent reply	other threads:[~2022-10-06 20:39 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-06 19:43 [PATCH 0/2] Fix syntax errors under clang 11.0.0 on MacOS Jeff Hostetler via GitGitGadget
2022-10-06 19:43 ` [PATCH 1/2] builtin/merge-file: fix compiler error on MacOS with clang 11.0.0 Jeff Hostetler via GitGitGadget
2022-10-06 21:09   ` René Scharfe
2022-10-06 22:30     ` Eric Sunshine
2022-10-06 22:51       ` Junio C Hamano
2022-10-06 19:43 ` [PATCH 2/2] builtin/unpack-objects.c: " Jeff Hostetler via GitGitGadget
2022-10-06 20:38 ` Junio C Hamano [this message]
2022-10-06 20:58   ` [PATCH 0/2] Fix syntax errors under clang 11.0.0 on MacOS Eric Sunshine
2022-10-07 11:02 ` Ævar Arnfjörð Bjarmason
2022-10-07 15:19 ` Jeff Hostetler
2022-10-07 17:49   ` Junio C Hamano
2022-10-07 20:28     ` René Scharfe
2022-10-07 20:56       ` Jeff Hostetler
2022-10-07 21:33         ` Junio C Hamano
2022-10-08  3:46           ` Eric Sunshine
2022-10-08  6:52             ` René Scharfe
2022-10-09  5:19               ` Junio C Hamano
2022-10-07 21:30       ` Junio C Hamano
2022-10-10 15:39 ` [PATCH v2] config.mak.dev: disable suggest braces error on old clang versions Jeff Hostetler via GitGitGadget
2022-10-10 18:13   ` Junio C Hamano
2022-10-11  0:15     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh70gk7qk.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jeffhost@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).