{"thread":{"id":"56887","subject":"[PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code","startedAt":"2021-11-13T12:29:28Z","lastAt":"2021-12-08T17:05:20Z","messageCount":22,"participants":["Saksham Mittal","Johannes Altmanninger","Junio C Hamano","Ævar Arnfjörð Bjarmason","Carlo Arenas","brian m. carlson","Martin Ågren","Phillip Wood","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441065","messageId":"20211113122833.174330-1-gotlouemail@gmail.com","threadId":"56887","inReplyTo":null,"subject":"[PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code","fromName":"Saksham Mittal","fromEmail":"gotlouemail@gmail.com","sentAt":"2021-11-13T12:28:35Z","receivedAt":"2021-11-13T12:29:28Z","isPatch":true,"sender":{"key":"gotlouemail@gmail.com","avatar":null},"body":"In the sample code given to print the arguments given to ```git psuh```,\nthe iterating variable i is not declared as integer. I have fixed it so\nthe error is no longer there.\n\nSigned-off-by: Saksham Mittal \n---\n Documentation/MyFirstContribution.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt\nindex 015cf24631..434a833a0b 100644\n--- a/Documentation/MyFirstContribution.txt\n+++ b/Documentation/MyFirstContribution.txt\n@@ -297,7 +297,7 @@ existing `printf()` calls in place:\n \t\t \"Your args (there are %d):\\n\",\n \t\t argc),\n \t argc);\n-\tfor (i = 0; i < argc; i++)\n+\tfor (int i = 0; i < argc; i++)\n \t\tprintf(\"%d: %s\\n\", i, argv[i]);\n \n \tprintf(_(\"Your current working directory:\\n%s%s\\n\"),\n-- \n2.33.1\n\n"},{"id":"441066","messageId":"20211113130508.zziheannky6dcilj@gmail.com","threadId":"56887","inReplyTo":"20211113122833.174330-1-gotlouemail@gmail.com","subject":"Re: [PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2021-11-13T13:05:08Z","receivedAt":"2021-11-13T13:05:18Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Sat, Nov 13, 2021 at 05:58:35PM +0530, Saksham Mittal wrote:\n> In the sample code given to print the arguments given to ```git psuh```,\n> the iterating variable i is not declared as integer. I have fixed it so\n> the error is no longer there.\n> \n> Signed-off-by: Saksham Mittal \n> ---\n> Documentation/MyFirstContribution.txt | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt\n> index 015cf24631..434a833a0b 100644\n> --- a/Documentation/MyFirstContribution.txt\n> +++ b/Documentation/MyFirstContribution.txt\n> @@ -297,7 +297,7 @@ existing `printf()` calls in place:\n> \t\t \"Your args (there are %d):\\n\",\n> \t\t argc),\n> \t argc);\n> -\tfor (i = 0; i < argc; i++)\n> +\tfor (int i = 0; i < argc; i++)\n\nIt is declared, there is an \"int i;\" a few lines up.\n"},{"id":"441067","messageId":"2b2386b9-045d-a0b8-6dbc-8a9d0c446bea@gmail.com","threadId":"56887","inReplyTo":"20211113130508.zziheannky6dcilj@gmail.com","subject":"Re: [PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code","fromName":"Saksham Mittal","fromEmail":"gotlouemail@gmail.com","sentAt":"2021-11-13T13:08:25Z","receivedAt":"2021-11-13T13:08:48Z","isPatch":true,"sender":{"key":"gotlouemail@gmail.com","avatar":null},"body":"\n> It is declared, there is an \"int i;\" a few lines up.\n\nOh, man, I never even saw that! The patch is completely unnecessary\nthen. Sorry for that!\n\nSaksham\n"},{"id":"441076","messageId":"xmqq7ddbme7q.fsf@gitster.g","threadId":"56887","inReplyTo":"2b2386b9-045d-a0b8-6dbc-8a9d0c446bea@gmail.com","subject":"Re: [PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-14T06:41:13Z","receivedAt":"2021-11-14T06:41:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Saksham Mittal writes:\n\n>> It is declared, there is an \"int i;\" a few lines up.\n>\n> Oh, man, I never even saw that! The patch is completely unnecessary\n> then. Sorry for that!\n\nNo need to say sorry; you'd want to be a bit more careful next time,\nthat's all.\n\nAlso, our code does not introduce a new variable in the first part\nof \"for (;;)\" loop control, so even if the original lacked decl for\n\"i\", the posted patch is not how we write our code for this project.\n\nThanks.\n\n"},{"id":"441083","messageId":"211114.868rxqu7hr.gmgdl@evledraar.gmail.com","threadId":"56887","inReplyTo":"xmqq7ddbme7q.fsf@gitster.g","subject":"Is 'for (int i = [...]' bad for C STD compliance reasons? (was: [PATCH] MyFirstContribution.txt: fix undeclared variable i in sample code)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-14T14:28:35Z","receivedAt":"2021-11-14T14:39:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Nov 13 2021, Junio C Hamano wrote:\n\n> Saksham Mittal writes:\n>\n>>> It is declared, there is an \"int i;\" a few lines up.\n>>\n>> Oh, man, I never even saw that! The patch is completely unnecessary\n>> then. Sorry for that!\n>\n> No need to say sorry; you'd want to be a bit more careful next time,\n> that's all.\n>\n> Also, our code does not introduce a new variable in the first part\n> of \"for (;;)\" loop control, so even if the original lacked decl for\n> \"i\", the posted patch is not how we write our code for this project.\n\nJust curious: Out of preference, or for compatibility with older C\nstandards?\n\nI'd think with the things we depend on in C99 it's probable that we\ncould start using this if standards conformance is the only obstacle.\n\nBut I haven't tested, so maybe I'm wrong, I'm just assuming that with\nthe C99 features we do have a hard dependency on surely anyone\nimplementing those would have implemented this too.\n\nThere's also a stylistic reason to avoid this pattern, i.e. some would\nargue that it's better to declare variables up-front, since it tends to\nencourage one to keep function definitions smaller (various in-tree\nevidence to the contrary, but whatever).\n\nI'd generally agree with that viewpoint & desire, but there's also cases\nwhere being able to declare things in-line helps readability, e.g. when\nyour function needs two for-loops for some reason, they're set a bit\napart. Then the reader doesn't need to scan for whether an \"i\" is used\nin-between the two.\n\nI was thinking of the below code in bundle.c, I suppose some might find\nthe post-image less readable, but I remember starting to hunt around for\nother out-of-loop uses of \"i\", which the post-image makes clear could be\navoided as far as variable scoping goes:\n\ndiff --git a/bundle.c b/bundle.c\nindex a0bb687b0f4..94edc186187 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -194,14 +194,14 @@ int verify_bundle(struct repository *r,\n \tstruct rev_info revs;\n \tconst char *argv[] = {NULL, \"--all\", NULL};\n \tstruct commit *commit;\n-\tint i, ret = 0, req_nr;\n+\tint ret = 0, req_nr;\n \tconst char *message = _(\"Repository lacks these prerequisite commits:\");\n \n \tif (!r || !r->objects || !r->objects->odb)\n \t\treturn error(_(\"need a repository to verify a bundle\"));\n \n \trepo_init_revisions(r, &revs, NULL);\n-\tfor (i = 0; i < p->nr; i++) {\n+\tfor (int i = 0; i < p->nr; i++) {\n \t\tstruct string_list_item *e = p->items + i;\n \t\tconst char *name = e->string;\n \t\tstruct object_id *oid = e->util;\n@@ -223,12 +223,11 @@ int verify_bundle(struct repository *r,\n \tif (prepare_revision_walk(&revs))\n \t\tdie(_(\"revision walk setup failed\"));\n \n-\ti = req_nr;\n-\twhile (i && (commit = get_revision(&revs)))\n+\tfor (int i = req_nr; i && (commit = get_revision(&revs));)\n \t\tif (commit->object.flags & PREREQ_MARK)\n \t\t\ti--;\n \n-\tfor (i = 0; i < p->nr; i++) {\n+\tfor (int i = 0; i < p->nr; i++) {\n \t\tstruct string_list_item *e = p->items + i;\n \t\tconst char *name = e->string;\n \t\tconst struct object_id *oid = e->util;\n@@ -242,7 +241,7 @@ int verify_bundle(struct repository *r,\n \t}\n \n \t/* Clean up objects used, as they will be reused. */\n-\tfor (i = 0; i < p->nr; i++) {\n+\tfor (int i = 0; i < p->nr; i++) {\n \t\tstruct string_list_item *e = p->items + i;\n \t\tstruct object_id *oid = e->util;\n \t\tcommit = lookup_commit_reference_gently(r, oid, 1);\n"},{"id":"441090","messageId":"xmqqilwulims.fsf@gitster.g","threadId":"56887","inReplyTo":"211114.868rxqu7hr.gmgdl@evledraar.gmail.com","subject":"Re: Is 'for (int i = [...]' bad for C STD compliance reasons?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-14T18:03:23Z","receivedAt":"2021-11-14T18:03:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> Also, our code does not introduce a new variable in the first part\n>> of \"for (;;)\" loop control, so even if the original lacked decl for\n>> \"i\", the posted patch is not how we write our code for this project.\n>\n> Just curious: Out of preference, or for compatibility with older C\n> standards?\n\nThe latter.\n\ncc0c4297 (CodingGuidelines: spell out post-C89 rules, 2019-07-16)\nadds a few \"weather balloons say these are OK\" together with this\nexact one as \"not yet allowed\". We (at least, those of us who have\nenough knowledge and authority to propose changes to the guidelines)\nall know that particular feature is a nice thing to use if everybody\nwe care about supports it [*1*].\n\nHere is the thread that resulted in the relevant part of the\nguideilne.\n\nhttps://lore.kernel.org/git/CAPUEspgjSAqHUP2vsCCjqG8b0QkWdgoAByh4XdqsThQMt=V38w@mail.gmail.com/\n\nThe \"another patch that tried to use it late last year\" the thread\nrefers to is\nhttps://lore.kernel.org/git/20181114004745.GH30222@szeder.dev/\n\nIf I am not mistaken, Carlo added gcc-4.8 CI job to catch these\nrecently?\n\nNow, \"Centos 6 is no longer\" cannot be called a good response to\nthis message. We stopped at seeing the first failure, and breakages\non other platforms were not even counted back then. To those whose\ncompilers also barfed, it was sufficient that we pulled the plug\nafter seeing a failure on Centos 6.\n\nBut two years may be long enough for us to try again. If we want to\npursue it, we'd need to raise a weather balloon that would break\ncompilers that have been happily grokking our code loudly by being\nin a central place that will never be conditionally compiled out,\nand is easy to back out by being in ultra-stable location.\n\ncbc0f81d (strbuf: use designated initializers in STRBUF_INIT,\n2017-07-10) is an example that Peff found and used a great such\nlocation.\n\nI know you are capable of reading Documentation/CodingGuidelines and\nrunning \"git blame\" on it, and then use mailing list archive to dig\nto find the answer, and it was a bit of disappointment to see this\nwas asked as a question, rather than a well researched \"now after\ntwo years, let's try this again\".\n\n\n[References]\n\n*1* https://lore.kernel.org/git/xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com/\n"},{"id":"441091","messageId":"211114.86zgq6si94.gmgdl@evledraar.gmail.com","threadId":"56887","inReplyTo":"xmqqilwulims.fsf@gitster.g","subject":"Re: Is 'for (int i = [...]' bad for C STD compliance reasons?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-14T18:25:31Z","receivedAt":"2021-11-14T18:29:50Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Nov 14 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> Also, our code does not introduce a new variable in the first part\n>>> of \"for (;;)\" loop control, so even if the original lacked decl for\n>>> \"i\", the posted patch is not how we write our code for this project.\n>>\n>> Just curious: Out of preference, or for compatibility with older C\n>> standards?\n>\n> The latter.\n>\n> cc0c4297 (CodingGuidelines: spell out post-C89 rules, 2019-07-16)\n> adds a few \"weather balloons say these are OK\" together with this\n> exact one as \"not yet allowed\". We (at least, those of us who have\n> enough knowledge and authority to propose changes to the guidelines)\n> all know that particular feature is a nice thing to use if everybody\n> we care about supports it [*1*].\n>\n> Here is the thread that resulted in the relevant part of the\n> guideilne.\n>\n> https://lore.kernel.org/git/CAPUEspgjSAqHUP2vsCCjqG8b0QkWdgoAByh4XdqsThQMt=V38w@mail.gmail.com/\n>\n> The \"another patch that tried to use it late last year\" the thread\n> refers to is\n> https://lore.kernel.org/git/20181114004745.GH30222@szeder.dev/\n>\n> If I am not mistaken, Carlo added gcc-4.8 CI job to catch these\n> recently?\n>\n> Now, \"Centos 6 is no longer\" cannot be called a good response to\n> this message. We stopped at seeing the first failure, and breakages\n> on other platforms were not even counted back then. To those whose\n> compilers also barfed, it was sufficient that we pulled the plug\n> after seeing a failure on Centos 6.\n>\n> But two years may be long enough for us to try again. If we want to\n> pursue it, we'd need to raise a weather balloon that would break\n> compilers that have been happily grokking our code loudly by being\n> in a central place that will never be conditionally compiled out,\n> and is easy to back out by being in ultra-stable location.\n>\n> cbc0f81d (strbuf: use designated initializers in STRBUF_INIT,\n> 2017-07-10) is an example that Peff found and used a great such\n> location.\n>\n> I know you are capable of reading Documentation/CodingGuidelines and\n> running \"git blame\" on it, and then use mailing list archive to dig\n> to find the answer, and it was a bit of disappointment to see this\n> was asked as a question, rather than a well researched \"now after\n> two years, let's try this again\".\n>\n>\n> [References]\n>\n> *1* https://lore.kernel.org/git/xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com/\n\nThe issue on CentOS 6 isn't one of incompatibility with C99, but that\nthe version of GCC refuses to compile C99 code without -std=c99 or\n-std=gnu99. See [1] downthread of one of your links.\n\nBut yes, it would be the first C99 feature where we have a known\ncompiler that needs an opt-in -std=* option to support the C99 feature,\nI think.\n\n1. https://lore.kernel.org/git/20190717004231.GA93801@google.com/\n"},{"id":"441093","messageId":"CAPUEspgHm2py_irKKFucrnnCHrgAHraQkSnAJngORGVegzn3Nw@mail.gmail.com","threadId":"56887","inReplyTo":"211114.86zgq6si94.gmgdl@evledraar.gmail.com","subject":"Re: Is 'for (int i = [...]' bad for C STD compliance reasons?","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-11-14T19:01:24Z","receivedAt":"2021-11-14T19:01:40Z","isPatch":false,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Nov 14, 2021 at 10:31 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> The issue on CentOS 6 isn't one of incompatibility with C99, but that\n> the version of GCC refuses to compile C99 code without -std=c99 or\n> -std=gnu99. See [1] downthread of one of your links.\n\nFWIW while CentOS 6 is EOL, CentOS 7 (gcc 4.8.5) is also affected and\nhas at least one more year of \"support\".\n\nYou are correct that without a specific -std flag the build will\nbreak, and unlike what is expected from all other C99 features that\nwere supported by gnu89 (the default until gcc >= 5) and that are\ncurrently in use. The fact that the pedantic rollout went smoothly is\nencouraging in that respect, but take into consideration that is also\nlimited only to DEVELOPER=1.\n\nCarlo\n\nPS. there is a CI job in travis but travis is dead\n"},{"id":"441094","messageId":"YZFa3YDe1/a6uZod@camp.crustytoothpaste.net","threadId":"56887","inReplyTo":"211114.86zgq6si94.gmgdl@evledraar.gmail.com","subject":"Re: Is 'for (int i = [...]' bad for C STD compliance reasons?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-11-14T18:57:25Z","receivedAt":"2021-11-14T19:07:43Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-11-14 at 18:25:31, Ævar Arnfjörð Bjarmason wrote:\n> The issue on CentOS 6 isn't one of incompatibility with C99, but that\n> the version of GCC refuses to compile C99 code without -std=c99 or\n> -std=gnu99. See [1] downthread of one of your links.\n> \n> But yes, it would be the first C99 feature where we have a known\n> compiler that needs an opt-in -std=* option to support the C99 feature,\n> I think.\n\nYeah, as I've mentioned in the past, the impediment to C99 features is\nMSVC. All Unix compilers support it because it's obligatory for POSIX\n1003.1-2001, and they have for some time, even if it's not the default\nbehavior.\n\nMSVC recently learned C11, but I haven't fooled around with our CI\nenough recently to see if I can get it to use C11. I'll try to play\naround some more.\n\nI should also point out that CentOS 6 is now EOL and not receiving\nsecurity updates, and as such it shouldn't be a consideration in what we\ndo and don't support. I think providing 10 years of support for an OS\nin our project is already exceedingly generous, and most other projects\ndon't do so.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"441095","messageId":"CAPUEspgLU2oeh3dG0TeF6OJR9o8Q3HAXRBJSxE6-aUa_u1upxw@mail.gmail.com","threadId":"56887","inReplyTo":"YZFa3YDe1/a6uZod@camp.crustytoothpaste.net","subject":"Re: Is 'for (int i = [...]' bad for C STD compliance reasons?","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-11-14T19:33:31Z","receivedAt":"2021-11-14T19:34:01Z","isPatch":false,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Nov 14, 2021 at 11:08 AM brian m. carlson\n wrote:\n>\n> MSVC recently learned C11, but I haven't fooled around with our CI\n> enough recently to see if I can get it to use C11. I'll try to play\n> around some more.\n\nThe last 2 releases support C17, but I think that is only using the\nuniversal crt which is not what git for windows link with and might\nnot be available by default (10 or later feature) in all windows\nversions they target as well.\n\nCarlo\n\nPS. no Windows expert at all, and I am sure more authoritative answers\nwill come along, just wanted to let you know to probably avoid wasting\nyour time with the CI\n"},{"id":"441111","messageId":"xmqqpmr2j5lq.fsf_-_@gitster.g","threadId":"56887","inReplyTo":"xmqqilwulims.fsf@gitster.g","subject":"[PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-15T06:27:45Z","receivedAt":"2021-11-15T06:27:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are certain C99 features that might be nice to use in our code\nbase, but we've hesitated to do so in order to avoid breaking\ncompatibility with older compilers. But we don't actually know if\npeople are even using pre-C99 compilers these days.\n\nOne way to figure that out is to introduce a very small use of a\nfeature, and see if anybody complains, and we've done so to probe\nthe portability for a few features like \"trailing comma in enum\ndeclaration\", \"designated initializer for struct\", and \"designated\ninitializer for array\". A few years ago, we tried to use a handy\n\n for (int i = 0; i < n; i++)\n\tuse(i);\n\nto introduce a new variable valid only in the loop, but found that\nsome compilers we cared about didn't like it back then. Two years\nis a long-enough time, so let's try it agin.\n\nIf this patch can survive a few releases without complaint, then we\ncan feel more confident that variable declaration in for() loop is\nsupported by the compilers our user base use. And if we do get\ncomplaints, then we'll have gained some data and we can easily\nrevert this patch.\n\nSigned-off-by: Junio C Hamano \n---\n revision.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 9dff845bed..44492f2c02 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -43,10 +43,8 @@ static inline int want_ancestry(const struct rev_info *revs);\n \n void show_object_with_name(FILE *out, struct object *obj, const char *name)\n {\n-\tconst char *p;\n-\n \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n-\tfor (p = name; *p && *p != '\\n'; p++)\n+\tfor (const char *p = name; *p && *p != '\\n'; p++)\n \t\tfputc(*p, out);\n \tfputc('\\n', out);\n }\n-- \n2.34.0-rc2-165-g9b3c04af29\n\n"},{"id":"441116","messageId":"CAN0heSpLy8c6WM1UpyEXJLfmnX=5B0eFhJwH3wqSZN15HAJGeg@mail.gmail.com","threadId":"56887","inReplyTo":"xmqqpmr2j5lq.fsf_-_@gitster.g","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-11-15T07:44:55Z","receivedAt":"2021-11-15T07:45:29Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Mon, 15 Nov 2021 at 07:30, Junio C Hamano wrote:\n>\n> There are certain C99 features that might be nice to use in our code\n> base, but we've hesitated to do so in order to avoid breaking\n> compatibility with older compilers. But we don't actually know if\n> people are even using pre-C99 compilers these days.\n\n> is a long-enough time, so let's try it agin.\n\ns/agin/again/\n\n> void show_object_with_name(FILE *out, struct object *obj, const char *name)\n> {\n> - const char *p;\n> -\n> fprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> - for (p = name; *p && *p != '\\n'; p++)\n> + for (const char *p = name; *p && *p != '\\n'; p++)\n> fputc(*p, out);\n> fputc('\\n', out);\n> }\n\nThis seems like a stable-enough function for this experiment.\n\nSimilar to 765dc16888 (\"git-compat-util: always enable variadic macros\",\n2021-01-28), maybe we should add something like\n\n /*\n * This \"for (const char *p = ...\" is made as a first step towards\n * making use of such declarations elsewhere in our codebase. If\n * it causes compilation problems on your platform, please report\n * it to the Git mailing list at git@vger.kernel.org.\n */\n\nto reduce the chance of someone patching it up locally thinking that\nit's just a one-off.\n\nMartin\n"},{"id":"441199","messageId":"YZLemOWM0rAuRTRe@camp.crustytoothpaste.net","threadId":"56887","inReplyTo":"xmqqpmr2j5lq.fsf_-_@gitster.g","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-11-15T22:26:32Z","receivedAt":"2021-11-15T23:29:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-11-15 at 06:27:45, Junio C Hamano wrote:\n> There are certain C99 features that might be nice to use in our code\n> base, but we've hesitated to do so in order to avoid breaking\n> compatibility with older compilers. But we don't actually know if\n> people are even using pre-C99 compilers these days.\n> \n> One way to figure that out is to introduce a very small use of a\n> feature, and see if anybody complains, and we've done so to probe\n> the portability for a few features like \"trailing comma in enum\n> declaration\", \"designated initializer for struct\", and \"designated\n> initializer for array\". A few years ago, we tried to use a handy\n> \n> for (int i = 0; i < n; i++)\n> \tuse(i);\n> \n> to introduce a new variable valid only in the loop, but found that\n> some compilers we cared about didn't like it back then. Two years\n> is a long-enough time, so let's try it agin.\n\nI think you absolutely need a compiler option for this to work on older\nsystems. Many of those compilers support C99 just fine but need an\noption to enable it.\n\nI think this could go on top of my patch, though.\n\n> If this patch can survive a few releases without complaint, then we\n> can feel more confident that variable declaration in for() loop is\n> supported by the compilers our user base use. And if we do get\n> complaints, then we'll have gained some data and we can easily\n> revert this patch.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> revision.c | 4 +---\n> 1 file changed, 1 insertion(+), 3 deletions(-)\n> \n> diff --git a/revision.c b/revision.c\n> index 9dff845bed..44492f2c02 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -43,10 +43,8 @@ static inline int want_ancestry(const struct rev_info *revs);\n> \n> void show_object_with_name(FILE *out, struct object *obj, const char *name)\n> {\n> -\tconst char *p;\n> -\n> \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> -\tfor (p = name; *p && *p != '\\n'; p++)\n> +\tfor (const char *p = name; *p && *p != '\\n'; p++)\n> \t\tfputc(*p, out);\n> \tfputc('\\n', out);\n> }\n> -- \n> 2.34.0-rc2-165-g9b3c04af29\n> \n\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"441263","messageId":"xmqqee7gbj1n.fsf@gitster.g","threadId":"56887","inReplyTo":"CAN0heSpLy8c6WM1UpyEXJLfmnX=5B0eFhJwH3wqSZN15HAJGeg@mail.gmail.com","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-16T08:29:08Z","receivedAt":"2021-11-16T08:29:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Ågren writes:\n\n> On Mon, 15 Nov 2021 at 07:30, Junio C Hamano wrote:\n>>\n>> There are certain C99 features that might be nice to use in our code\n>> base, but we've hesitated to do so in order to avoid breaking\n>> compatibility with older compilers. But we don't actually know if\n>> people are even using pre-C99 compilers these days.\n>\n>> is a long-enough time, so let's try it agin.\n>\n> s/agin/again/\n>\n>> void show_object_with_name(FILE *out, struct object *obj, const char *name)\n>> {\n>> - const char *p;\n>> -\n>> fprintf(out, \"%s \", oid_to_hex(&obj->oid));\n>> - for (p = name; *p && *p != '\\n'; p++)\n>> + for (const char *p = name; *p && *p != '\\n'; p++)\n>> fputc(*p, out);\n>> fputc('\\n', out);\n>> }\n>\n> This seems like a stable-enough function for this experiment.\n\nYup, the callers and the implementation are from several years ago,\nif I am not mistaken.\n\n> Similar to 765dc16888 (\"git-compat-util: always enable variadic macros\",\n> 2021-01-28), maybe we should add something like\n>\n> /*\n> * This \"for (const char *p = ...\" is made as a first step towards\n> * making use of such declarations elsewhere in our codebase. If\n> * it causes compilation problems on your platform, please report\n> * it to the Git mailing list at git@vger.kernel.org.\n> */\n>\n> to reduce the chance of someone patching it up locally thinking that\n> it's just a one-off.\n\nProbably. It would help people to refrain from copying and pasting\nthis and making it harder to back out, too.\n"},{"id":"441469","messageId":"61518213-9ce8-00d2-efd9-7f2091c574c4@gmail.com","threadId":"56887","inReplyTo":"xmqqpmr2j5lq.fsf_-_@gitster.g","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-11-17T11:03:58Z","receivedAt":"2021-11-17T11:04:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 15/11/2021 06:27, Junio C Hamano wrote:\n> There are certain C99 features that might be nice to use in our code\n> base, but we've hesitated to do so in order to avoid breaking\n> compatibility with older compilers. But we don't actually know if\n> people are even using pre-C99 compilers these days.\n> \n> One way to figure that out is to introduce a very small use of a\n> feature, and see if anybody complains, and we've done so to probe\n> the portability for a few features like \"trailing comma in enum\n> declaration\", \"designated initializer for struct\", and \"designated\n> initializer for array\". A few years ago, we tried to use a handy\n> \n> for (int i = 0; i < n; i++)\n> \tuse(i);\n> \n> to introduce a new variable valid only in the loop, but found that\n> some compilers we cared about didn't like it back then. Two years\n> is a long-enough time, so let's try it agin.\n> \n> If this patch can survive a few releases without complaint, then we\n> can feel more confident that variable declaration in for() loop is\n> supported by the compilers our user base use. And if we do get\n> complaints, then we'll have gained some data and we can easily\n> revert this patch.\n\nI like the idea of using a specific test balloon for the features that \nwe want to use but wont this one break the build for anyone doing 'make \nDEVELOPER=1' because -Wdeclaration-after-statement will error out. I \nthink we could wrap the loop in gcc's warning pragmas to avoid that.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Junio C Hamano \n> ---\n> revision.c | 4 +---\n> 1 file changed, 1 insertion(+), 3 deletions(-)\n> \n> diff --git a/revision.c b/revision.c\n> index 9dff845bed..44492f2c02 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -43,10 +43,8 @@ static inline int want_ancestry(const struct rev_info *revs);\n> \n> void show_object_with_name(FILE *out, struct object *obj, const char *name)\n> {\n> -\tconst char *p;\n> -\n> \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> -\tfor (p = name; *p && *p != '\\n'; p++)\n> +\tfor (const char *p = name; *p && *p != '\\n'; p++)\n> \t\tfputc(*p, out);\n> \tfputc('\\n', out);\n> }\n> \n"},{"id":"441479","messageId":"211117.86bl2j6j6z.gmgdl@evledraar.gmail.com","threadId":"56887","inReplyTo":"61518213-9ce8-00d2-efd9-7f2091c574c4@gmail.com","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-17T12:39:33Z","receivedAt":"2021-11-17T12:49:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 17 2021, Phillip Wood wrote:\n\n> Hi Junio\n>\n> On 15/11/2021 06:27, Junio C Hamano wrote:\n>> There are certain C99 features that might be nice to use in our code\n>> base, but we've hesitated to do so in order to avoid breaking\n>> compatibility with older compilers. But we don't actually know if\n>> people are even using pre-C99 compilers these days.\n>> One way to figure that out is to introduce a very small use of a\n>> feature, and see if anybody complains, and we've done so to probe\n>> the portability for a few features like \"trailing comma in enum\n>> declaration\", \"designated initializer for struct\", and \"designated\n>> initializer for array\". A few years ago, we tried to use a handy\n>> for (int i = 0; i < n; i++)\n>> \tuse(i);\n>> to introduce a new variable valid only in the loop, but found that\n>> some compilers we cared about didn't like it back then. Two years\n>> is a long-enough time, so let's try it agin.\n>> If this patch can survive a few releases without complaint, then we\n>> can feel more confident that variable declaration in for() loop is\n>> supported by the compilers our user base use. And if we do get\n>> complaints, then we'll have gained some data and we can easily\n>> revert this patch.\n>\n> I like the idea of using a specific test balloon for the features that\n> we want to use but wont this one break the build for anyone doing\n> 'make DEVELOPER=1' because -Wdeclaration-after-statement will error\n> out. I think we could wrap the loop in gcc's warning pragmas to avoid\n> that.\n\nGood point.\n\nOverall something that brings us to the end-state 765dc168882\n(git-compat-util: always enable variadic macros, 2021-01-28) brought us\nto is probably better, i.e. something you can work around by defining or\nundefining a macro via a Makefile parameter, instead of needing to patch\ngit's sources.\n\n\n"},{"id":"441529","messageId":"20211117223002.GC5811@szeder.dev","threadId":"56887","inReplyTo":"61518213-9ce8-00d2-efd9-7f2091c574c4@gmail.com","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-11-17T22:30:02Z","receivedAt":"2021-11-17T22:30:19Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Nov 17, 2021 at 11:03:58AM +0000, Phillip Wood wrote:\n> Hi Junio\n> \n> On 15/11/2021 06:27, Junio C Hamano wrote:\n> > There are certain C99 features that might be nice to use in our code\n> > base, but we've hesitated to do so in order to avoid breaking\n> > compatibility with older compilers. But we don't actually know if\n> > people are even using pre-C99 compilers these days.\n> > \n> > One way to figure that out is to introduce a very small use of a\n> > feature, and see if anybody complains, and we've done so to probe\n> > the portability for a few features like \"trailing comma in enum\n> > declaration\", \"designated initializer for struct\", and \"designated\n> > initializer for array\". A few years ago, we tried to use a handy\n> > \n> > for (int i = 0; i < n; i++)\n> > \tuse(i);\n> > \n> > to introduce a new variable valid only in the loop, but found that\n> > some compilers we cared about didn't like it back then. Two years\n> > is a long-enough time, so let's try it agin.\n> > \n> > If this patch can survive a few releases without complaint, then we\n> > can feel more confident that variable declaration in for() loop is\n> > supported by the compilers our user base use. And if we do get\n> > complaints, then we'll have gained some data and we can easily\n> > revert this patch.\n> \n> I like the idea of using a specific test balloon for the features that we\n> want to use but wont this one break the build for anyone doing 'make\n> DEVELOPER=1' because -Wdeclaration-after-statement will error out. I think\n> we could wrap the loop in gcc's warning pragmas to avoid that.\n\nThe scope of the loop variable is limited to the loop, so I don't\nthink this is considered as declaration after statement, just like\nother variable declarations in limited scopes that are abundant in\nGit's codebase, e.g.:\n\n printf(\"...\");\n if (var) {\n int a;\n ...\n }\n\nFWIW, I've spent some time with Compiler Explorer compiling a for loop\ninitial declaration after a statement with '-std=c99 -Werror\n-Wdeclaration-after-statement', and none of them complained (though\nthere were some that didn't understand the '-std=c99' or '-Wdecl...'\noptions or couldn't compile it for some other reason).\n\n"},{"id":"441574","messageId":"xmqq1r3eym7f.fsf@gitster.g","threadId":"56887","inReplyTo":"61518213-9ce8-00d2-efd9-7f2091c574c4@gmail.com","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-18T07:09:08Z","receivedAt":"2021-11-18T07:10:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I like the idea of using a specific test balloon for the features that\n> we want to use but wont this one break the build for anyone doing\n> 'make DEVELOPER=1' because -Wdeclaration-after-statement will error\n> out.\n\nI think you are missing '?' at the end of the sentence, but the\nanswer is \"no, at least not for me\".\n\n # pardon my \"make\" wrapper; it is to pass DEVELOPER=1 etc. to\n # the underlying \"make\" command.\n $ Meta/Make V=1 revision.o\n cc -o revision.o -c -MF ./.depend/revision.o.d -MQ revision.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -fno-common -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"cache.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH=\"/proc/self/exe\"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' -DPAGER_ENV='\"LESS=FRX LV=-c\"' revision.c\n $ cc --version\n cc (Debian 10.3.0-11) 10.3.0\n Copyright (C) 2020 Free Software Foundation, Inc.\n This is free software; see the source for copying conditions. There is NO\n warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\n\nIt would be quite sad if we had to allow decl-after-stmt, only to\nallow\n\n\tstmt;\n\tfor (type var = init; ...; ...) {\n\t\t...;\n\t}\n\nbecause it should merely be a short-hand for\n\n\tstmt;\n\t{\n\t type var;\n\t for (var = init; ...; ...) {\n\t\t...;\n\t }\n\t}\n\nthat does not need to allow decl-after-stmt.\n\nDifferent compilers may behave differently, so it might be an issue\nfor somebody else, but I am hoping any reasonable compiler would\nbehave sensibly.\n\nThanks for raising a potential issue, as others can try it out in\ntheir environment and see if their compilers behave well.\n\n\n\n"},{"id":"443265","messageId":"836296d0-6eee-7c6b-04d0-d93909948611@gmail.com","threadId":"56887","inReplyTo":"xmqq1r3eym7f.fsf@gitster.g","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-12-07T11:10:14Z","receivedAt":"2021-12-07T11:10:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 18/11/2021 07:09, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> I like the idea of using a specific test balloon for the features that\n>> we want to use but wont this one break the build for anyone doing\n>> 'make DEVELOPER=1' because -Wdeclaration-after-statement will error\n>> out.\n> \n> I think you are missing '?' at the end of the sentence,\n\nsorry yes I am\n\n> but the\n> answer is \"no, at least not for me\".\n> \n> # pardon my \"make\" wrapper; it is to pass DEVELOPER=1 etc. to\n> # the underlying \"make\" command.\n> $ Meta/Make V=1 revision.o\n> cc -o revision.o -c -MF ./.depend/revision.o.d -MQ revision.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -fno-common -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"cache.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH=\"/proc/self/exe\"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' -DPAGER_ENV='\"LESS=FRX LV=-c\"' revision.c\n> $ cc --version\n> cc (Debian 10.3.0-11) 10.3.0\n> Copyright (C) 2020 Free Software Foundation, Inc.\n> This is free software; see the source for copying conditions. There is NO\n> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n> \n> \n> It would be quite sad if we had to allow decl-after-stmt, only to\n> allow\n> \n> \tstmt;\n> \tfor (type var = init; ...; ...) {\n> \t\t...;\n> \t}\n> \n> because it should merely be a short-hand for\n> \n> \tstmt;\n> \t{\n> \t type var;\n> \t for (var = init; ...; ...) {\n> \t\t...;\n> \t }\n> \t}\n> \n> that does not need to allow decl-after-stmt.\n> \n> Different compilers may behave differently, so it might be an issue\n> for somebody else, but I am hoping any reasonable compiler would\n> behave sensibly.\n> \n> Thanks for raising a potential issue, as others can try it out in\n> their environment and see if their compilers behave well.\n\nOh it seems I misunderstood exactly what decl-after-stmt does, thinking \nabout it your explanation makes sense. I guess that means we have not \nhad any warning flags set (because no such flags exist?) to protect \nagainst the accidental introduction of the construct that this patch is \ntesting compiler support for.\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"443362","messageId":"xmqq35n46tif.fsf@gitster.g","threadId":"56887","inReplyTo":"836296d0-6eee-7c6b-04d0-d93909948611@gmail.com","subject":"Re: [PATCH] revision: use C99 declaration of variable in for() loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-07T20:37:44Z","receivedAt":"2021-12-07T20:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> ... I guess that means we\n> have not had any warning flags set (because no such flags exist?) to\n> protect against the accidental introduction of the construct that this\n> patch is testing compiler support for.\n\nI think we actually saw some instances of \"for (type var = ...\"\nslipped through the review process, only to later get caught by\nsome other means.\n\n"},{"id":"443422","messageId":"211208.86wnkfl1ni.gmgdl@evledraar.gmail.com","threadId":"56887","inReplyTo":"xmqq1r3eym7f.fsf@gitster.g","subject":"Removing -Wdeclaration-after-statement (was: [PATCH] revision: use C99 declaration of variable in for() loop)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-08T12:17:16Z","receivedAt":"2021-12-08T12:30:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 17 2021, Junio C Hamano wrote:\n\n> Phillip Wood writes:\n>\n>> I like the idea of using a specific test balloon for the features that\n>> we want to use but wont this one break the build for anyone doing\n>> 'make DEVELOPER=1' because -Wdeclaration-after-statement will error\n>> out.\n>\n> I think you are missing '?' at the end of the sentence, but the\n> answer is \"no, at least not for me\".\n>\n> # pardon my \"make\" wrapper; it is to pass DEVELOPER=1 etc. to\n> # the underlying \"make\" command.\n> $ Meta/Make V=1 revision.o\n> cc -o revision.o -c -MF ./.depend/revision.o.d -MQ revision.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -fno-common -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"cache.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH=\"/proc/self/exe\"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' -DPAGER_ENV='\"LESS=FRX LV=-c\"' revision.c\n> $ cc --version\n> cc (Debian 10.3.0-11) 10.3.0\n> Copyright (C) 2020 Free Software Foundation, Inc.\n> This is free software; see the source for copying conditions. There is NO\n> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n>\n>\n> It would be quite sad if we had to allow decl-after-stmt, only to\n> allow\n>\n> \tstmt;\n> \tfor (type var = init; ...; ...) {\n> \t\t...;\n> \t}\n>\n> because it should merely be a short-hand for\n>\n> \tstmt;\n> \t{\n> \t type var;\n> \t for (var = init; ...; ...) {\n> \t\t...;\n> \t }\n> \t}\n>\n> that does not need to allow decl-after-stmt.\n\nWhy would that be sad? The intent of -Wdeclaration-after-statement is to\ncatch C90 compatibility issues. Maybe we don't want to enable everything\nC99-related in this area at once, but why shouldn't we be removing\n-Wdeclaration-after-statement once we have a hard C99 dependency?\n\nI usually prefer declaring variables up-front just as a metter of style,\nand it usually encourages you to split up functions that are\nunnecessarily long.\n\nBut I think being able to do it in some situations also helps\nreadability. E.g. I'm re-rolling my cat-file usage topic now and spotted\nthis nice candidate (which we'd error on now with CC=gcc and\nDEVELOPER=1):\n\t\n\tdiff --git a/builtin/cat-file.c b/builtin/cat-file.c\n\tindex f5437c2d045..a43df23a7cd 100644\n\t--- a/builtin/cat-file.c\n\t+++ b/builtin/cat-file.c\n\t@@ -644,8 +644,6 @@ static int batch_option_callback(const struct option *opt,\n\t int cmd_cat_file(int argc, const char **argv, const char *prefix)\n\t {\n\t \tint opt = 0;\n\t-\tint opt_cw = 0;\n\t-\tint opt_epts = 0;\n\t \tconst char *exp_type = NULL, *obj_name = NULL;\n\t \tstruct batch_options batch = {0};\n\t \tint unknown_type = 0;\n\t@@ -708,8 +706,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)\n\t \tbatch.buffer_output = -1;\n\t \n\t \targc = parse_options(argc, argv, prefix, options, usage, 0);\n\t-\topt_cw = (opt == 'c' || opt == 'w');\n\t-\topt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');\n\t+\tconst int opt_cw = (opt == 'c' || opt == 'w');\n\t+\tconst opt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');\n\t \n\t \t/* --batch-all-objects? */\n\t \tif (opt == 'b')\n\nI.e. in this case I'm declaring a variable merely as a short-hand for\naccessing \"opt\", and due to the need for parse_options() we can't really\ndeclare it in a way that's resonable before any statement in the\nfunction.\n\nBy having -Wdeclaration-after-statement we're forced to make it\nnon-const, and having it \"const\" helps readability, you know as soon as\nyou see it that it won't be modified.\n\nThat particular example is certainly open to bikeshedding, but I think\nthe general point that it's not categorically bad holds, and therefore\nif we don't need it for compiler compatibility it's probably a good idea\nto allow it.\n"},{"id":"443484","messageId":"xmqq35n32fjs.fsf@gitster.g","threadId":"56887","inReplyTo":"211208.86wnkfl1ni.gmgdl@evledraar.gmail.com","subject":"Re: Removing -Wdeclaration-after-statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-08T17:05:11Z","receivedAt":"2021-12-08T17:05:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Why would that be sad? The intent of -Wdeclaration-after-statement is to\n> catch C90 compatibility issues. Maybe we don't want to enable everything\n> C99-related in this area at once, but why shouldn't we be removing\n> -Wdeclaration-after-statement once we have a hard C99 dependency?\n\nWe already heard from people that we do not want vla, and I agree\nthat we do not want all C99. decl-after-stmt is something I\ndefinitely do not want in our code, in order to keep the code more\nreadable by declaring the things that will be used in the scope\nupfront, with documentation if needed. It tends to encourage us to\nkeep our blocks smaller.\n"}]}