Re: [PATCH v2 1/1] git-compat-util: add a test balloon for C99 support

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 16, 2021, 12:54 UTC
Message-ID: <211116.86pmr0p82k.gmgdl@evledraar.gmail.com>
In-Reply-To: <YZOh370ZMMqSADUE@coredump.intra.peff.net>

On Tue, Nov 16 2021, Jeff King wrote:

> On Tue, Nov 16, 2021 at 02:12:41AM +0000, brian m. carlson wrote:

[...]

Show 26 quoted lines

>> diff --git a/Makefile b/Makefile
>> index 12be39ac49..893d533d22 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1204,7 +1204,7 @@ endif
>>  # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be
>>  # tweaked by config.* below as well as the command-line, both of
>>  # which'll override these defaults.
>> -CFLAGS = -g -O2 -Wall
>> +CFLAGS = -g -O2 -Wall -std=gnu99
>>  LDFLAGS =
>>  CC_LD_DYNPATH = -Wl,-rpath,
>>  BASIC_CFLAGS = -I.
>
> Do most compilers understand -std=gnu99? It seems like we're breaking
> the out-of-the-box build for everything that isn't gcc or clang.
>
> I understand that older versions of gcc (prior to 5.1.0, from my
> digging) default to gnu89, and so they would be broken _without_ this.
> So it is a tradeoff one way or the other. But somehow this seems
> backwards to me. We should assume that modern compilers support C99 out
> of the box, and put the burden on older ones to trigger C99 support in
> whatever non-portable way they need.
>
> I also checked clang, and it looks like it has defaulted to gnu11 since
> clang-7 (I'm not sure about clang-6; its documentation wasn't clear).

Yes, this seems like something we'd really want to feed to "detect-compiler" after extracting it out of config.mak.dev.

And as you note it's not only that older or non-gcc non-clang compilers may not understand this at all, but are we getting worse behavior on modern versions of those two because they're forced into some 20 year old C99 standard mode, instead of the current preferred default?

If we do go for this there's also the additional breakage I mentioned upthread of th [1], and which I think I wasn't clear enough about. I.e. if we do:

    CFLAGS = -O2 -g -std=whatever

And then in config.mak.uname do e.g.:

    CFLAGS = -g -O0 -Winline

We'll override the -std=whatever, but just wanted to overrride the -O2.

This came up in another context recently (Carlo had series to hack in this area), but I think we shouldn't add anything new to CFLAGS. Let's instead add a new variable like BASIC_CFLAGS, so in this case CFLAGS_C_STANDARD or whatever, and then later in the file:

    -ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
    +ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_C_STANDARD)

But IMO this whole thing of trying to make this work on every compiler etc. just isn't worth it.

Let's just start using C99 features, and if anyone's compiler breaks on something like CentOS 6 document that they'll need to tweak a flag somewhere. We already know that works for all the other C99 features we have, there seems to just be this one exception of the ancient GCC version in this particular case.

1. https://lore.kernel.org/git/YZG9wF56unj7eYhl@camp.crustytoothpaste.net/