{"thread":{"id":"46343","subject":"[PATCH] strbuf: use designated initializers in STRBUF_INIT","startedAt":"2017-07-10T07:04:09Z","lastAt":"2017-07-24T18:06:01Z","messageCount":39,"participants":["Jeff King","Ben Peart","Junio C Hamano","Andreas Schwab","Stefan Beller","Johannes Sixt","Brandon Williams","brian m. carlson","Mike Hommey","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"324050","messageId":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","threadId":"46343","inReplyTo":null,"subject":"[PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-10T07:03:42Z","receivedAt":"2017-07-10T07:04:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 09, 2017 at 10:05:49AM -0700, Junio C Hamano wrote:\n\n> René Scharfe writes:\n> \n> > I wonder when we can begin to target C99 in git's source, though. :)\n> \n> Let's get the ball rolling by starting to use some of the useful\n> features like designated initializers, perhaps, in a small, critical\n> and reasonably stable part of the system that anybody must compile,\n> leave it in one full release cycle or two, and when we hear nobody\n> complains, introduce it en masse for the remainder of the system?\n> \n> That way, we will see if there are people who need pre-C99 soon\n> enough, and we won't have to scramble reverting too many changes\n> when it happens.\n\nNeat idea. Something like this?\n\n-- >8 --\nSubject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n\nThere are certain C99 features that might be nice to use in\nour code base, but we've hesitated to do so in order to\navoid breaking compatibility with older compilers. But we\ndon't actually know if people are even using pre-C99\ncompilers these days.\n\nOne way to figure that out is to introduce a very small use\nof a feature, and see if anybody complains. The strbuf code\nis a good place to do this for a few reasons:\n\n - it always gets compiled, no matter which Makefile knobs\n have been tweaked.\n\n - it's very stable; this definition hasn't changed in a\n long time and is not likely to (so if we have to revert,\n it's unlikely to cause headaches)\n\nIf this patch can survive a few releases without complaint,\nthen we can feel more confident that designated initializers\nare widely supported by our user base. It also is an\nindication that other C99 features may be supported, but not\na guarantee (e.g., gcc had designated initializers before\nC99 existed).\n\nAnd if we do get complaints, then we'll have gained some\ndata and we can easily revert this patch.\n\nSigned-off-by: Jeff King \n---\nI suspected we could also do something with __STDC_VERSION__, though I\nwonder what compilers set it to when not in standards-compliant mode.\ngcc-6 claims C11 when no specific -std flag is given.\n\nAnd obviously before releasing this or anything similar, it would be\nnice to see results from people building pu. I'm especially curious\nwhether MSVC would work with this (or if people even still use it, since\nGit for Windows is pretty mature?).\n\n strbuf.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/strbuf.h b/strbuf.h\nindex 2075384e0..e705b94db 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -68,7 +68,7 @@ struct strbuf {\n };\n \n extern char strbuf_slopbuf[];\n-#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n+#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n \n /**\n * Life Cycle Functions\n-- \n2.13.2.1071.gcd8104b61\n\n"},{"id":"324073","messageId":"4d4f2af7-60b9-5866-50bc-ecf002f74cba@gmail.com","threadId":"46343","inReplyTo":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-07-10T14:57:57Z","receivedAt":"2017-07-10T14:58:15Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/10/2017 3:03 AM, Jeff King wrote:\n> On Sun, Jul 09, 2017 at 10:05:49AM -0700, Junio C Hamano wrote:\n> \n>> René Scharfe writes:\n>>\n>>> I wonder when we can begin to target C99 in git's source, though. :)\n>>\n>> Let's get the ball rolling by starting to use some of the useful\n>> features like designated initializers, perhaps, in a small, critical\n>> and reasonably stable part of the system that anybody must compile,\n>> leave it in one full release cycle or two, and when we hear nobody\n>> complains, introduce it en masse for the remainder of the system?\n>>\n>> That way, we will see if there are people who need pre-C99 soon\n>> enough, and we won't have to scramble reverting too many changes\n>> when it happens.\n> \n> Neat idea. Something like this?\n> \n> -- >8 --\n> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n> \n> There are certain C99 features that might be nice to use in\n> our code base, but we've hesitated to do so in order to\n> avoid breaking compatibility with older compilers. But we\n> don't actually know if people are even using pre-C99\n> compilers these days.\n> \n> One way to figure that out is to introduce a very small use\n> of a feature, and see if anybody complains. The strbuf code\n> is a good place to do this for a few reasons:\n> \n> - it always gets compiled, no matter which Makefile knobs\n> have been tweaked.\n> \n> - it's very stable; this definition hasn't changed in a\n> long time and is not likely to (so if we have to revert,\n> it's unlikely to cause headaches)\n> \n> If this patch can survive a few releases without complaint,\n> then we can feel more confident that designated initializers\n> are widely supported by our user base. It also is an\n> indication that other C99 features may be supported, but not\n> a guarantee (e.g., gcc had designated initializers before\n> C99 existed).\n> \n\nCorrect. MSVC also supports designated initializers but does not fully \nsupport C99.\n\n> And if we do get complaints, then we'll have gained some\n> data and we can easily revert this patch.\n> \n> Signed-off-by: Jeff King \n> ---\n> I suspected we could also do something with __STDC_VERSION__, though I\n> wonder what compilers set it to when not in standards-compliant mode.\n> gcc-6 claims C11 when no specific -std flag is given.\n> \n> And obviously before releasing this or anything similar, it would be\n> nice to see results from people building pu. I'm especially curious\n> whether MSVC would work with this (or if people even still use it, since\n> Git for Windows is pretty mature?).\n> \n\nWe do use MSVC internally as that gives us access to the great debuggers \nand profilers on the Windows platform. Fortunately, this particular C99 \nconstruct _is_ supported by MSVC. I applied the patch below and \ncomplied it with both MSVC and gcc for Windows and both builds succeeded.\n\n> strbuf.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/strbuf.h b/strbuf.h\n> index 2075384e0..e705b94db 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -68,7 +68,7 @@ struct strbuf {\n> };\n> \n> extern char strbuf_slopbuf[];\n> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n> \n> /**\n> * Life Cycle Functions\n> \n"},{"id":"324077","messageId":"20170710160432.4mmcoqh7yi5cqdhy@sigill.intra.peff.net","threadId":"46343","inReplyTo":"4d4f2af7-60b9-5866-50bc-ecf002f74cba@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-10T16:04:32Z","receivedAt":"2017-07-10T16:04:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 10, 2017 at 10:57:57AM -0400, Ben Peart wrote:\n\n> > If this patch can survive a few releases without complaint,\n> > then we can feel more confident that designated initializers\n> > are widely supported by our user base. It also is an\n> > indication that other C99 features may be supported, but not\n> > a guarantee (e.g., gcc had designated initializers before\n> > C99 existed).\n> \n> Correct. MSVC also supports designated initializers but does not fully\n> support C99.\n\nOut of curiosity, does MSVC define __STDC_VERSION__, and if so, to what?\n\n> > And obviously before releasing this or anything similar, it would be\n> > nice to see results from people building pu. I'm especially curious\n> > whether MSVC would work with this (or if people even still use it, since\n> > Git for Windows is pretty mature?).\n> \n> We do use MSVC internally as that gives us access to the great debuggers and\n> profilers on the Windows platform. Fortunately, this particular C99\n> construct _is_ supported by MSVC. I applied the patch below and complied it\n> with both MSVC and gcc for Windows and both builds succeeded.\n\nThanks. This kind of prompt testing and response is very appreciated. It\nis unfortunate if we have to pick and choose C99-isms rather than using\nthe whole thing as a base. But that's probably just reality.\n\n-Peff\n"},{"id":"324084","messageId":"xmqqr2xo6y2s.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-10T16:44:43Z","receivedAt":"2017-07-10T16:44:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> That way, we will see if there are people who need pre-C99 soon\n>> enough, and we won't have to scramble reverting too many changes\n>> when it happens.\n>\n> Neat idea. Something like this?\n\nYes, your log message said everything I wanted to say, including\npossiblity that some compilers may have specific features without\nsupporting all of c99.\n\nWe accidentally started using \"trailing comma at the end of enum\ndefinition is allowed in c99\", and we know it has been safe at least\nfor a cycle. Credits goes to Brandon for 4538eef5 (\"grep: add\nsubmodules as a grep source type\", 2016-12-16).\n\n> -- >8 --\n> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n>\n> There are certain C99 features that might be nice to use in\n> our code base, but we've hesitated to do so in order to\n> avoid breaking compatibility with older compilers. But we\n> don't actually know if people are even using pre-C99\n> compilers these days.\n>\n> One way to figure that out is to introduce a very small use\n> of a feature, and see if anybody complains. The strbuf code\n> is a good place to do this for a few reasons:\n>\n> - it always gets compiled, no matter which Makefile knobs\n> have been tweaked.\n>\n> - it's very stable; this definition hasn't changed in a\n> long time and is not likely to (so if we have to revert,\n> it's unlikely to cause headaches)\n>\n> If this patch can survive a few releases without complaint,\n> then we can feel more confident that designated initializers\n> are widely supported by our user base. It also is an\n> indication that other C99 features may be supported, but not\n> a guarantee (e.g., gcc had designated initializers before\n> C99 existed).\n>\n> And if we do get complaints, then we'll have gained some\n> data and we can easily revert this patch.\n>\n> Signed-off-by: Jeff King \n> ---\n> I suspected we could also do something with __STDC_VERSION__, though I\n> wonder what compilers set it to when not in standards-compliant mode.\n> gcc-6 claims C11 when no specific -std flag is given.\n>\n> And obviously before releasing this or anything similar, it would be\n> nice to see results from people building pu. I'm especially curious\n> whether MSVC would work with this (or if people even still use it, since\n> Git for Windows is pretty mature?).\n>\n> strbuf.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/strbuf.h b/strbuf.h\n> index 2075384e0..e705b94db 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -68,7 +68,7 @@ struct strbuf {\n> };\n> \n> extern char strbuf_slopbuf[];\n> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n> \n> /**\n> * Life Cycle Functions\n"},{"id":"324089","messageId":"87wp7g9pzx.fsf@linux-m68k.org","threadId":"46343","inReplyTo":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-07-10T17:10:58Z","receivedAt":"2017-07-10T17:32:40Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Jul 10 2017, Jeff King wrote:\n\n> If this patch can survive a few releases without complaint,\n> then we can feel more confident that designated initializers\n> are widely supported by our user base. It also is an\n> indication that other C99 features may be supported, but not\n> a guarantee (e.g., gcc had designated initializers before\n> C99 existed).\n\nNote that the GNU C designated initializers initially used a different\nsyntax than the one C99 adopted.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"324090","messageId":"CAGZ79kbbqj++1HZVY3gBaghn+knWa+cAgwWUw9Z+zdNkjQHCWg@mail.gmail.com","threadId":"46343","inReplyTo":"xmqqr2xo6y2s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-10T17:33:37Z","receivedAt":"2017-07-10T17:33:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 10, 2017 at 9:44 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>>> That way, we will see if there are people who need pre-C99 soon\n>>> enough, and we won't have to scramble reverting too many changes\n>>> when it happens.\n>>\n>> Neat idea. Something like this?\n>\n> Yes, your log message said everything I wanted to say, including\n> possiblity that some compilers may have specific features without\n> supporting all of c99.\n>\n> We accidentally started using \"trailing comma at the end of enum\n> definition is allowed in c99\", and we know it has been safe at least\n> for a cycle. Credits goes to Brandon for 4538eef5 (\"grep: add\n> submodules as a grep source type\", 2016-12-16).\n\nCredit goes to Brandon for spotting it, but the introduction of\n\"trailing comma at the end of enum definition is allowed in c99\"\nis e1327023ea (grep: refactor the concept of \"grep source\" into\nan object, 2012-02-02) IMHO, which is more time that proved this\nfeature being supported on all compilers.\n\nThanks for getting the ball rolling, just wondering if the patch needs\na comment in the code. The commit message is very thorough though.\n\nThanks,\nStefan\n"},{"id":"324093","messageId":"15867357-5b3f-b12e-8bc3-125b3f2f18f0@gmail.com","threadId":"46343","inReplyTo":"20170710160432.4mmcoqh7yi5cqdhy@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-07-10T17:57:21Z","receivedAt":"2017-07-10T17:57:29Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 7/10/2017 12:04 PM, Jeff King wrote:\n> On Mon, Jul 10, 2017 at 10:57:57AM -0400, Ben Peart wrote:\n> \n>>> If this patch can survive a few releases without complaint,\n>>> then we can feel more confident that designated initializers\n>>> are widely supported by our user base. It also is an\n>>> indication that other C99 features may be supported, but not\n>>> a guarantee (e.g., gcc had designated initializers before\n>>> C99 existed).\n>>\n>> Correct. MSVC also supports designated initializers but does not fully\n>> support C99.\n> \n> Out of curiosity, does MSVC define __STDC_VERSION__, and if so, to what?\n\nIt does not. While it does support a subset of C99's features, it \ndoesn't support the entirety of that language so defining \n__STDC_VERSION__ would be inaccurate and misleading.\n\n> \n>>> And obviously before releasing this or anything similar, it would be\n>>> nice to see results from people building pu. I'm especially curious\n>>> whether MSVC would work with this (or if people even still use it, since\n>>> Git for Windows is pretty mature?).\n>>\n>> We do use MSVC internally as that gives us access to the great debuggers and\n>> profilers on the Windows platform. Fortunately, this particular C99\n>> construct _is_ supported by MSVC. I applied the patch below and complied it\n>> with both MSVC and gcc for Windows and both builds succeeded.\n> \n> Thanks. This kind of prompt testing and response is very appreciated. It\n> is unfortunate if we have to pick and choose C99-isms rather than using\n> the whole thing as a base. But that's probably just reality.\n> \n> -Peff\n> \n"},{"id":"324101","messageId":"c349f324-8f6d-2fe0-8982-2e37869d37b5@kdbg.org","threadId":"46343","inReplyTo":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-07-10T19:57:40Z","receivedAt":"2017-07-10T19:57:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.07.2017 um 09:03 schrieb Jeff King:\n> On Sun, Jul 09, 2017 at 10:05:49AM -0700, Junio C Hamano wrote:\n> \n>> René Scharfe writes:\n>>\n>>> I wonder when we can begin to target C99 in git's source, though. :)\n>>\n>> Let's get the ball rolling...\n\nGood to know that you do not resist moving to a more modern build \nenvironment.\n\n>> by starting to use some of the useful\n>> features like designated initializers,\n\nIt's a pity, though, that you do not suggest something even more useful, \nsuch as C++14.\n\n> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n\n> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n\nWhile this may serve as a test balloon, changing STRBUF_INIT, or any of \nthose _INIT macros, is actually the least interesting. The interesting \ninstances are initializations for which we do *not* have a macro.\n\n-- Hannes\n"},{"id":"324105","messageId":"xmqq4luk58ot.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"c349f324-8f6d-2fe0-8982-2e37869d37b5@kdbg.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-10T20:38:26Z","receivedAt":"2017-07-10T20:38:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> It's a pity, though, that you do not suggest something even more\n> useful, such as C++14.\n\nI cannot tell if this part is tongue-in-cheek (especially the \"++\"),\nso I will ignore it to avoid wasting time.\n\n>> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n>\n>> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n>> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n>\n> While this may serve as a test balloon, changing STRBUF_INIT, or any\n> of those _INIT macros, is actually the least interesting. The\n> interesting instances are initializations for which we do *not* have a\n> macro.\n\nI am not sure what negative impact you think the macro-ness would\nhave to the validity of the result from this test balloon. An old\ncompiler that does not understand designated initializer syntax\nwould fail to compile both the same way, no?\n\n\tstruct strbuf buf0 = STRBUF_INIT;\n\tstruct strbuf buf1 = { .alloc = 0, .len = 0, .buf = strbuf_slopbuf };\n"},{"id":"324106","messageId":"962da692-8874-191c-59d4-65b9562cf87f@kdbg.org","threadId":"46343","inReplyTo":"xmqq4luk58ot.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-07-10T21:11:35Z","receivedAt":"2017-07-10T21:11:43Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.07.2017 um 22:38 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n> \n>> It's a pity, though, that you do not suggest something even more\n>> useful, such as C++14.\n> \n> I cannot tell if this part is tongue-in-cheek (especially the \"++\"),\n> so I will ignore it to avoid wasting time.\n\nActually, I'm serious.\n\n>>> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n>>\n>>> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n>>> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n>>\n>> While this may serve as a test balloon, changing STRBUF_INIT, or any\n>> of those _INIT macros, is actually the least interesting. The\n>> interesting instances are initializations for which we do *not* have a\n>> macro.\n> \n> I am not sure what negative impact you think the macro-ness would\n> have to the validity of the result from this test balloon. An old\n> compiler that does not understand designated initializer syntax\n> would fail to compile both the same way, no?\n> \n> \tstruct strbuf buf0 = STRBUF_INIT;\n> \tstruct strbuf buf1 = { .alloc = 0, .len = 0, .buf = strbuf_slopbuf };\n\nI said it is uninteresting, not that there is a negative impact. There \nis simply nothing gained for strbuf users: They would use STRBUF_INIT \nbefore and after the change and would not benefit from designated \ninitializers.\n\nThis change may serve well as a test balloon, but not as an example of \nthe sort of changes that we would want to see later (of the kind \"change \nFOO_INIT macro to use designated initializers\"; they are just code churn).\n\n-- Hannes\n"},{"id":"324107","messageId":"xmqqy3rw3rc8.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"962da692-8874-191c-59d4-65b9562cf87f@kdbg.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-10T21:38:31Z","receivedAt":"2017-07-10T21:38:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> I am not sure what negative impact you think the macro-ness would\n>> have to the validity of the result from this test balloon. An old\n>> compiler that does not understand designated initializer syntax\n>> would fail to compile both the same way, no?\n>>\n>> \tstruct strbuf buf0 = STRBUF_INIT;\n>> \tstruct strbuf buf1 = { .alloc = 0, .len = 0, .buf = strbuf_slopbuf };\n>\n> I said it is uninteresting, not that there is a negative impact. There\n> is simply nothing gained for strbuf users: They would use STRBUF_INIT\n> before and after the change and would not benefit from designated\n> initializers.\n>\n> This change may serve well as a test balloon, but not as an example of\n> the sort of changes that we would want to see later (of the kind\n> \"change FOO_INIT macro to use designated initializers\"; they are just\n> code churn).\n\nOh, absolutely.\n\nHere is another possible test balloon, that may actually be useful\nas an example. I think there is a topic in flight that touches this\narray, unfortunately, so I probably would find another one that is\nmore stable for a real follow-up patch to the one from Peff.\n\n diff.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 00b4c86698..b3864a2e03 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -47,15 +47,15 @@ static long diff_algorithm;\n static unsigned ws_error_highlight_default = WSEH_NEW;\n \n static char diff_colors[][COLOR_MAXLEN] = {\n-\tGIT_COLOR_RESET,\n-\tGIT_COLOR_NORMAL,\t/* CONTEXT */\n-\tGIT_COLOR_BOLD,\t\t/* METAINFO */\n-\tGIT_COLOR_CYAN,\t\t/* FRAGINFO */\n-\tGIT_COLOR_RED,\t\t/* OLD */\n-\tGIT_COLOR_GREEN,\t/* NEW */\n-\tGIT_COLOR_YELLOW,\t/* COMMIT */\n-\tGIT_COLOR_BG_RED,\t/* WHITESPACE */\n-\tGIT_COLOR_NORMAL,\t/* FUNCINFO */\n+\t[DIFF_RESET] = GIT_COLOR_RESET,\n+\t[DIFF_CONTEXT] = GIT_COLOR_NORMAL,\n+\t[DIFF_METAINFO] = GIT_COLOR_BOLD,\n+\t[DIFF_FRAGINFO] = GIT_COLOR_CYAN,\n+\t[DIFF_FILE_OLD] = GIT_COLOR_RED,\n+\t[DIFF_FILE_NEW] = GIT_COLOR_GREEN,\n+\t[DIFF_COMMIT] = GIT_COLOR_YELLOW,\n+\t[DIFF_WHITESPACE] = GIT_COLOR_BG_RED,\n+\t[DIFF_FUNCINFO] = GIT_COLOR_NORMAL,\n };\n \n static NORETURN void die_want_option(const char *option_name)\n"},{"id":"324108","messageId":"xmqqtw2k3qzb.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"CAGZ79kbbqj++1HZVY3gBaghn+knWa+cAgwWUw9Z+zdNkjQHCWg@mail.gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-10T21:46:16Z","receivedAt":"2017-07-10T21:46:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Mon, Jul 10, 2017 at 9:44 AM, Junio C Hamano wrote:\n>\n> Credit goes to Brandon for spotting it, but the introduction of\n> \"trailing comma at the end of enum definition is allowed in c99\"\n> is e1327023ea (grep: refactor the concept of \"grep source\" into\n> an object, 2012-02-02) IMHO, which is more time that proved this\n> feature being supported on all compilers.\n\nYup, I did dig down to that commit earlier when I wrote\n\n https://public-inbox.org/git/xmqqlgolm2jk.fsf@gitster.mtv.corp.google.com/\n\nI just forgot about it, as very many things going on the list ;-)\n\nThanks.\n"},{"id":"324118","messageId":"20170710224134.GA161700@google.com","threadId":"46343","inReplyTo":"20170710070342.txmlwwq6gvjkwtw7@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-07-10T22:41:34Z","receivedAt":"2017-07-10T22:41:42Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 07/10, Jeff King wrote:\n> On Sun, Jul 09, 2017 at 10:05:49AM -0700, Junio C Hamano wrote:\n> \n> > René Scharfe writes:\n> > \n> > > I wonder when we can begin to target C99 in git's source, though. :)\n> > \n> > Let's get the ball rolling by starting to use some of the useful\n> > features like designated initializers, perhaps, in a small, critical\n> > and reasonably stable part of the system that anybody must compile,\n> > leave it in one full release cycle or two, and when we hear nobody\n> > complains, introduce it en masse for the remainder of the system?\n> > \n> > That way, we will see if there are people who need pre-C99 soon\n> > enough, and we won't have to scramble reverting too many changes\n> > when it happens.\n> \n> Neat idea. Something like this?\n> \n> -- >8 --\n> Subject: [PATCH] strbuf: use designated initializers in STRBUF_INIT\n> \n> There are certain C99 features that might be nice to use in\n> our code base, but we've hesitated to do so in order to\n> avoid breaking compatibility with older compilers. But we\n> don't actually know if people are even using pre-C99\n> compilers these days.\n> \n> One way to figure that out is to introduce a very small use\n> of a feature, and see if anybody complains. The strbuf code\n> is a good place to do this for a few reasons:\n> \n> - it always gets compiled, no matter which Makefile knobs\n> have been tweaked.\n> \n> - it's very stable; this definition hasn't changed in a\n> long time and is not likely to (so if we have to revert,\n> it's unlikely to cause headaches)\n> \n> If this patch can survive a few releases without complaint,\n> then we can feel more confident that designated initializers\n> are widely supported by our user base. It also is an\n> indication that other C99 features may be supported, but not\n> a guarantee (e.g., gcc had designated initializers before\n> C99 existed).\n> \n> And if we do get complaints, then we'll have gained some\n> data and we can easily revert this patch.\n> \n> Signed-off-by: Jeff King \n> ---\n> I suspected we could also do something with __STDC_VERSION__, though I\n> wonder what compilers set it to when not in standards-compliant mode.\n> gcc-6 claims C11 when no specific -std flag is given.\n> \n> And obviously before releasing this or anything similar, it would be\n> nice to see results from people building pu. I'm especially curious\n> whether MSVC would work with this (or if people even still use it, since\n> Git for Windows is pretty mature?).\n> \n> strbuf.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/strbuf.h b/strbuf.h\n> index 2075384e0..e705b94db 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -68,7 +68,7 @@ struct strbuf {\n> };\n> \n> extern char strbuf_slopbuf[];\n> -#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n> +#define STRBUF_INIT { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }\n\nI love that this is happening! And maybe someday soon we can do:\n\n for (int i = 0; i < n; i++)\n\nSo that we can scope loop variables to the loops themselves.\n\n-- \nBrandon Williams\n"},{"id":"324140","messageId":"20170711000540.4tdytyiit27trmzo@genre.crustytoothpaste.net","threadId":"46343","inReplyTo":"c349f324-8f6d-2fe0-8982-2e37869d37b5@kdbg.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-11T00:05:40Z","receivedAt":"2017-07-11T00:05:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jul 10, 2017 at 09:57:40PM +0200, Johannes Sixt wrote:\n> It's a pity, though, that you do not suggest something even more useful,\n> such as C++14.\n\nI have tried compiling Git with a C++ compiler, so that I could test\nwhether that was a viable alternative for MSVC in case its C++ mode\nsupported features its C mode did not. Let's just say that the\ncompilation aborted very quickly and I gave up after a few minutes.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"324141","messageId":"CAGZ79kbWQT9n9BAmoHQsZfaEC5GahgOfOVAQGTSGWU1Np8=Nkw@mail.gmail.com","threadId":"46343","inReplyTo":"20170711000540.4tdytyiit27trmzo@genre.crustytoothpaste.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-11T00:07:43Z","receivedAt":"2017-07-11T00:07:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 10, 2017 at 5:05 PM, brian m. carlson\n wrote:\n> On Mon, Jul 10, 2017 at 09:57:40PM +0200, Johannes Sixt wrote:\n>> It's a pity, though, that you do not suggest something even more useful,\n>> such as C++14.\n>\n> I have tried compiling Git with a C++ compiler, so that I could test\n> whether that was a viable alternative for MSVC in case its C++ mode\n> supported features its C mode did not. Let's just say that the\n> compilation aborted very quickly and I gave up after a few minutes.\n\n... because we use reserved C++ keywords such as 'new' as variable names?\n"},{"id":"324142","messageId":"20170711001042.73ht73fvecjoprt5@genre.crustytoothpaste.net","threadId":"46343","inReplyTo":"CAGZ79kbWQT9n9BAmoHQsZfaEC5GahgOfOVAQGTSGWU1Np8=Nkw@mail.gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-11T00:10:42Z","receivedAt":"2017-07-11T00:10:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jul 10, 2017 at 05:07:43PM -0700, Stefan Beller wrote:\n> On Mon, Jul 10, 2017 at 5:05 PM, brian m. carlson\n> wrote:\n> > On Mon, Jul 10, 2017 at 09:57:40PM +0200, Johannes Sixt wrote:\n> >> It's a pity, though, that you do not suggest something even more useful,\n> >> such as C++14.\n> >\n> > I have tried compiling Git with a C++ compiler, so that I could test\n> > whether that was a viable alternative for MSVC in case its C++ mode\n> > supported features its C mode did not. Let's just say that the\n> > compilation aborted very quickly and I gave up after a few minutes.\n> \n> ... because we use reserved C++ keywords such as 'new' as variable names?\n\nYes, that was part of it (\"template\" stuck out to me). I don't remember\nall the issues, but they seemed quite numerous. I'm sure it can be\ndone, though, if it's valuable to someone.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"324148","messageId":"20170711043831.cffutjnrxo3pcyst@sigill.intra.peff.net","threadId":"46343","inReplyTo":"962da692-8874-191c-59d4-65b9562cf87f@kdbg.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-11T04:38:31Z","receivedAt":"2017-07-11T04:38:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 10, 2017 at 11:11:35PM +0200, Johannes Sixt wrote:\n\n> > I am not sure what negative impact you think the macro-ness would\n> > have to the validity of the result from this test balloon. An old\n> > compiler that does not understand designated initializer syntax\n> > would fail to compile both the same way, no?\n> > \n> > \tstruct strbuf buf0 = STRBUF_INIT;\n> > \tstruct strbuf buf1 = { .alloc = 0, .len = 0, .buf = strbuf_slopbuf };\n> \n> I said it is uninteresting, not that there is a negative impact. There is\n> simply nothing gained for strbuf users: They would use STRBUF_INIT before\n> and after the change and would not benefit from designated initializers.\n> \n> This change may serve well as a test balloon, but not as an example of the\n> sort of changes that we would want to see later (of the kind \"change\n> FOO_INIT macro to use designated initializers\"; they are just code churn).\n\nBut that is exactly the point. First the test balloon, wait a release or\ntwo, and then make real useful changes. The purpose of the test balloon\nis to gather data with minimal impact. To be useful, the changes would\nbe pervasive, and that would not have minimal impact.\n\n-Peff\n"},{"id":"324151","messageId":"20170711050128.zi63rjltzolfm4ag@glandium.org","threadId":"46343","inReplyTo":"4d4f2af7-60b9-5866-50bc-ecf002f74cba@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2017-07-11T05:01:28Z","receivedAt":"2017-07-11T05:01:52Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, Jul 10, 2017 at 10:57:57AM -0400, Ben Peart wrote:\n> Correct. MSVC also supports designated initializers but does not fully\n> support C99.\n\nPrecision: *recent versions* of MSVC support designated initializer.\n2013 introduced them, but there were bugs until 2015, see e.g.\nhttps://stackoverflow.com/questions/24090739/possible-compiler-bug-in-msvc12-vs2013-with-designated-initializer\n\nMike\n"},{"id":"324152","messageId":"577e31a0-ecb2-d0a0-b259-b65b31116be8@kdbg.org","threadId":"46343","inReplyTo":"20170711000540.4tdytyiit27trmzo@genre.crustytoothpaste.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-07-11T05:24:07Z","receivedAt":"2017-07-11T05:24:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.07.2017 um 02:05 schrieb brian m. carlson:\n> On Mon, Jul 10, 2017 at 09:57:40PM +0200, Johannes Sixt wrote:\n>> It's a pity, though, that you do not suggest something even more useful,\n>> such as C++14.\n> \n> I have tried compiling Git with a C++ compiler, so that I could test\n> whether that was a viable alternative for MSVC in case its C++ mode\n> supported features its C mode did not. Let's just say that the\n> compilation aborted very quickly and I gave up after a few minutes.\n\nIt's 3 cleanup patches and one hacky patch with this size:\n\n 80 files changed, 899 insertions(+), 807 deletions(-)\n\nto compile with C++. It passed the test suite last time I tried. Getting \nrid of the remaining 1000+ -fpermissive warnings is a different matter, \nthough.\n\nI can revive the patches if there is interest.\n\n-- Hannes\n"},{"id":"324196","messageId":"xmqqeftn3s7t.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"4d4f2af7-60b9-5866-50bc-ecf002f74cba@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-11T15:31:50Z","receivedAt":"2017-07-11T15:32:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n>> If this patch can survive a few releases without complaint,\n>> then we can feel more confident that designated initializers\n>> are widely supported by our user base. It also is an\n>> indication that other C99 features may be supported, but not\n>> a guarantee (e.g., gcc had designated initializers before\n>> C99 existed).\n>\n> Correct. MSVC also supports designated initializers but does not\n> fully support C99.\n\nThanks for a datapoint.\n\nJust so that people do not misunderstand, it is not our goal to\ndeclare that now you need a fully C99 compiler to build Git.\n\nWhen deciding what shell scripting tools with what options in our\nscripted Porcelains and tests, we use POSIX.1 as a rough guideline.\nWe say \"let's not use this, as it is not even in POSIX\" but we never\nsay \"use of it is OK because it is in POSIX\", and we sometimes even\nsay \"even it is in POSIX, various implementation of it is buggy and\nit does not work properly in practice\" to certain things [*1*].\n\nC89 has been the base of such a guideline for our C programs, and\npeople must not to view this patch as an attempt to raise the base\nto C99. It is rather an attempt to see how far we can safely raise\nthe base by checking some selected useful new features [*2*] that we\nhave had occasions to wish that they were available, and designated\ninitializer for structure fields is one of them.\n\nWe may find out that, after starting with \"C89, plus this and that\nfeature that are known to be commonly supported\", the guideline may\nbecome more succinct to say \"C99, minus this and that feature that\nare not usable on some platforms\", if it turns out that majority of\nthe systems that are not fully C99 have all of the things we care\nabout. We do not know yet, and we are only at the beginning of the\njourney to find it out.\n\n\n[Footnote]\n\n*1* Like `backtick` command substitutions that is very hard to make\n them nest properly and impossible to mix portably with quoting.\n\n*2* New, relative to our current practice, that is.\n"},{"id":"324252","messageId":"20170712012631.o7jdced5hqkqzug2@genre.crustytoothpaste.net","threadId":"46343","inReplyTo":"577e31a0-ecb2-d0a0-b259-b65b31116be8@kdbg.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-07-12T01:26:31Z","receivedAt":"2017-07-12T01:26:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Jul 11, 2017 at 07:24:07AM +0200, Johannes Sixt wrote:\n> Am 11.07.2017 um 02:05 schrieb brian m. carlson:\n> > I have tried compiling Git with a C++ compiler, so that I could test\n> > whether that was a viable alternative for MSVC in case its C++ mode\n> > supported features its C mode did not. Let's just say that the\n> > compilation aborted very quickly and I gave up after a few minutes.\n> \n> It's 3 cleanup patches and one hacky patch with this size:\n> \n> 80 files changed, 899 insertions(+), 807 deletions(-)\n> \n> to compile with C++. It passed the test suite last time I tried. Getting rid\n> of the remaining 1000+ -fpermissive warnings is a different matter, though.\n\nYeah, that's the size I was thinking. My goal was \"easily achievable in\nhalf an hour,\" which it didn't seem like at the time.\n\n> I can revive the patches if there is interest.\n\nI'd be interested in at least a pointer to them if you have one. I\nthink it might allow us to take advantage of desirable features that are\nin the intersection of C99 and C++.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"324283","messageId":"9e1e9c83-95b4-30ed-e391-1a09618abce7@kdbg.org","threadId":"46343","inReplyTo":"20170712012631.o7jdced5hqkqzug2@genre.crustytoothpaste.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-07-12T18:25:12Z","receivedAt":"2017-07-12T18:25:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2017 um 03:26 schrieb brian m. carlson:\n> On Tue, Jul 11, 2017 at 07:24:07AM +0200, Johannes Sixt wrote:\n>> I can revive the patches if there is interest.\n> \n> I'd be interested in at least a pointer to them if you have one. I\n> think it might allow us to take advantage of desirable features that are\n> in the intersection of C99 and C++.\n\nSee here: https://github.com/j6t/git.git c-plus-plus\n\nI do not think that there is a lot of intersection, though.\n\n-- Hannes\n"},{"id":"324290","messageId":"87d195zcy0.fsf@gmail.com","threadId":"46343","inReplyTo":"xmqqeftn3s7t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-12T19:12:55Z","receivedAt":"2017-07-12T19:13:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jul 11 2017, Junio C. Hamano jotted:\n\n> Ben Peart writes:\n>\n>>> If this patch can survive a few releases without complaint,\n>>> then we can feel more confident that designated initializers\n>>> are widely supported by our user base. It also is an\n>>> indication that other C99 features may be supported, but not\n>>> a guarantee (e.g., gcc had designated initializers before\n>>> C99 existed).\n>>\n>> Correct. MSVC also supports designated initializers but does not\n>> fully support C99.\n>\n> Thanks for a datapoint.\n>\n> Just so that people do not misunderstand, it is not our goal to\n> declare that now you need a fully C99 compiler to build Git.\n>\n> When deciding what shell scripting tools with what options in our\n> scripted Porcelains and tests, we use POSIX.1 as a rough guideline.\n> We say \"let's not use this, as it is not even in POSIX\" but we never\n> say \"use of it is OK because it is in POSIX\", and we sometimes even\n> say \"even it is in POSIX, various implementation of it is buggy and\n> it does not work properly in practice\" to certain things [*1*].\n>\n> C89 has been the base of such a guideline for our C programs, and\n> people must not to view this patch as an attempt to raise the base\n> to C99. It is rather an attempt to see how far we can safely raise\n> the base by checking some selected useful new features [*2*] that we\n> have had occasions to wish that they were available, and designated\n> initializer for structure fields is one of them.\n>\n> We may find out that, after starting with \"C89, plus this and that\n> feature that are known to be commonly supported\", the guideline may\n> become more succinct to say \"C99, minus this and that feature that\n> are not usable on some platforms\", if it turns out that majority of\n> the systems that are not fully C99 have all of the things we care\n> about. We do not know yet, and we are only at the beginning of the\n> journey to find it out.\n\nI think in the context of this desire Johannes Sixt's \"Actually, I'm\nserious [about let's compile with c++]\"[1] should be given some more\nconsideration.\n\nI've just compiled Git with it and it passes all tests. I think the\nendeavor is worthwhile in itself as C++ source-level compatibility for\ngit.git is clearly easy to achieve, and would effectively give us access\nto more compilers (albeit in different modes, but they may discover\nnovel bugs that also apply to the C mode code).\n\nMost of his patch is just avoiding C++ keywords, e.g. new -> wen, try ->\ntry_, this -> this_, namespace -> name_space, template -> templ\netc. It's going to be relatively easy to avoid a few keywords as\nvariable names, especially if we set up CI for it via Travis.\n\nBut why do it? Aside from the \"more compilers\" argument, we may find\nthat it's going to be much easier to use some C99 features we want by\nhaving C++ source-level compatibility, and on compilers like that may\nnot support those features in C use the C++ mode that may support those.\n\nI don't know enough about e.g. MSVC to say if that's the case, but if so\nthat might be an easier way to use some C99 features.\n\nIf not C++ support would be interesting for other reasons. Johannes\nSixt: It would be very nice to get those patches on-list.\n\n1. 962da692-8874-191c-59d4-65b9562cf87f@kdbg.org\n (https://public-inbox.org/git/962da692-8874-191c-59d4-65b9562cf87f@kdbg.org/)\n\n> [Footnote]\n>\n> *1* Like `backtick` command substitutions that is very hard to make\n> them nest properly and impossible to mix portably with quoting.\n>\n> *2* New, relative to our current practice, that is.\n"},{"id":"324302","messageId":"xmqqd1951hye.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"87d195zcy0.fsf@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-12T21:08:41Z","receivedAt":"2017-07-12T21:08:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Tue, Jul 11 2017, Junio C. Hamano jotted:\n>\n>> Just so that people do not misunderstand, it is not our goal to\n>> declare that now you need a fully C99 compiler to build Git.\n>> ...\n>\n> I think in the context of this desire Johannes Sixt's \"Actually, I'm\n> serious [about let's compile with c++]\"[1] should be given some more\n> consideration.\n> ...\n> Most of his patch is just avoiding C++ keywords, e.g. new -> wen, try ->\n> try_, this -> this_, namespace -> name_space, template -> templ\n> etc. It's going to be relatively easy to avoid a few keywords as\n> variable names, especially if we set up CI for it via Travis.\n\nI am not interested at all in building the binary I personally use\nwith any C++ compiler, but I do not mind too much if people made it\neasier for other people to do so, but only if the did it the right\nway.\n\nI do like the fact that we call two things we are comparing with a\npair of matching words, 'old' and 'new'. When two variables or\nfields have certain relationship, they should be named with words\nthat have constrasting meaning that explains what they are.\n\nI would very much mind if a \"let's make it buildable with C++\"\neffort made the code compare 'old' and 'wen'. C++ is not that\ninteresting to sacrifice the readability of the code. Don't invent\nnon-words like wen; don't truncate a word like 'template' in the\nmiddle to 'templ' to make it unreadable and invite inconsistencies\n(e.g. \"was it templ, templa, or something else?\").\n\nIf a \"let's make it buildable with C++\" effort needs to avoid 'new',\nreplace *both* 'old' and 'new' to a matching pair of words (perhaps\n'pre' and 'post'? but it is making it worse by choosing a pair with\ndifferent length. 'one' vs 'two' would be OK if there is no strong\nconnotation that the 'old' side is always truly older in the\nfunction in question).\n\nI would not mind the result of such an update that much. We already\ndo use different pair of words in places that we could have used\n after all.\n\nHaving to review too many updates like that in a single sitting\nwould be annoying, though.\n\nThe same thing for where we use 'this'; if the existing code is\ncontrasting 'this' with 'that', and if your C++ effort wants to\nreplace 'this', you MUST replace 'that' as well so that we would\nstill be contrasting a pair of variables appropriately named.\n"},{"id":"324458","messageId":"2f7f7af2-585c-9374-34e3-e34680678935@kdbg.org","threadId":"46343","inReplyTo":"87d195zcy0.fsf@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-07-13T22:24:43Z","receivedAt":"2017-07-13T22:24:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2017 um 21:12 schrieb Ævar Arnfjörð Bjarmason:\n> I think in the context of this desire Johannes Sixt's \"Actually, I'm\n> serious [about let's compile with c++]\"[1] should be given some more\n> consideration.\n\nThank you for your support.\n\n> I've just compiled Git with it and it passes all tests. I think the\n> endeavor is worthwhile in itself as C++ source-level compatibility for\n> git.git is clearly easy to achieve, and would effectively give us access\n> to more compilers (albeit in different modes, but they may discover\n> novel bugs that also apply to the C mode code).\n\nPlease keep in mind that my patches only show that it can be done. \nNevertheless, the result is far, far away from valid C++ code. It can be \ncompiled by GCC (thanks to its -fpermissive flag) and happens to produce \nsomething that works. But that does not mean that other compilers would \ngrok it.\n\nSource-level compatibility is only necessary as a stop gap in the \ntransition to C++. If the transition is not going to happen, I doubt \nthat there is any value. It is simply too much code churn for too little \ngain. The real gains are in the features of C++(11,14).\n\n> But why do it? Aside from the \"more compilers\" argument, we may find\n> that it's going to be much easier to use some C99 features we want by\n> having C++ source-level compatibility, and on compilers like that may\n> not support those features in C use the C++ mode that may support those.\n\nI would be happy to learn about a C99 feature where C++ mode of some \ncompiler would help.\n\nThe only C99 feature mentioned so far was designated initializers. \nUnfortunately, that actually widens the gap between C and C++, not \nlessens it. (C++ does not have designated initializers, and they are not \non the agenda.)\n\n> If not C++ support would be interesting for other reasons. Johannes\n> Sixt: It would be very nice to get those patches on-list.\n\nI don't think it's worth to swamp the list with the patches at this \ntime. Interested parties can find them here:\n\nhttps://github.com/j6t/git.git c-plus-plus\n\nI may continue the work, slowly and as long as I find it funny. It's \njust mental exercise, unless the Git community copies the GCC Steering \nCommittee's mindeset with regard to C++ in the code base \n(https://gcc.gnu.org/ml/gcc/2010-05/msg00705.html).\n\n-- Hannes\n"},{"id":"324534","messageId":"xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"xmqqy3rw3rc8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T16:11:33Z","receivedAt":"2017-07-14T16:11:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Oh, absolutely.\n>\n> Here is another possible test balloon, that may actually be useful\n> as an example. I think there is a topic in flight that touches this\n> array, unfortunately, so I probably would find another one that is\n> more stable for a real follow-up patch to the one from Peff.\n\nAnd here it is.\n\nAs to other things that we currently not allow in our codebase that\nnewer compilers can grok, here is what *I* think. It is *not* meant\nto be an exhaustive \"what's new in C99 that is not in C89? what is\nthe final verdict on each of them?\":\n\n - There were occasional cases where we wished if variable-length\n arrays, flexible array members and variadic macros were available\n in our codebase during the course of this project. We would\n probably want to add a similar test baloon patch for each of\n them to this series that is currently two-patch long.\n\n - I prefer to keep decl-after-statement out of our codebase. I\n view it as a big plus in code-readability to be able to see a\n complete list of variables that will be used in a block upfront\n before starting to read the code that uses them.\n\n - Corollary to the above, I do not mind to have a variable\n declaration in the initialization clause of a for() statement\n (e.g. \"for (int i = 0; i < 4; i++) { ... }\"), as the scoping rule\n is very sensible. Some of our \"for()\" statements use the value\n of the variable after iteration, for which this new construct\n cannot be used, though.\n\n - This may be showing I am not just old fashioned but also am\n ignorant, but I do not see much point in using the following in\n our codebase (iow, I am not aware of places in the existing code\n that they can be improved by employing these features):\n\n . // comments\n . restricted pointers\n . static and type qualifiers in parameter array declarators\n\n\n\n-- >8 --\nSubject: [PATCH] clean.c: use designated initializer\n\nThis is another test balloon to see if we get complaints from people\nwhose compilers do not support designated initializer for arrays.\n\nThe use of the feature is not all that interesting for cases like\nthe one this patch touches, where the initialized elements of the\narray is dense, but it would be nice if we can use the feature to\ninitialize an array that has elements initialized to interesting\nvalues only sparsely.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 19 ++++++++++---------\n 1 file changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 057fc97fe4..858df2c4ee 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -33,15 +33,6 @@ static const char *msg_skip_git_dir = N_(\"Skipping repository %s\\n\");\n static const char *msg_would_skip_git_dir = N_(\"Would skip repository %s\\n\");\n static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n-static int clean_use_color = -1;\n-static char clean_colors[][COLOR_MAXLEN] = {\n-\tGIT_COLOR_RESET,\n-\tGIT_COLOR_NORMAL,\t/* PLAIN */\n-\tGIT_COLOR_BOLD_BLUE,\t/* PROMPT */\n-\tGIT_COLOR_BOLD,\t\t/* HEADER */\n-\tGIT_COLOR_BOLD_RED,\t/* HELP */\n-\tGIT_COLOR_BOLD_RED,\t/* ERROR */\n-};\n enum color_clean {\n \tCLEAN_COLOR_RESET = 0,\n \tCLEAN_COLOR_PLAIN = 1,\n@@ -51,6 +42,16 @@ enum color_clean {\n \tCLEAN_COLOR_ERROR = 5\n };\n \n+static int clean_use_color = -1;\n+static char clean_colors[][COLOR_MAXLEN] = {\n+\t[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,\n+\t[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,\n+\t[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,\n+\t[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,\n+\t[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,\n+\t[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,\n+};\n+\n #define MENU_OPTS_SINGLETON\t\t01\n #define MENU_OPTS_IMMEDIATE\t\t02\n #define MENU_OPTS_LIST_ONLY\t\t04\n-- \n2.14.0-rc0-148-g5448d1895b\n\n"},{"id":"324537","messageId":"CAGZ79kaFH5BXEsTEz55NbfhrJoWfutTVQi-bv3NpomCoK9OzWw@mail.gmail.com","threadId":"46343","inReplyTo":"xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-14T17:13:50Z","receivedAt":"2017-07-14T17:13:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jul 14, 2017 at 9:11 AM, Junio C Hamano wrote:\n\n> - I prefer to keep decl-after-statement out of our codebase. I\n> view it as a big plus in code-readability to be able to see a\n> complete list of variables that will be used in a block upfront\n> before starting to read the code that uses them.\n\nsounds good to me.\n\n> - Corollary to the above, I do not mind to have a variable\n> declaration in the initialization clause of a for() statement\n> (e.g. \"for (int i = 0; i < 4; i++) { ... }\"), as the scoping rule\n> is very sensible. Some of our \"for()\" statements use the value\n> of the variable after iteration, for which this new construct\n> cannot be used, though.\n\nI might send a test balloon for this.\n\n> . // comments\n\nI would think these are useful for two reasons:\n(a) these seem to be used widely outside of these old-style project\n (git, gcc, kernel), such that most new contributors need to be told\n to avoid these which adds to the entry burden.\n(b) recursive nesting of comments is possible. We may not need this\n in the final patch, but I sure use that in development. To comment out\n a whole function containing multiple comments I have to select all\n lines and prefix them with // currently. If all comments were //, I could\n put /* .. */ around the function, which seems easier. I just realize\n I can use #if 0 .. #endif though.\n\n (a) may be a concern, (b) not so much from the projects point of view.\n\n> . restricted pointers\n\nThat sounds like the ultimate micro optimization, but may be hard\nto reason about after a years of churn, so it may become hard to\nmaintain.\n\n> . static and type qualifiers in parameter array declarators\n\nThat sounds equally arcane.\n\n> -- >8 --\n> Subject: [PATCH] clean.c: use designated initializer\n>\n> This is another test balloon to see if we get complaints from people\n> whose compilers do not support designated initializer for arrays.\n\nThis sounds as if it is applied on top of Jeffs test balloon patch, such\nthat we do not need to re-iterate the criteria why to do it here, e.g.\nthis code is always compiled and did not change a lot over the last\ncouple month, so potentially easy to revert.\n\nThanks,\nStefan\n"},{"id":"324540","messageId":"20170714173658.2q24oxhatwh5qrqk@sigill.intra.peff.net","threadId":"46343","inReplyTo":"xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-14T17:36:58Z","receivedAt":"2017-07-14T17:37:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 14, 2017 at 09:11:33AM -0700, Junio C Hamano wrote:\n\n> As to other things that we currently not allow in our codebase that\n> newer compilers can grok, here is what *I* think. It is *not* meant\n> to be an exhaustive \"what's new in C99 that is not in C89? what is\n> the final verdict on each of them?\":\n> \n> - There were occasional cases where we wished if variable-length\n> arrays, flexible array members and variadic macros were available\n> in our codebase during the course of this project. We would\n> probably want to add a similar test baloon patch for each of\n> them to this series that is currently two-patch long.\n\nI think variable-length arrays are potentially dangerous. They're\nallocated on the stack, which creates two issues:\n\n 1. You can run out of stack space and segfault, whereas the same\n operation with a heap buffer would be fine. You can say \"but this\n VLA will only be used for small things\". But then, you can just as\n easily declare a small stack buffer.\n\n 2. My understanding of the recent \"Stack Clash\" class of\n vulnerabilities[1] is that VLAs make the attacker's job much easier\n (since they can often just send a large input to get you to\n allocate a large stack).\n\nI think variadic macros are a good candidate, though. There have been a\nnumber of times where we've had to sacrifice functionality or\nreadability in our helper functions. E.g., the case mentioned in\n368953912 (add helpers for allocating flex-array structs, 2016-02-22).\n\nThe weather-balloon patch for that should be easy, too: just drop the\nfallback macros from BUG() or the trace code.\n\n[1] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt\n\n> - I prefer to keep decl-after-statement out of our codebase. I\n> view it as a big plus in code-readability to be able to see a\n> complete list of variables that will be used in a block upfront\n> before starting to read the code that uses them.\n> \n> - Corollary to the above, I do not mind to have a variable\n> declaration in the initialization clause of a for() statement\n> (e.g. \"for (int i = 0; i < 4; i++) { ... }\"), as the scoping rule\n> is very sensible. Some of our \"for()\" statements use the value\n> of the variable after iteration, for which this new construct\n> cannot be used, though.\n\nI agree with both of those points. I think the decl-in-for is nice\nexactly because it highlights those cases where the iteration variable's\nvalue is relevant after the loop ends.\n\n> - This may be showing I am not just old fashioned but also am\n> ignorant, but I do not see much point in using the following in\n> our codebase (iow, I am not aware of places in the existing code\n> that they can be improved by employing these features):\n> \n> . // comments\n> . restricted pointers\n> . static and type qualifiers in parameter array declarators\n\nAgreed, though I think the comment thing is a personal taste issue (just\nnot my taste).\n\n> +static int clean_use_color = -1;\n> +static char clean_colors[][COLOR_MAXLEN] = {\n> +\t[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,\n> +\t[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,\n> +\t[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,\n> +\t[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,\n> +\t[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,\n> +\t[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,\n> +};\n\nI think this is much nicer to read. I assume if we have a \"hole\" in our\nnumbering that the hole is initialized in the usual static way (a\nCOLOR_MAXLEN array full of NULs in this case, I guess)?\n\n-Peff\n"},{"id":"324545","messageId":"xmqqd192rh1t.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"20170714173658.2q24oxhatwh5qrqk@sigill.intra.peff.net","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T18:48:14Z","receivedAt":"2017-07-14T18:48:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +static int clean_use_color = -1;\n>> +static char clean_colors[][COLOR_MAXLEN] = {\n>> +\t[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,\n>> +\t[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,\n>> +\t[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,\n>> +\t[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,\n>> +\t[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,\n>> +\t[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,\n>> +};\n>\n> I think this is much nicer to read. I assume if we have a \"hole\" in our\n> numbering that the hole is initialized in the usual static way (a\n> COLOR_MAXLEN array full of NULs in this case, I guess)?\n\nI would expect that would be the case. \n\nDo we need to have a check to detect a buggy compiler that takes the\nsyntax but produces an incorrectly initialized array? I could add a\ntest to ensure that HEADER comes out BOLD, etc. (or we may already\nhave such a test) and then reorder these lines in this patch, if\nthat is the kind of breakage we anticipate.\n"},{"id":"324546","messageId":"xmqq8tjqrfq4.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"xmqqd192rh1t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T19:16:51Z","receivedAt":"2017-07-14T19:16:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Do we need to have a check to detect a buggy compiler that takes the\n> syntax but produces an incorrectly initialized array? I could add a\n> test to ensure that HEADER comes out BOLD, etc. (or we may already\n> have such a test) and then reorder these lines in this patch, if\n> that is the kind of breakage we anticipate.\n\nSo here is a lunch-time hack I did to replace the patch I sent\nearlier. I kind of like the ordering of the elements better than\nthe original, in that it somehow feels more logical, even though I\nmerely sorted alphabetically ;-).\n\n\n builtin/clean.c | 19 ++++++++++---------\n t/t7301-clean-interactive.sh | 10 ++++++++++\n 2 files changed, 20 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 057fc97fe4..e2bb3c69ed 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -33,15 +33,6 @@ static const char *msg_skip_git_dir = N_(\"Skipping repository %s\\n\");\n static const char *msg_would_skip_git_dir = N_(\"Would skip repository %s\\n\");\n static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n-static int clean_use_color = -1;\n-static char clean_colors[][COLOR_MAXLEN] = {\n-\tGIT_COLOR_RESET,\n-\tGIT_COLOR_NORMAL,\t/* PLAIN */\n-\tGIT_COLOR_BOLD_BLUE,\t/* PROMPT */\n-\tGIT_COLOR_BOLD,\t\t/* HEADER */\n-\tGIT_COLOR_BOLD_RED,\t/* HELP */\n-\tGIT_COLOR_BOLD_RED,\t/* ERROR */\n-};\n enum color_clean {\n \tCLEAN_COLOR_RESET = 0,\n \tCLEAN_COLOR_PLAIN = 1,\n@@ -51,6 +42,16 @@ enum color_clean {\n \tCLEAN_COLOR_ERROR = 5\n };\n \n+static int clean_use_color = -1;\n+static char clean_colors[][COLOR_MAXLEN] = {\n+\t[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,\n+\t[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,\n+\t[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,\n+\t[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,\n+\t[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,\n+\t[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,\n+};\n+\n #define MENU_OPTS_SINGLETON\t\t01\n #define MENU_OPTS_IMMEDIATE\t\t02\n #define MENU_OPTS_LIST_ONLY\t\t04\ndiff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh\nindex 3ae394e934..556e1850e2 100755\n--- a/t/t7301-clean-interactive.sh\n+++ b/t/t7301-clean-interactive.sh\n@@ -472,4 +472,14 @@ test_expect_success 'git clean -id with prefix and path (ask)' '\n \n '\n \n+test_expect_success 'git clean -i paints the header in HEADER color' '\n+\t>a.out &&\n+\techo q |\n+\tgit -c color.ui=always clean -i |\n+\ttest_decode_color |\n+\thead -n 1 >header &&\n+\t# not i18ngrep\n+\tgrep \"^\" header\n+'\n+\n test_done\n"},{"id":"324547","messageId":"87pod23jix.fsf@gmail.com","threadId":"46343","inReplyTo":"xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-14T19:28:38Z","receivedAt":"2017-07-14T19:28:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 14 2017, Junio C. Hamano jotted:\n\n> Junio C Hamano writes:\n>\n> - This may be showing I am not just old fashioned but also am\n> ignorant, but I do not see much point in using the following in\n> our codebase (iow, I am not aware of places in the existing code\n> that they can be improved by employing these features):\n>\n> . // comments\n\n[Feel free to ignore this E-Mail and my fascination with C syntax\ntrivia]\n\nI wouldn't advocate switching to them on this basis, but since you asked\nfor cases where things could be improved with // comments:\n\nThe other day I submitted a patch that included this line in a comment:\n\n + * \"t/**.sh\" and then conclude that there's a directory \"t\",\n\nWhich you fixed up to, before integrating it:\n\n + * \"t/\" + \"**.sh\" and then conclude that there's a directory \"t\",\n\nThat was only necessary due to limitations in one of two comment\nsyntaxes modern C supports.\n\nWell, it wasn't *necessary*, but a compiler warned about the /* there as\na possibly confusing construct, and any compiler would have ended the\ncomment right there + errored out if it contained \"t/**/*.sh\".\n"},{"id":"324557","messageId":"xmqq4luer6yl.fsf@gitster.mtv.corp.google.com","threadId":"46343","inReplyTo":"87pod23jix.fsf@gmail.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-14T22:26:10Z","receivedAt":"2017-07-14T22:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Fri, Jul 14 2017, Junio C. Hamano jotted:\n>\n>> Junio C Hamano writes:\n>>\n>> - This may be showing I am not just old fashioned but also am\n>> ignorant, but I do not see much point in using the following in\n>> our codebase (iow, I am not aware of places in the existing code\n>> that they can be improved by employing these features):\n>>\n>> . // comments\n>\n> [Feel free to ignore this E-Mail and my fascination with C syntax\n> trivia]\n>\n> I wouldn't advocate switching to them on this basis, but since you asked\n> for cases where things could be improved with // comments:\n>\n> The other day I submitted a patch that included this line in a comment:\n>\n> + * \"t/**.sh\" and then conclude that there's a directory \"t\",\n\nYes, obviously I am very aware of this one. \n\nThe thing is, I had to do this only once in the 10+ year since\nproject inception. I find that much more relevant anecdata ;-).\n\n"},{"id":"324562","messageId":"20170714224309.7p4a4vuuv3k2cdjg@glandium.org","threadId":"46343","inReplyTo":"xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2017-07-14T22:43:09Z","receivedAt":"2017-07-14T22:43:20Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Fri, Jul 14, 2017 at 09:11:33AM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > Oh, absolutely.\n> >\n> > Here is another possible test balloon, that may actually be useful\n> > as an example. I think there is a topic in flight that touches this\n> > array, unfortunately, so I probably would find another one that is\n> > more stable for a real follow-up patch to the one from Peff.\n> \n> And here it is.\n> \n> As to other things that we currently not allow in our codebase that\n> newer compilers can grok, here is what *I* think. It is *not* meant\n> to be an exhaustive \"what's new in C99 that is not in C89? what is\n> the final verdict on each of them?\":\n> \n> - There were occasional cases where we wished if variable-length\n> arrays, flexible array members and variadic macros were available\n> in our codebase during the course of this project. We would\n> probably want to add a similar test baloon patch for each of\n> them to this series that is currently two-patch long.\n\nFWIW, variadic macros have subtle implementation differences that can\ncause problems.\nFor instance, MSVC only supports ##__VA_ARGS__ by way of\nignoring ## somehow, but has the default behavior of dropping the comma\nwhen __VA_ARGS__ is empty, which means , __VA_ARGS__ *without* ## has a\ndifferent behavior.\nSee also https://connect.microsoft.com/VisualStudio/feedback/details/380090/variadic-macro-replacement\n\nMike\n"},{"id":"324571","messageId":"20170715110839.hefdnf4eqhdwboxq@sigill.intra.peff.net","threadId":"46343","inReplyTo":"20170714224309.7p4a4vuuv3k2cdjg@glandium.org","subject":"Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-15T11:08:39Z","receivedAt":"2017-07-15T11:08:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 15, 2017 at 07:43:09AM +0900, Mike Hommey wrote:\n\n> > - There were occasional cases where we wished if variable-length\n> > arrays, flexible array members and variadic macros were available\n> > in our codebase during the course of this project. We would\n> > probably want to add a similar test baloon patch for each of\n> > them to this series that is currently two-patch long.\n> \n> FWIW, variadic macros have subtle implementation differences that can\n> cause problems.\n> For instance, MSVC only supports ##__VA_ARGS__ by way of\n> ignoring ## somehow, but has the default behavior of dropping the comma\n> when __VA_ARGS__ is empty, which means , __VA_ARGS__ *without* ## has a\n> different behavior.\n> See also https://connect.microsoft.com/VisualStudio/feedback/details/380090/variadic-macro-replacement\n\nWhen there's any other non-optional argument, you can work around this\nby just lumping that argument in with the varargs. So don't do:\n\n #define FOO(fmt, ...) printf(fmt, __VA_ARGS__)\n\nwhich fails when the format has no placeholders. Instead do:\n\n #define FOO(...) printf(__VA_ARGS__)\n\nThat limits your interface for variadic macros, but in practice it\ndoesn't usually come up (there are also disgusting macro tricks you can\ndo to cover the true 0-arg case portably, but we haven't had to turn to\nthem yet).\n\n-Peff\n"},{"id":"324772","messageId":"20170719181956.15845-1-sbeller@google.com","threadId":"46343","inReplyTo":"xmqq8tjqrfq4.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] objects: scope count variable to loop","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-19T18:19:56Z","receivedAt":"2017-07-19T18:20:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is another test balloon to see if we get complaints from people\nwhose compilers do not support variables scoped to for loops.\n\nThis part of the code base was chosen as it is very old code that does\nnot change often, such that a potential revert is easy.\n\nSigned-off-by: Stefan Beller \n---\n\nThis is a rather aggressive test ballon, my compiler needed some\ngood arguments to accept the new world order:\n\nobject.c: In function ‘object_array_remove_duplicates’:\nobject.c:404:2: error: ‘for’ loop initial declarations are only allowed in C99 mode\n for (unsigned src = 0; src < nr; src++) {\n ^\nobject.c:404:2: note: use option -std=c99 or -std=gnu99 to compile your code\n\nUsing -std=c99 works for me.\n\nThanks,\nStefan\n\n object.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object.c b/object.c\nindex f818777412..af26ee2fbc 100644\n--- a/object.c\n+++ b/object.c\n@@ -397,11 +397,11 @@ static int contains_name(struct object_array *array, const char *name)\n \n void object_array_remove_duplicates(struct object_array *array)\n {\n-\tunsigned nr = array->nr, src;\n+\tunsigned nr = array->nr;\n \tstruct object_array_entry *objects = array->objects;\n \n \tarray->nr = 0;\n-\tfor (src = 0; src < nr; src++) {\n+\tfor (unsigned src = 0; src < nr; src++) {\n \t\tif (!contains_name(array, objects[src].name)) {\n \t\t\tif (src != array->nr)\n \t\t\t\tobjects[array->nr] = objects[src];\n-- \n2.14.0.rc0.3.g6c2e499285\n\n"},{"id":"324773","messageId":"20170719182342.GA158344@google.com","threadId":"46343","inReplyTo":"20170719181956.15845-1-sbeller@google.com","subject":"Re: [PATCH] objects: scope count variable to loop","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-07-19T18:23:42Z","receivedAt":"2017-07-19T18:23:49Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 07/19, Stefan Beller wrote:\n> This is another test balloon to see if we get complaints from people\n> whose compilers do not support variables scoped to for loops.\n> \n> This part of the code base was chosen as it is very old code that does\n> not change often, such that a potential revert is easy.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> \n> This is a rather aggressive test ballon, my compiler needed some\n> good arguments to accept the new world order:\n> \n> object.c: In function ‘object_array_remove_duplicates’:\n> object.c:404:2: error: ‘for’ loop initial declarations are only allowed in C99 mode\n> for (unsigned src = 0; src < nr; src++) {\n> ^\n> object.c:404:2: note: use option -std=c99 or -std=gnu99 to compile your code\n> \n> Using -std=c99 works for me.\n\nThis would need a change to the makefile then wouldn't it?\n\n> \n> Thanks,\n> Stefan\n> \n> object.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/object.c b/object.c\n> index f818777412..af26ee2fbc 100644\n> --- a/object.c\n> +++ b/object.c\n> @@ -397,11 +397,11 @@ static int contains_name(struct object_array *array, const char *name)\n> \n> void object_array_remove_duplicates(struct object_array *array)\n> {\n> -\tunsigned nr = array->nr, src;\n> +\tunsigned nr = array->nr;\n> \tstruct object_array_entry *objects = array->objects;\n> \n> \tarray->nr = 0;\n> -\tfor (src = 0; src < nr; src++) {\n> +\tfor (unsigned src = 0; src < nr; src++) {\n> \t\tif (!contains_name(array, objects[src].name)) {\n> \t\t\tif (src != array->nr)\n> \t\t\t\tobjects[array->nr] = objects[src];\n> -- \n> 2.14.0.rc0.3.g6c2e499285\n> \n\n-- \nBrandon Williams\n"},{"id":"324955","messageId":"20170724170813.scceigybl5d3fvdd@sigill.intra.peff.net","threadId":"46343","inReplyTo":"20170719182342.GA158344@google.com","subject":"Re: [PATCH] objects: scope count variable to loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-24T17:08:13Z","receivedAt":"2017-07-24T17:08:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 19, 2017 at 11:23:42AM -0700, Brandon Williams wrote:\n\n> > object.c: In function ‘object_array_remove_duplicates’:\n> > object.c:404:2: error: ‘for’ loop initial declarations are only allowed in C99 mode\n> > for (unsigned src = 0; src < nr; src++) {\n> > ^\n> > object.c:404:2: note: use option -std=c99 or -std=gnu99 to compile your code\n> > \n> > Using -std=c99 works for me.\n> \n> This would need a change to the makefile then wouldn't it?\n\nActually, it complicates things even more, I'd think. We probably can't\njust blindly add \"-std=c99\" to CFLAGS, as not all compilers would\nsupport it (even if they _do_ support this construct).\n\nInterestingly I have no problems compiling it here. I wonder if Stefan's\nconfig.mak is supplying -std=c89 or some other restrictive flag. Or if\nhis compiler is a different version (though I tried with gcc-6, gcc-4.9,\nand clang-3.8).\n\n-Peff\n"},{"id":"324959","messageId":"CAGZ79kZafHBJOVwZzw_ii3T6_X24Uci4vUudp45r8-GTn0a4Sg@mail.gmail.com","threadId":"46343","inReplyTo":"20170724170813.scceigybl5d3fvdd@sigill.intra.peff.net","subject":"Re: [PATCH] objects: scope count variable to loop","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-24T17:12:59Z","receivedAt":"2017-07-24T17:13:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 24, 2017 at 10:08 AM, Jeff King wrote:\n> On Wed, Jul 19, 2017 at 11:23:42AM -0700, Brandon Williams wrote:\n>\n>> > object.c: In function ‘object_array_remove_duplicates’:\n>> > object.c:404:2: error: ‘for’ loop initial declarations are only allowed in C99 mode\n>> > for (unsigned src = 0; src < nr; src++) {\n>> > ^\n>> > object.c:404:2: note: use option -std=c99 or -std=gnu99 to compile your code\n>> >\n>> > Using -std=c99 works for me.\n>>\n>> This would need a change to the makefile then wouldn't it?\n>\n> Actually, it complicates things even more, I'd think. We probably can't\n> just blindly add \"-std=c99\" to CFLAGS, as not all compilers would\n> support it (even if they _do_ support this construct).\n>\n> Interestingly I have no problems compiling it here. I wonder if Stefan's\n> config.mak is supplying -std=c89 or some other restrictive flag. Or if\n> his compiler is a different version (though I tried with gcc-6, gcc-4.9,\n> and clang-3.8).\n\nBefore this patch, I only had\n CFLAGS += -g -O0\nin config.mak (as I switched working directories recently), I'll throw in\n DEVELOPER=1\n\nMy compiler version is ancient (gcc 4.8.4-2ubuntu1~14.04.3)\napparently (why did I never check in this environment?)\n"},{"id":"324964","messageId":"20170724180530.smcjifvwytc5wcwa@sigill.intra.peff.net","threadId":"46343","inReplyTo":"CAGZ79kZafHBJOVwZzw_ii3T6_X24Uci4vUudp45r8-GTn0a4Sg@mail.gmail.com","subject":"Re: [PATCH] objects: scope count variable to loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-07-24T18:05:30Z","receivedAt":"2017-07-24T18:06:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 24, 2017 at 10:12:59AM -0700, Stefan Beller wrote:\n\n> > Interestingly I have no problems compiling it here. I wonder if Stefan's\n> > config.mak is supplying -std=c89 or some other restrictive flag. Or if\n> > his compiler is a different version (though I tried with gcc-6, gcc-4.9,\n> > and clang-3.8).\n> \n> Before this patch, I only had\n> CFLAGS += -g -O0\n> in config.mak (as I switched working directories recently), I'll throw in\n> DEVELOPER=1\n> \n> My compiler version is ancient (gcc 4.8.4-2ubuntu1~14.04.3)\n> apparently (why did I never check in this environment?)\n\nAh, indeed, it's the compiler version. And I actually screwed up my\ngcc-4.9 test. It complains, too. It looks like the default for gcc\nbumped from gnu90 to gnu11 in gcc 5.\n\n-Peff\n"}]}