From e88a2d02dc5fb1043e6871159a8d41657baa7449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 10 Oct 2022 13:37:56 -0700 Subject: [PATCH 1/5] CodingGuidelines: update for C99 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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 From 442c27dde784049a2c0f8815ac5030817123386c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 10 Oct 2022 13:37:57 -0700 Subject: [PATCH 2/5] CodingGuidelines: mention dynamic C99 initializer elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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 From 82dd01d81b1babf6714435b80683e756de1a4d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 10 Oct 2022 13:37:58 -0700 Subject: [PATCH 3/5] CodingGuidelines: allow declaring variables in for loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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); From d7d850e2b979c2063a7b39c6554233a8af189f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 10 Oct 2022 13:37:59 -0700 Subject: [PATCH 4/5] CodingGuidelines: mention C99 features we can't use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- 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). From 438c2f859b2c8c01c57a6969ec5858c0253cfa69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 10 Oct 2022 13:38:00 -0700 Subject: [PATCH 5/5] CodingGuidelines: recommend against unportable C99 struct syntax MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 9598b45f7e..1d95a142b2 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -242,6 +242,11 @@ 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 initializations is known to + trip up an older IBM XLC version, use ".a = { .b = *c }" instead. + See the 33665d98 (reftable: make assignments portable to AIX xlc + v12.01, 2022-03-28). + - Variables have to be declared at the beginning of the block, before the first statement (i.e. -Wdeclaration-after-statement).