[PATCH 0/5] CodingGuidelines: various C99 updates

[PATCH 0/5] CodingGuidelines: various C99 updates

[Ævar Arnfjörð Bjarmason]

This series:

 * Rephrases CodingGuidelines so that we note we're on C99, and then
   lists exceptions and features we use. The previous prose assumed
   C89 by default.

   We still explicitly avoid opening the "feel free to use an C99
   feature" floodgates.

 * Mentions that you can use dynamic C99 initializer elements. See the
   recent discussion at
   https://lore.kernel.org/git/221006.86a668r5mf.gmgdl@evledraar.gmail.com/

 * Allows us to use "for (int i". I didn't set out to (slightly) jump
   the gun on this, but just pulling the trigger around ~20 days early
   makes it easier to ...

 * ...add the natural follow-up section of C99 features you explicitly
   shouldn't be using yet, to which I added the two cases I could
   remember (in 4-5/5).

Ævar Arnfjörð Bjarmason (5):
  CodingGuidelines: update for C99
  CodingGuidelines: mention dynamic C99 initializer elements
  CodingGuidelines: allow declaring variables in for loops
  CodingGuidelines: mention C99 features we can't use
  CodingGuidelines: recommend against unportable C99 struct syntax

 Documentation/CodingGuidelines | 34 ++++++++++++++++++++++++----------
 revision.c                     |  7 -------
 t/helper/test-parse-options.c  |  3 +--
 3 files changed, 25 insertions(+), 19 deletions(-)

-- 
2.38.0.971.ge79ff6d20e7

[PATCH 1/5] CodingGuidelines: update for C99

[Ævar Arnfjörð Bjarmason]

Since 7bc341e21b5 (git-compat-util: add a test balloon for C99
support, 2021-12-01) we've had a hard dependency on C99, but the prose
in CodingGuidelines was written under the assumption that we were
using C89 with a few C99 features.

As the updated prose notes we'd still like to hold off on novel C99
features, but let's make it clear that we target that C version, and
then enumerate new C99 features that are safe to use.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/CodingGuidelines | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 9fca21cc5f9..386ca0a0d22 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -204,10 +204,14 @@ For C programs:
    by e.g. "echo DEVELOPER=1 >>config.mak".
 
  - We try to support a wide range of C compilers to compile Git with,
-   including old ones.  You should not use features from newer C
+   including old ones.  As of Git v2.35.0 Git requires C99 (we check
+   "__STDC_VERSION__"). You should not use features from a newer C
    standard, even if your compiler groks them.
 
-   There are a few exceptions to this guideline:
+   New C99 features have been phased in gradually, if something's new
+   in C99 but not used yet don't assume that it's safe to use, some
+   compilers we target have only partial support for it. These are
+   considered safe to use:
 
    . since early 2012 with e1327023ea, we have been using an enum
      definition whose last element is followed by a comma.  This, like
-- 
2.38.0.971.ge79ff6d20e7

Re: [PATCH 1/5] CodingGuidelines: update for C99

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Since 7bc341e21b5 (git-compat-util: add a test balloon for C99
> support, 2021-12-01) we've had a hard dependency on C99, but the prose
> in CodingGuidelines was written under the assumption that we were
> using C89 with a few C99 features.
>
> As the updated prose notes we'd still like to hold off on novel C99
> features, but let's make it clear that we target that C version, and
> then enumerate new C99 features that are safe to use.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/CodingGuidelines | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 9fca21cc5f9..386ca0a0d22 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -204,10 +204,14 @@ For C programs:
>     by e.g. "echo DEVELOPER=1 >>config.mak".
>  
>   - We try to support a wide range of C compilers to compile Git with,
> -   including old ones.  You should not use features from newer C
> +   including old ones.  As of Git v2.35.0 Git requires C99 (we check
> +   "__STDC_VERSION__"). You should not use features from a newer C
>     standard, even if your compiler groks them.
>  
> -   There are a few exceptions to this guideline:
> +   New C99 features have been phased in gradually, if something's new
> +   in C99 but not used yet don't assume that it's safe to use, some
> +   compilers we target have only partial support for it. These are
> +   considered safe to use:
>  
>     . since early 2012 with e1327023ea, we have been using an enum
>       definition whose last element is followed by a comma.  This, like

