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

On Tue, Nov 16 2021, Jeff King wrote:

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

[...]

quoted

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/ (local)