{"thread":{"id":"57640","subject":"[PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","startedAt":"2022-03-31T00:09:24Z","lastAt":"2022-04-01T09:55:27Z","messageCount":7,"participants":["Junio C Hamano","Ævar Arnfjörð Bjarmason","Phillip Wood","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"452725","messageId":"xmqqy20r3rv7.fsf@gitster.g","threadId":"57640","inReplyTo":null,"subject":"[PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-31T00:09:16Z","receivedAt":"2022-03-31T00:09:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We raised the weather balloon to see if we can allow the construct\nin 44ba10d6 (revision: use C99 declaration of variable in for()\nloop, 2021-11-14), which was shipped as a part of Git v2.35.\nDocument that fact in the coding guidelines, and more importantly,\ngive ourselves a deadline to revisit and update.\n\nLet's declare that we will officially adopt the variable declaration\nin the initializaiton part of \"for ()\" statement this winter, unless\nwe find that a platform we care about does not grok it.\n\nA separate weather balloon for C99 as a whole was raised separately\nin 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n2021-12-01).  Hopefully, as we find out that all C99 features are OK\non all platforms we care about, we can stop probing the features we\nwant one-by-one like this (it does not necessarily mean that we\nwould automatically start using any and all C99 language features,\nthough).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 0e27b5395d..f0475c1770 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -217,7 +217,10 @@ For C programs:\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n  - Declaring a variable in the for loop \"for (int i = 0; i < 10; i++)\"\n-   is still not allowed in this codebase.\n+   is still not allowed in this codebase.  We are in the process of\n+   allowing it by waiting to see that 44ba10d6 (revision: use C99\n+   declaration of variable in for() loop, 2021-11-14) does not get\n+   complaints.  Let's revisit this around November 2022.\n \n  - NULL pointers shall be written as NULL, not as 0.\n \n-- \n2.35.1-898-ge9d02b9944\n\n"},{"id":"452784","messageId":"220331.86v8vuqv95.gmgdl@evledraar.gmail.com","threadId":"57640","inReplyTo":"xmqqy20r3rv7.fsf@gitster.g","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-31T10:10:20Z","receivedAt":"2022-03-31T10:19:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 30 2022, Junio C Hamano wrote:\n\n> We raised the weather balloon to see if we can allow the construct\n> in 44ba10d6 (revision: use C99 declaration of variable in for()\n> loop, 2021-11-14), which was shipped as a part of Git v2.35.\n> Document that fact in the coding guidelines, and more importantly,\n> give ourselves a deadline to revisit and update.\n>\n> Let's declare that we will officially adopt the variable declaration\n> in the initializaiton [...]\n\nTypo: initialization.\n\n> part of \"for ()\" statement this winter, unless we find that a platform\n> we care about does not grok it.\n\nI'd think that waiting a couple of releases would be sufficient for this\nsort of thing. I.e. contributors to this project already have\naccess/knowledge about a wide variety of compilers, especially the\n\"usual suspects\" (mainly MSVC) that have been blockers for using new\nlanguage features in the past.\n\nSo I'm in no rush to use this, and the winter deadline sounds fine to\nme in that regard.\n\nBut on the other hand I think the likelihood that waiting until November\nv.s. May revealing that a hitherto unknown compiler or platform has\nissues with a new language feature is vanishingly small.\n\n> A separate weather balloon for C99 as a whole was raised separately\n> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n> on all platforms we care about, we can stop probing the features we\n> want one-by-one like this\n\nUnfortunately this really isn't the case at all, the norm is for\ncompilers to advertise that they support verison X of the standard via\nthese macros when they consider the support \"good enough\", but while\nthere's still a long list of unimplemented features before they're at\n100% support (and most never fully get to 100%).\n\nWe also need to worry about the stdlib implementation, and not just the\ncompiler, see e.g. the %zu format and MinGW in the exchange at\nhttps://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\nand\nhttps://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n\nBut I think we're thoroughly past needing to worry about basic language\nfeatures in C99 such as these inline variable declarations.\n\n> (it does not necessarily mean that we would automatically start using\n> any and all C99 language features, though).\n\nYes, particularly those that the standards committee backed out of or\nmade optional after C99 would be good candidates for avoiding\npermanently.\n"},{"id":"452792","messageId":"bf44f83b-0d18-8132-58cf-13155bfec40e@gmail.com","threadId":"57640","inReplyTo":"220331.86v8vuqv95.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-31T14:48:56Z","receivedAt":"2022-03-31T14:49:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 31/03/2022 11:10, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Mar 30 2022, Junio C Hamano wrote:\n> \n>> We raised the weather balloon to see if we can allow the construct\n>> in 44ba10d6 (revision: use C99 declaration of variable in for()\n>> loop, 2021-11-14), which was shipped as a part of Git v2.35.\n>> Document that fact in the coding guidelines, and more importantly,\n>> give ourselves a deadline to revisit and update.\n>>\n>> Let's declare that we will officially adopt the variable declaration\n>> in the initializaiton [...]\n> \n> Typo: initialization.\n> \n>> part of \"for ()\" statement this winter, unless we find that a platform\n>> we care about does not grok it.\n> \n> I'd think that waiting a couple of releases would be sufficient for this\n> sort of thing. I.e. contributors to this project already have\n> access/knowledge about a wide variety of compilers, especially the\n> \"usual suspects\" (mainly MSVC) that have been blockers for using new\n> language features in the past.\n> \n> So I'm in no rush to use this, and the winter deadline sounds fine to\n> me in that regard.\n\nAgreed, I think it is worth waiting so we don't get into a situation \nwhere we end up having to revert changes that are using the new features \nbecause we discover they are not supported by a platform we care about.\n\n> But on the other hand I think the likelihood that waiting until November\n> v.s. May revealing that a hitherto unknown compiler or platform has\n> issues with a new language feature is vanishingly small.\n> \n>> A separate weather balloon for C99 as a whole was raised separately\n>> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n>> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n>> on all platforms we care about, we can stop probing the features we\n>> want one-by-one like this\n> \n> Unfortunately this really isn't the case at all, the norm is for\n> compilers to advertise that they support verison X of the standard via\n> these macros when they consider the support \"good enough\", but while\n> there's still a long list of unimplemented features before they're at\n> 100% support (and most never fully get to 100%).\n> \n> We also need to worry about the stdlib implementation, and not just the\n> compiler, see e.g. the %zu format and MinGW in the exchange at\n> https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\n> and\n> https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n\nThat's a good point, it was a surprise to me that the problem is with \nMinGW rather than MSVC.\n\n\nBest Wishes\n\nPhillip\n\n> But I think we're thoroughly past needing to worry about basic language\n> features in C99 such as these inline variable declarations.\n> \n>> (it does not necessarily mean that we would automatically start using\n>> any and all C99 language features, though).\n> \n> Yes, particularly those that the standards committee backed out of or\n> made optional after C99 would be good candidates for avoiding\n> permanently.\n"},{"id":"452794","messageId":"220331.86sfqyp3ed.gmgdl@evledraar.gmail.com","threadId":"57640","inReplyTo":"bf44f83b-0d18-8132-58cf-13155bfec40e@gmail.com","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-31T14:58:07Z","receivedAt":"2022-03-31T15:06:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 31 2022, Phillip Wood wrote:\n\n> On 31/03/2022 11:10, Ævar Arnfjörð Bjarmason wrote:\n>> On Wed, Mar 30 2022, Junio C Hamano wrote:\n>> \n>>> We raised the weather balloon to see if we can allow the construct\n>>> in 44ba10d6 (revision: use C99 declaration of variable in for()\n>>> loop, 2021-11-14), which was shipped as a part of Git v2.35.\n>>> Document that fact in the coding guidelines, and more importantly,\n>>> give ourselves a deadline to revisit and update.\n>>>\n>>> Let's declare that we will officially adopt the variable declaration\n>>> in the initializaiton [...]\n>> Typo: initialization.\n>> \n>>> part of \"for ()\" statement this winter, unless we find that a platform\n>>> we care about does not grok it.\n>> I'd think that waiting a couple of releases would be sufficient for\n>> this\n>> sort of thing. I.e. contributors to this project already have\n>> access/knowledge about a wide variety of compilers, especially the\n>> \"usual suspects\" (mainly MSVC) that have been blockers for using new\n>> language features in the past.\n>> So I'm in no rush to use this, and the winter deadline sounds fine\n>> to\n>> me in that regard.\n>\n> Agreed, I think it is worth waiting so we don't get into a situation\n> where we end up having to revert changes that are using the new\n> features because we discover they are not supported by a platform we\n> care about.\n>\n>> But on the other hand I think the likelihood that waiting until November\n>> v.s. May revealing that a hitherto unknown compiler or platform has\n>> issues with a new language feature is vanishingly small.\n>> \n>>> A separate weather balloon for C99 as a whole was raised separately\n>>> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n>>> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n>>> on all platforms we care about, we can stop probing the features we\n>>> want one-by-one like this\n>> Unfortunately this really isn't the case at all, the norm is for\n>> compilers to advertise that they support verison X of the standard via\n>> these macros when they consider the support \"good enough\", but while\n>> there's still a long list of unimplemented features before they're at\n>> 100% support (and most never fully get to 100%).\n>> We also need to worry about the stdlib implementation, and not just\n>> the\n>> compiler, see e.g. the %zu format and MinGW in the exchange at\n>> https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\n>> and\n>> https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n>\n> That's a good point, it was a surprise to me that the problem is with\n> MinGW rather than MSVC.\n\nYes, thanks a lot for tracking that down.\n\nI wonder if we can supply a compatibility sprintf() shim for that setup,\nthere's nothing urgent about it, but the verbosity of the casts and\nPRIuMAX inline adds up, especially as we've started using size_t more\nwidely:\n\n\tgit grep PRIuMAX -- '*.[ch]'\n\nEither by e.g. grabbing the sprintf() shim from say gnulib, or our own\nshim that would intercept the \"const char *format\" for sprintf(), and\npull a trick similar to what we do in strbuf_addftime() to rewrite the\nformat (of a copied string) on-the-fly.\n"},{"id":"452832","messageId":"xmqqo81l286e.fsf@gitster.g","threadId":"57640","inReplyTo":"220331.86v8vuqv95.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-31T20:12:09Z","receivedAt":"2022-03-31T20:12:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> A separate weather balloon for C99 as a whole was raised separately\n>> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n>> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n>> on all platforms we care about, we can stop probing the features we\n>> want one-by-one like this\n>\n> Unfortunately this really isn't the case at all, the norm is for\n> compilers to advertise that they support verison X of the standard via\n> these macros when they consider the support \"good enough\", but while\n> there's still a long list of unimplemented features before they're at\n> 100% support (and most never fully get to 100%).\n>\n> We also need to worry about the stdlib implementation, and not just the\n> compiler, see e.g. the %zu format and MinGW in the exchange at\n> https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\n> and\n> https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n>\n> But I think we're thoroughly past needing to worry about basic language\n> features in C99 such as these inline variable declarations.\n\nWell, that makes it sound like the C99 weather balloon was almost\nuseless, doesn't it?\n\nIn any case, I'll strick the last paragraph from the final log\nmessage, as the patch text itself is about one specific feature, and\nnot about deciding what our policy for various C99 language\nfeeatures ought to be.\n\nThanks.\n"},{"id":"452836","messageId":"YkYa2NyA6IwIqZ4C@camp.crustytoothpaste.net","threadId":"57640","inReplyTo":"xmqqo81l286e.fsf@gitster.g","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-03-31T21:19:20Z","receivedAt":"2022-03-31T21:19:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-03-31 at 20:12:09, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n> \n> >> A separate weather balloon for C99 as a whole was raised separately\n> >> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n> >> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n> >> on all platforms we care about, we can stop probing the features we\n> >> want one-by-one like this\n> >\n> > Unfortunately this really isn't the case at all, the norm is for\n> > compilers to advertise that they support verison X of the standard via\n> > these macros when they consider the support \"good enough\", but while\n> > there's still a long list of unimplemented features before they're at\n> > 100% support (and most never fully get to 100%).\n> >\n> > We also need to worry about the stdlib implementation, and not just the\n> > compiler, see e.g. the %zu format and MinGW in the exchange at\n> > https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\n> > and\n> > https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n> >\n> > But I think we're thoroughly past needing to worry about basic language\n> > features in C99 such as these inline variable declarations.\n> \n> Well, that makes it sound like the C99 weather balloon was almost\n> useless, doesn't it?\n\nI think if we were talking about C17, maybe.  But as I said in my commit\nmessage, C99 is over two decades old and required for the POSIX version\nwhich came out in 2001.  I'm aware of only two platforms we care about\nthat don't support that POSIX version, which are NonStop and Windows.\n\nI think the likelihood of this being a problem is very low.  And I think\nwe can justifiably expect that major syntactic functionality is\navailable when the define is set accordingly.  I am also willing to\nsimply tell people that a compiler that includes the define and doesn't\ninclude the requisite features is buggy and ask them to use a modern\nversion of GCC or clang.  But, ultimately, given we're talking about\nC99, this is extremely unlikely to ever be a problem in 2022.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"452857","messageId":"220401.86tubdnn5j.gmgdl@evledraar.gmail.com","threadId":"57640","inReplyTo":"YkYa2NyA6IwIqZ4C@camp.crustytoothpaste.net","subject":"Re: [PATCH] CodingGuidelines: give deadline for \"for (int i = 0; ...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-01T09:29:13Z","receivedAt":"2022-04-01T09:55:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 31 2022, brian m. carlson wrote:\n\n> [[PGP Signed Part:Undecided]]\n> On 2022-03-31 at 20:12:09, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>> \n>> >> A separate weather balloon for C99 as a whole was raised separately\n>> >> in 7bc341e2 (git-compat-util: add a test balloon for C99 support,\n>> >> 2021-12-01).  Hopefully, as we find out that all C99 features are OK\n>> >> on all platforms we care about, we can stop probing the features we\n>> >> want one-by-one like this\n>> >\n>> > Unfortunately this really isn't the case at all, the norm is for\n>> > compilers to advertise that they support verison X of the standard via\n>> > these macros when they consider the support \"good enough\", but while\n>> > there's still a long list of unimplemented features before they're at\n>> > 100% support (and most never fully get to 100%).\n>> >\n>> > We also need to worry about the stdlib implementation, and not just the\n>> > compiler, see e.g. the %zu format and MinGW in the exchange at\n>> > https://lore.kernel.org/git/220318.86bky3cr8j.gmgdl@evledraar.gmail.com/\n>> > and\n>> > https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/;\n>> >\n>> > But I think we're thoroughly past needing to worry about basic language\n>> > features in C99 such as these inline variable declarations.\n>> \n>> Well, that makes it sound like the C99 weather balloon was almost\n>> useless, doesn't it?\n>\n> I think if we were talking about C17, maybe.  But as I said in my commit\n> message, C99 is over two decades old and required for the POSIX version\n> which came out in 2001.  I'm aware of only two platforms we care about\n> that don't support that POSIX version, which are NonStop and Windows.\n\nWhat I'm referring to upthread is that if you look at\ne.g. https://gcc.gnu.org/c99status.html and compare it to\nhttps://gcc.gnu.org/releases.html you can see that GCC (which is usually\nreally early with these things) was still implementing major C99\nfeatures like \"inline\" (it had its own syntax, but not the C99 one) in\n2008.\n\nSo, part of this is just unintentional commentary on how old I'm getting\n:), I hadn't looked before sending the upthread, but I remembered this\nbeing a bit closer to $(date).\n\nIn any case, I think that shows a typical pattern where compiler\nimplementations are still catching up with standards at least 5-10 years\nafter they're released.\n\nIf you then look at C11 at https://gcc.gnu.org/wiki/C11Status you can\nsee that _Generic took until April 2014 with GCC 4.9, and\ne.g. https://access.redhat.com/solutions/19458 notes that RHEL 7 (still\nin wide use) was released with GCC 4.8, and RHEL 6 with 4.4.\n\n(I wouldn't be surprised if RedHat had sneakily backported some\nfeatures, so don't take that as gospel about what is and isn't supported\non that platform).\n\n> I think the likelihood of this being a problem is very low.  And I think\n> we can justifiably expect that major syntactic functionality is\n> available when the define is set accordingly.\n\nIndeed, I only meant to suggest that we might still need to be careful\nwith some of the more exotic features, and do some research similar to\nthe above.\n\nE.g. I wouldn't be at all surprised if some of the long-tail compilers\nwe support (suncc, xlc, msvc) had implemented some parts of C99 much\nlater.\n\n> I am also willing to simply tell people that a compiler that includes\n> the define and doesn't include the requisite features is buggy and ask\n> them to use a modern version of GCC or clang.\n\nSo we can drop MinGW & I can use %zu? Yay! :)\n\nMore seriously, I don't think we should have such a blanket\npolicy.\n\nSure, we might decide that's worth it, but when we run into such issues\nI think it's good to just weigh the trade-offs for particular\nfeature. See \"We live in the real world\" in\nDocumentation/CodingGuidelines for some nice wisdom.\n\nE.g. (and I didn't recall this when I wrote most of the above) I've got\n33665d98e6b (reftable: make assignments portable to AIX xlc v12.01,\n2022-03-28) in \"next\" right now which undoes some small use of C99\nsyntax because a (buggy) version of xlc isn't too happy about an\nedge-case of valid C99 initializer syntax.\n\nNow, we could just say we won't support that compiler, but as argued in\nthat commit message it's relatively easy to accommodate it, so being a\nbit lenient with what subset of the standard we insist on seems prudent.\n\n> But, ultimately, given we're talking about\n> C99, this is extremely unlikely to ever be a problem in 2022.\n\nPer the above it was a (small) problem on 2022-03-28 :)\n"}]}