Looking good.

[PATCH 2/5] CodingGuidelines: mention dynamic C99 initializer elements

[Ævar Arnfjörð Bjarmason]

The first use of variables in initializer elements appears to have
been 2b6854c863a (Cleanup variables in cat-file, 2007-04-21) released
with v1.5.2.

Some of those caused portability issues, and e.g. that "cat-file" use
was changed in 66dbfd55e38 (Rewrite dynamic structure initializations
to runtime assignment, 2010-05-14) which went out with v1.7.2.

But curiously 66dbfd55e38 missed some of them, e.g. an archive.c use
added in d5f53d6d6f2 (archive: complain about path specs that don't
match anything, 2009-12-12), and another one in merge-index.c (later
builtin/merge-index.c) in 0077138cd9d (Simplify some instances of
run_command() by using run_command_v_opt()., 2009-06-08).

As far as I can tell there's been no point since 2b6854c863a in 2007
where a compiler that didn't support this has been able to compile
git. Presumably 66dbfd55e38 was an attempt to make headway with wider
portability that ultimately wasn't completed.

In any case, we are thoroughly reliant on this syntax at this point,
so let's update the guidelines, see
https://lore.kernel.org/git/xmqqy1tunjgp.fsf@gitster.g/ for the
initial discussion.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/CodingGuidelines | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 386ca0a0d22..8afda28cfce 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -213,6 +213,11 @@ For C programs:
    compilers we target have only partial support for it. These are
    considered safe to use:
 
+   . since around 2007 with 2b6854c863a, we have been using
+     initializer elements which are not computable at load time. E.g.:
+
+	const char *args[] = {"constant", variable, NULL};
+
    . since early 2012 with e1327023ea, we have been using an enum
      definition whose last element is followed by a comma.  This, like
      an array initializer that ends with a trailing comma, can be used
-- 
2.38.0.971.ge79ff6d20e7

[PATCH 3/5] CodingGuidelines: allow declaring variables in for loops

[Ævar Arnfjörð Bjarmason]

Since 44ba10d6712 (revision: use C99 declaration of variable in for()
loop, 2021-11-14) released with v2.35.0 we've had a variable declared
with in a for loop.

Since then we've had inadvertent follow-ups to that with at least
cb2607759e2 (merge-ort: store more specific conflict information,
2022-06-18) released with v2.38.0.

As November 2022 is within the window of this upcoming release let's
update the guideline to allow this, and revert the recent
6983f4e3b20 (test-parse-options.c: don't use for loop initial
declaration, 2022-09-05).

It's better to update the guidelines than to have back & forth churn
like that, we clearly don't have portability issues related to this
syntax.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/CodingGuidelines | 10 ++--------
 revision.c                     |  7 -------
 t/helper/test-parse-options.c  |  3 +--
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 8afda28cfce..f9affc4050a 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -232,18 +232,12 @@ For C programs:
    . since early 2021 with 765dc168882, we have been using variadic
      macros, mostly for printf-like trace and debug macros.
 
-   These used to be forbidden, but we have not heard any breakage
-   report, and they are assumed to be safe.
+   . since late 2021 with 44ba10d6, we have had variables declared in
+     the for loop "for (int i = 0; i < 10; i++)".
 
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
- - Declaring a variable in the for loop "for (int i = 0; i < 10; i++)"
-   is still not allowed in this codebase.  We are in the process of
-   allowing it by waiting to see that 44ba10d6 (revision: use C99
-   declaration of variable in for() loop, 2021-11-14) does not get
-   complaints.  Let's revisit this around November 2022.
-
  - NULL pointers shall be written as NULL, not as 0.
 
  - When declaring pointers, the star sides with the variable
