All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] CodingGuidelines: give deadline for "for (int i = 0; ..."
Date: Thu, 31 Mar 2022 16:58:07 +0200	[thread overview]
Message-ID: <220331.86sfqyp3ed.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <bf44f83b-0d18-8132-58cf-13155bfec40e@gmail.com>


On Thu, Mar 31 2022, Phillip Wood wrote:

> On 31/03/2022 11:10, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Mar 30 2022, Junio C Hamano wrote:
>> 
>>> We raised the weather balloon to see if we can allow the construct
>>> in 44ba10d6 (revision: use C99 declaration of variable in for()
>>> loop, 2021-11-14), which was shipped as a part of Git v2.35.
>>> Document that fact in the coding guidelines, and more importantly,
>>> give ourselves a deadline to revisit and update.
>>>
>>> Let's declare that we will officially adopt the variable declaration
>>> in the initializaiton [...]
>> Typo: initialization.
>> 
>>> part of "for ()" statement this winter, unless we find that a platform
>>> we care about does not grok it.
>> I'd think that waiting a couple of releases would be sufficient for
>> this
>> sort of thing. I.e. contributors to this project already have
>> access/knowledge about a wide variety of compilers, especially the
>> "usual suspects" (mainly MSVC) that have been blockers for using new
>> language features in the past.
>> So I'm in no rush to use this, and the winter deadline sounds fine
>> to
>> me in that regard.
>
> Agreed, I think it is worth waiting so we don't get into a situation
> where we end up having to revert changes that are using the new
> features because we discover they are not supported by a platform we
> care about.
>
>> But on the other hand I think the likelihood that waiting until November
>> v.s. May revealing that a hitherto unknown compiler or platform has
>> issues with a new language feature is vanishingly small.
>> 
>>> A separate weather balloon for C99 as a whole was raised separately
>>> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,
>>> 2021-12-01).  Hopefully, as we find out that all C99 features are OK
>>> on all platforms we care about, we can stop probing the features we
>>> want one-by-one like this
>> Unfortunately this really isn't the case at all, the norm is for
>> compilers to advertise that they support verison X of the standard via
>> these macros when they consider the support "good enough", but while
>> there's still a long list of unimplemented features before they're at
>> 100% support (and most never fully get to 100%).
>> We also need to worry about the stdlib implementation, and not just
>> the
>> compiler, see e.g. the %zu format and MinGW in the exchange at
>> https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/
>> and
>> https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;
>
> That's a good point, it was a surprise to me that the problem is with
> MinGW rather than MSVC.

Yes, thanks a lot for tracking that down.

I wonder if we can supply a compatibility sprintf() shim for that setup,
there's nothing urgent about it, but the verbosity of the casts and
PRIuMAX inline adds up, especially as we've started using size_t more
widely:

	git grep PRIuMAX -- '*.[ch]'

Either by e.g. grabbing the sprintf() shim from say gnulib, or our own
shim that would intercept the "const char *format" for sprintf(), and
pull a trick similar to what we do in strbuf_addftime() to rewrite the
format (of a copied string) on-the-fly.

  reply	other threads:[~2022-03-31 15:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-31  0:09 [PATCH] CodingGuidelines: give deadline for "for (int i = 0; ..." Junio C Hamano
2022-03-31 10:10 ` Ævar Arnfjörð Bjarmason
2022-03-31 14:48   ` Phillip Wood
2022-03-31 14:58     ` Ævar Arnfjörð Bjarmason [this message]
2022-03-31 20:12   ` Junio C Hamano
2022-03-31 21:19     ` brian m. carlson
2022-04-01  9:29       ` Ævar Arnfjörð Bjarmason

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=220331.86sfqyp3ed.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.