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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Nov 14 2021, brian m. carlson wrote:

> The C99 standard was released in January 1999, now 22 years ago.  It
> provides a variety of useful features, including variadic arguments for
> macros, declarations after statements, variable length arrays, and a
> wide variety of other useful features, many of which we already use.
>
> We'd like to take advantage of these features, but we want to be
> cautious.  As far as we know, all major compilers now support C99 or a
> later C standard, such as C11 or C17.  POSIX has required C99 support as
> a requirement for the 2001 revision, so we can safely assume any POSIX
> system which we are interested in supporting has C99.

I like this direction.

> Sparse is also updated with a reference to the gnu99 standard, without
> which it defaults to C89.

Do we really need it in SPARSE_FLAGS though...

> @@ -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.
> @@ -1215,7 +1215,7 @@ ARFLAGS = rcs
>  PTHREAD_CFLAGS =

Since $(CFLAGS) ends up in:

    ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)

...

>  # For the 'sparse' target
> -SPARSE_FLAGS ?=
> +SPARSE_FLAGS ?= -std=gnu99
>  SP_EXTRA_FLAGS = -Wno-universal-initializer

... and this will be used for this rule:

$(SP_OBJ): %.sp: %.c %.o
        $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
                -Wsparse-error \
                $(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< [...]

I.e. unless it needs to be later on the command-line the $(ALL_CFLAGS)
should put it there already.

Also (and this pre-dates this patch) it's unfortunate that CFLAGS is a
mixed bag of compiler tweaking and "mandatory" flags. I think the below
would be a better approach, particurly since our own config.mak.uname
will override CFLAGS in some cases, and probably everyone who works on
git to any degree has a local config.mak which sets it to something
already.

But why gnu99 and not c99?

diff --git a/Makefile b/Makefile
index 12be39ac497..7470d627165 100644
--- a/Makefile
+++ b/Makefile
@@ -1204,10 +1204,14 @@ 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.
+#
+# The MANDATORY_CFLAGS can be similarly overridden, but really
+# shouldn't.
 CFLAGS = -g -O2 -Wall
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
-BASIC_CFLAGS = -I.
+BASIC_CFLAGS =
+MANDATORY_CFLAGS = -I. -std=gnu99
 BASIC_LDFLAGS =
 
 # library flags
@@ -1249,7 +1253,7 @@ ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)
 ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)
 endif
 
-ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
+ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(MANDATORY_CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
 comma := ,