diff --git a/revision.c b/revision.c
index 36e31942cee..8f2623b3b5a 100644
--- a/revision.c
+++ b/revision.c
@@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);
 void show_object_with_name(FILE *out, struct object *obj, const char *name)
 {
 	fprintf(out, "%s ", oid_to_hex(&obj->oid));
-	/*
-	 * This "for (const char *p = ..." is made as a first step towards
-	 * making use of such declarations elsewhere in our codebase.  If
-	 * it causes compilation problems on your platform, please report
-	 * it to the Git mailing list at git@vger.kernel.org. In the meantime,
-	 * adding -std=gnu99 to CFLAGS may help if you are with older GCC.
-	 */
 	for (const char *p = name; *p && *p != '\n'; p++)
 		fputc(*p, out);
 	fputc('\n', out);
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 506835521a4..f8a62d892d9 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -195,8 +195,7 @@ int cmd__parse_options(int argc, const char **argv)
 
 static void print_args(int argc, const char **argv)
 {
-	int i;
-	for (i = 0; i < argc; i++)
+	for (int i = 0; i < argc; i++)
 		printf("arg %02d: %s\n", i, argv[i]);
 }
 
-- 
2.38.0.971.ge79ff6d20e7

Re: [PATCH 3/5] CodingGuidelines: allow declaring variables in for loops

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/revision.c b/revision.c
> index 36e31942cee..8f2623b3b5a 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);
>  void show_object_with_name(FILE *out, struct object *obj, const char *name)
>  {
>  	fprintf(out, "%s ", oid_to_hex(&obj->oid));
> -	/*
> -	 * This "for (const char *p = ..." is made as a first step towards
> -	 * making use of such declarations elsewhere in our codebase.  If
> -	 * it causes compilation problems on your platform, please report
> -	 * it to the Git mailing list at git@vger.kernel.org. In the meantime,
> -	 * adding -std=gnu99 to CFLAGS may help if you are with older GCC.
> -	 */
>  	for (const char *p = name; *p && *p != '\n'; p++)
>  		fputc(*p, out);
>  	fputc('\n', out);

Good.  Thanks for not forgetting to remove this.

> diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
> index 506835521a4..f8a62d892d9 100644
> --- a/t/helper/test-parse-options.c
> +++ b/t/helper/test-parse-options.c
> @@ -195,8 +195,7 @@ int cmd__parse_options(int argc, const char **argv)
>  
>  static void print_args(int argc, const char **argv)
>  {
> -	int i;
> -	for (i = 0; i < argc; i++)
> +	for (int i = 0; i < argc; i++)
>  		printf("arg %02d: %s\n", i, argv[i]);
>  }

This does not belong to this patch.  If the current code as written
is acceptable, it is not worth patch churn to rewrite it.

[PATCH 4/5] CodingGuidelines: mention C99 features we can't use

[Ævar Arnfjörð Bjarmason]

The C99 section of the CodingGuidelines is a good overview of what we
can use, but is sorely lacking in what we can't use. Something that
comes up occasionally is the portability of %z.

Per [1] we couldn't use it for the longest time due to MSVC not
supporting it, but nowadays by requiring C99 we rely on the MSVC
version that does, but we can't use it yet because a C library that
MinGW uses doesn't support it.

1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/CodingGuidelines | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f9affc4050a..893f960231f 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -235,6 +235,13 @@ For C programs:
    . since late 2021 with 44ba10d6, we have had variables declared in
      the for loop "for (int i = 0; i < 10; i++)".
 
+   New C99 features that we cannot use yet:
+
+   . %z and %zu as a printf() argument for a size_t (the %z being for
+     the POSIX-specific ssize_t). Instead you should use
+     printf("%"PRIuMAX, (uintmax_t)v); These days the MSVC version we
+     rely on supports %z, but the C library used by MinGW does not.
+
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
-- 
2.38.0.971.ge79ff6d20e7

Re: [PATCH 4/5] CodingGuidelines: mention C99 features we can't use

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> The C99 section of the CodingGuidelines is a good overview of what we
> can use, but is sorely lacking in what we can't use. Something that
> comes up occasionally is the portability of %z.
>
> Per [1] we couldn't use it for the longest time due to MSVC not
> supporting it, but nowadays by requiring C99 we rely on the MSVC
> version that does, but we can't use it yet because a C library that
> MinGW uses doesn't support it.
>
> 1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/CodingGuidelines | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index f9affc4050a..893f960231f 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -235,6 +235,13 @@ For C programs:
>     . since late 2021 with 44ba10d6, we have had variables declared in
>       the for loop "for (int i = 0; i < 10; i++)".
>  
> +   New C99 features that we cannot use yet:
> +
> +   . %z and %zu as a printf() argument for a size_t (the %z being for
> +     the POSIX-specific ssize_t). Instead you should use
> +     printf("%"PRIuMAX, (uintmax_t)v); These days the MSVC version we
> +     rely on supports %z, but the C library used by MinGW does not.
> +

Good.  s/; These days/; these days/, though.

[PATCH 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Ævar Arnfjörð Bjarmason]

Per 33665d98e6b (reftable: make assignments portable to AIX xlc
v12.01, 2022-03-28) forms like ".a.b = *c" can be replaced by using
".a = { .b = *c }" instead.

We'll probably allow these sooner than later, but since the workaround
is trivial let's note it among the C99 features we'd like to hold off
on for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/CodingGuidelines | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 893f960231f..65b608ca0a2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -242,6 +242,10 @@ For C programs:
      printf("%"PRIuMAX, (uintmax_t)v); These days the MSVC version we
      rely on supports %z, but the C library used by MinGW does not.
 
+   . Shorthand like ".a.b = *c" in struct assignments is known to trip
+     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
+     the 33665d98e6b portability fix from mid-2022.
+
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
-- 
2.38.0.971.ge79ff6d20e7

Re: [PATCH 0/5] CodingGuidelines: various C99 updates

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This series:
>
>  * Rephrases CodingGuidelines so that we note we're on C99, and then
>    lists exceptions and features we use. The previous prose assumed
>    C89 by default.
>
>    We still explicitly avoid opening the "feel free to use an C99
>    feature" floodgates.

The above contradicts with each other.  A sensible position to
support the "we do not open the floodgate" is that when in doubt
stick to C89 but use C99 features that are explicitly allowed.

>  * Mentions that you can use dynamic C99 initializer elements. See the
>    recent discussion at
>    https://lore.kernel.org/git/221006.86a668r5mf.gmgdl@evledraar.gmail.com/

Good.

>  * Allows us to use "for (int i". I didn't set out to (slightly) jump
>    the gun on this, but just pulling the trigger around ~20 days early
>    makes it easier to ...

This is a welcome change.  As anything this set of patches won't
become reality in any released version until mid December anyway,
this is the cycle to "revisit around November 2022".

>  * ...add the natural follow-up section of C99 features you explicitly
>    shouldn't be using yet, to which I added the two cases I could
>    remember (in 4-5/5).

And we do not have to say we do not use these from C99 if our base
is C89, with explicitly allowed features from C99.

Re: [PATCH 0/5] CodingGuidelines: various C99 updates

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This series:
>>
>>  * Rephrases CodingGuidelines so that we note we're on C99, and then
>>    lists exceptions and features we use. The previous prose assumed
>>    C89 by default.
>>
>>    We still explicitly avoid opening the "feel free to use an C99
>>    feature" floodgates.
>
> The above contradicts with each other.  A sensible position to
> support the "we do not open the floodgate" is that when in doubt
> stick to C89 but use C99 features that are explicitly allowed.

Ah, I think I misread the intention, which 1/5 clarified.  I think
"even though we require your CC claim to support C99, you are
expected to stick to C89 plus those features that are explicitly
allowed" is a very sensible thing to day.

[PATCH v2 0/5] CodingGUidelines: C99 updates

[Junio C Hamano]

As this is otherwise clearly good series, I got sick of waiting for
a trivial update, so here is what I did to update them myself.

Updates are in patches 3 and 4.  We promised to revisit the issue of
allowing "for (int i = 0; ..." in November this year, so we are
merely "proposing" to go ahead and allow it with this patch, and
will merge only after gaining consensus.  The wording has been
redone to avoid sounding like it is a fait accompli.

Patch 4 has been updated to avoid starting the latter clause with a
capitalized word in two independent clauses joined with a semicolon.


Ævar Arnfjörð Bjarmason (5):
  CodingGuidelines: update for C99
  CodingGuidelines: mention dynamic C99 initializer elements
  CodingGuidelines: allow declaring variables in for loops
  CodingGuidelines: mention C99 features we can't use
  CodingGuidelines: recommend against unportable C99 struct syntax

 Documentation/CodingGuidelines | 34 ++++++++++++++++++++++++----------
 revision.c                     |  7 -------
 2 files changed, 24 insertions(+), 17 deletions(-)

Range-diff against v1:
1:  3e3dd9cbd5 = 1:  3e3dd9cbd5 CodingGuidelines: update for C99
2:  fb6c58ef52 = 2:  fb6c58ef52 CodingGuidelines: mention dynamic C99 initializer elements
3:  719e235f9f ! 3:  f3237c88f8 CodingGuidelines: allow declaring variables in for loops
    @@ Commit message
         cb2607759e2 (merge-ort: store more specific conflict information,
         2022-06-18) released with v2.38.0.
     
    -    As November 2022 is within the window of this upcoming release let's
    -    update the guideline to allow this, and revert the recent
    -    6983f4e3b20 (test-parse-options.c: don't use for loop initial
    -    declaration, 2022-09-05).
    -
    -    It's better to update the guidelines than to have back & forth churn
    -    like that, we clearly don't have portability issues related to this
    -    syntax.
    +    As November 2022 is within the window of this upcoming release,
    +    let's update the guideline to allow this.  We can have the promised
    +    "revisit" discussion while this patch cooks, and drop it if it turns
    +    out that it is still premature, which is not expected to happen at
    +    this moment.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
    @@ revision.c: static inline int want_ancestry(const struct rev_info *revs);
      	for (const char *p = name; *p && *p != '\n'; p++)
      		fputc(*p, out);
      	fputc('\n', out);
    -
    - ## t/helper/test-parse-options.c ##
    -@@ t/helper/test-parse-options.c: int cmd__parse_options(int argc, const char **argv)
    - 
    - static void print_args(int argc, const char **argv)
    - {
    --	int i;
    --	for (i = 0; i < argc; i++)
    -+	for (int i = 0; i < argc; i++)
    - 		printf("arg %02d: %s\n", i, argv[i]);
    - }
    - 
4:  3d1edfd825 ! 4:  9d2479220d CodingGuidelines: mention C99 features we can't use
    @@ Documentation/CodingGuidelines: For C programs:
     +
     +   . %z and %zu as a printf() argument for a size_t (the %z being for
     +     the POSIX-specific ssize_t). Instead you should use
    -+     printf("%"PRIuMAX, (uintmax_t)v); These days the MSVC version we
    ++     printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
     +     rely on supports %z, but the C library used by MinGW does not.
     +
       - Variables have to be declared at the beginning of the block, before
5:  139b0f8cfd ! 5:  ef22b1756a CodingGuidelines: recommend against unportable C99 struct syntax
    @@ Commit message
     
      ## Documentation/CodingGuidelines ##
     @@ Documentation/CodingGuidelines: For C programs:
    -      printf("%"PRIuMAX, (uintmax_t)v); These days the MSVC version we
    +      printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
           rely on supports %z, but the C library used by MinGW does not.
      
     +   . Shorthand like ".a.b = *c" in struct assignments is known to trip

-- 
2.38.0-167-gf9a88ca9e9

[PATCH v2 1/5] CodingGuidelines: update for C99

[Junio C Hamano]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Since 7bc341e21b5 (git-compat-util: add a test balloon for C99
support, 2021-12-01) we've had a hard dependency on C99, but the prose
in CodingGuidelines was written under the assumption that we were
using C89 with a few C99 features.

As the updated prose notes we'd still like to hold off on novel C99
features, but let's make it clear that we target that C version, and
then enumerate new C99 features that are safe to use.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 9fca21cc5f..386ca0a0d2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -204,10 +204,14 @@ For C programs:
    by e.g. "echo DEVELOPER=1 >>config.mak".
 
  - We try to support a wide range of C compilers to compile Git with,
-   including old ones.  You should not use features from newer C
+   including old ones.  As of Git v2.35.0 Git requires C99 (we check
+   "__STDC_VERSION__"). You should not use features from a newer C
    standard, even if your compiler groks them.
 
-   There are a few exceptions to this guideline:
+   New C99 features have been phased in gradually, if something's new
+   in C99 but not used yet don't assume that it's safe to use, some
+   compilers we target have only partial support for it. These are
+   considered safe to use:
 
    . since early 2012 with e1327023ea, we have been using an enum
      definition whose last element is followed by a comma.  This, like
-- 
2.38.0-167-gf9a88ca9e9

[PATCH v2 2/5] CodingGuidelines: mention dynamic C99 initializer elements

[Junio C Hamano]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

The first use of variables in initializer elements appears to have
been 2b6854c863a (Cleanup variables in cat-file, 2007-04-21) released
with v1.5.2.

Some of those caused portability issues, and e.g. that "cat-file" use
was changed in 66dbfd55e38 (Rewrite dynamic structure initializations
to runtime assignment, 2010-05-14) which went out with v1.7.2.

But curiously 66dbfd55e38 missed some of them, e.g. an archive.c use
added in d5f53d6d6f2 (archive: complain about path specs that don't
match anything, 2009-12-12), and another one in merge-index.c (later
builtin/merge-index.c) in 0077138cd9d (Simplify some instances of
run_command() by using run_command_v_opt()., 2009-06-08).

As far as I can tell there's been no point since 2b6854c863a in 2007
where a compiler that didn't support this has been able to compile
git. Presumably 66dbfd55e38 was an attempt to make headway with wider
portability that ultimately wasn't completed.

In any case, we are thoroughly reliant on this syntax at this point,
so let's update the guidelines, see
https://lore.kernel.org/git/xmqqy1tunjgp.fsf@gitster.g/ for the
initial discussion.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 386ca0a0d2..8afda28cfc 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -213,6 +213,11 @@ For C programs:
    compilers we target have only partial support for it. These are
    considered safe to use:
 
+   . since around 2007 with 2b6854c863a, we have been using
+     initializer elements which are not computable at load time. E.g.:
+
+	const char *args[] = {"constant", variable, NULL};
+
    . since early 2012 with e1327023ea, we have been using an enum
      definition whose last element is followed by a comma.  This, like
      an array initializer that ends with a trailing comma, can be used
-- 
2.38.0-167-gf9a88ca9e9

[PATCH v2 3/5] CodingGuidelines: allow declaring variables in for loops

[Junio C Hamano]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Since 44ba10d6712 (revision: use C99 declaration of variable in for()
loop, 2021-11-14) released with v2.35.0 we've had a variable declared
with in a for loop.

Since then we've had inadvertent follow-ups to that with at least
cb2607759e2 (merge-ort: store more specific conflict information,
2022-06-18) released with v2.38.0.

As November 2022 is within the window of this upcoming release,
let's update the guideline to allow this.  We can have the promised
"revisit" discussion while this patch cooks, and drop it if it turns
out that it is still premature, which is not expected to happen at
this moment.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 10 ++--------
 revision.c                     |  7 -------
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 8afda28cfc..f9affc4050 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -232,18 +232,12 @@ For C programs:
    . since early 2021 with 765dc168882, we have been using variadic
      macros, mostly for printf-like trace and debug macros.
 
-   These used to be forbidden, but we have not heard any breakage
-   report, and they are assumed to be safe.
+   . since late 2021 with 44ba10d6, we have had variables declared in
+     the for loop "for (int i = 0; i < 10; i++)".
 
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
- - Declaring a variable in the for loop "for (int i = 0; i < 10; i++)"
-   is still not allowed in this codebase.  We are in the process of
-   allowing it by waiting to see that 44ba10d6 (revision: use C99
-   declaration of variable in for() loop, 2021-11-14) does not get
-   complaints.  Let's revisit this around November 2022.
-
  - NULL pointers shall be written as NULL, not as 0.
 
  - When declaring pointers, the star sides with the variable
diff --git a/revision.c b/revision.c
index 36e31942ce..8f2623b3b5 100644
--- a/revision.c
+++ b/revision.c
@@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);
 void show_object_with_name(FILE *out, struct object *obj, const char *name)
 {
 	fprintf(out, "%s ", oid_to_hex(&obj->oid));
-	/*
-	 * This "for (const char *p = ..." is made as a first step towards
-	 * making use of such declarations elsewhere in our codebase.  If
-	 * it causes compilation problems on your platform, please report
-	 * it to the Git mailing list at git@vger.kernel.org. In the meantime,
-	 * adding -std=gnu99 to CFLAGS may help if you are with older GCC.
-	 */
 	for (const char *p = name; *p && *p != '\n'; p++)
 		fputc(*p, out);
 	fputc('\n', out);
-- 
2.38.0-167-gf9a88ca9e9

[PATCH v2 4/5] CodingGuidelines: mention C99 features we can't use

[Junio C Hamano]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

The C99 section of the CodingGuidelines is a good overview of what we
can use, but is sorely lacking in what we can't use. Something that
comes up occasionally is the portability of %z.

Per [1] we couldn't use it for the longest time due to MSVC not
supporting it, but nowadays by requiring C99 we rely on the MSVC
version that does, but we can't use it yet because a C library that
MinGW uses doesn't support it.

1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f9affc4050..9598b45f7e 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -235,6 +235,13 @@ For C programs:
    . since late 2021 with 44ba10d6, we have had variables declared in
      the for loop "for (int i = 0; i < 10; i++)".
 
+   New C99 features that we cannot use yet:
+
+   . %z and %zu as a printf() argument for a size_t (the %z being for
+     the POSIX-specific ssize_t). Instead you should use
+     printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
+     rely on supports %z, but the C library used by MinGW does not.
+
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
-- 
2.38.0-167-gf9a88ca9e9

[PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Junio C Hamano]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Per 33665d98e6b (reftable: make assignments portable to AIX xlc
v12.01, 2022-03-28) forms like ".a.b = *c" can be replaced by using
".a = { .b = *c }" instead.

We'll probably allow these sooner than later, but since the workaround
is trivial let's note it among the C99 features we'd like to hold off
on for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 9598b45f7e..cbe0377699 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -242,6 +242,10 @@ For C programs:
      printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
      rely on supports %z, but the C library used by MinGW does not.
 
+   . Shorthand like ".a.b = *c" in struct assignments is known to trip
+     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
+     the 33665d98e6b portability fix from mid-2022.
+
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
-- 
2.38.0-167-gf9a88ca9e9

Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Jeff King]

On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:

> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 9598b45f7e..cbe0377699 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -242,6 +242,10 @@ For C programs:
>       printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
>       rely on supports %z, but the C library used by MinGW does not.
>  
> +   . Shorthand like ".a.b = *c" in struct assignments is known to trip
> +     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
> +     the 33665d98e6b portability fix from mid-2022.

FWIW, the use of the word "assignment" here left me scratching my head.
Reading 33665d98e6b, it is about struct initialization.

C99 does allow assignment from a compound literal, like:

  foo = (struct some_struct){ .a = 1, .b = 2 };

but I don't think we have any examples in the current code (and I think
we'd consider it a different feature for purposes of testing a weather
balloon).

-Peff

Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:
>
>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>> index 9598b45f7e..cbe0377699 100644
>> --- a/Documentation/CodingGuidelines
>> +++ b/Documentation/CodingGuidelines
>> @@ -242,6 +242,10 @@ For C programs:
>>       printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
>>       rely on supports %z, but the C library used by MinGW does not.
>>  
>> +   . Shorthand like ".a.b = *c" in struct assignments is known to trip
>> +     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
>> +     the 33665d98e6b portability fix from mid-2022.
>
> FWIW, the use of the word "assignment" here left me scratching my head.
> Reading 33665d98e6b, it is about struct initialization.

Thanks, I missed that confusion in the new description.  Perhaps
another round of reroll would make the series polished enough?

Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Jeff King]

On Mon, Oct 10, 2022 at 05:26:36PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:
> >
> >> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> >> index 9598b45f7e..cbe0377699 100644
> >> --- a/Documentation/CodingGuidelines
> >> +++ b/Documentation/CodingGuidelines
> >> @@ -242,6 +242,10 @@ For C programs:
> >>       printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
> >>       rely on supports %z, but the C library used by MinGW does not.
> >>  
> >> +   . Shorthand like ".a.b = *c" in struct assignments is known to trip
> >> +     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
> >> +     the 33665d98e6b portability fix from mid-2022.
> >
> > FWIW, the use of the word "assignment" here left me scratching my head.
> > Reading 33665d98e6b, it is about struct initialization.
> 
> Thanks, I missed that confusion in the new description.  Perhaps
> another round of reroll would make the series polished enough?

I read through the rest of it fairly lightly, but I didn't see have any
other complaints (and the overall goal of documenting our use of
compiler features is a great one).

-Peff

Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Ævar Arnfjörð Bjarmason]

On Mon, Oct 10 2022, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
>
>> On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:
>>
>>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
>>> index 9598b45f7e..cbe0377699 100644
>>> --- a/Documentation/CodingGuidelines
>>> +++ b/Documentation/CodingGuidelines
>>> @@ -242,6 +242,10 @@ For C programs:
>>>       printf("%"PRIuMAX, (uintmax_t)v).  These days the MSVC version we
>>>       rely on supports %z, but the C library used by MinGW does not.
>>>  
>>> +   . Shorthand like ".a.b = *c" in struct assignments is known to trip
>>> +     up an older IBM XLC version, use ".a = { .b = *c }" instead. See
>>> +     the 33665d98e6b portability fix from mid-2022.
>>
>> FWIW, the use of the word "assignment" here left me scratching my head.
>> Reading 33665d98e6b, it is about struct initialization.
>
> Thanks, I missed that confusion in the new description.  Perhaps
> another round of reroll would make the series polished enough?

I could re-roll it, but I also see you extensively fixed it up v.s. my
version. I think a re-roll here would just be
s/assignments/initializations/, so if you wanted to squash that in to
your already extensive squashes...

...or I could also re-roll it, up to you. Just let me know what you'd
prefer, thanks!

Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>>> FWIW, the use of the word "assignment" here left me scratching my head.
>>> Reading 33665d98e6b, it is about struct initialization.
>>
>> Thanks, I missed that confusion in the new description.  Perhaps
>> another round of reroll would make the series polished enough?
>
> I could re-roll it, but I also see you extensively fixed it up v.s. my
> version. I think a re-roll here would just be
> s/assignments/initializations/, so if you wanted to squash that in to
> your already extensive squashes...

I do not think there has been that much squashing.  Unless I took a
wrong range-diff in [0/5] of the series, the fix-ups are to reword
the proposed log message for [3/5] and to remove an unrelated hunk
that does "now we allow it, let's use it" which is better done
outside the topic with its own justification (and "it once used to
be there" is not a good justification) in [3/5], plus a three-byte
grammofix in [4/5].

So throwing "assignments" -> "initializations" into the mix is not
too much work for me.

Unless I forget, that is ;-)