Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

From: Stefan Beller <sbeller@google.com>
Date: Jul 14, 2017, 17:13 UTC
Message-ID: <CAGZ79kaFH5BXEsTEz55NbfhrJoWfutTVQi-bv3NpomCoK9OzWw@mail.gmail.com>
In-Reply-To: <xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 14, 2017 at 9:11 AM, Junio C Hamano <gitster@pobox.com> wrote:

>  - I prefer to keep decl-after-statement out of our codebase.  I
>    view it as a big plus in code-readability to be able to see a
>    complete list of variables that will be used in a block upfront
>    before starting to read the code that uses them.

sounds good to me.

Show 6 quoted lines

>  - Corollary to the above, I do not mind to have a variable
>    declaration in the initialization clause of a for() statement
>    (e.g. "for (int i = 0; i < 4; i++) { ... }"), as the scoping rule
>    is very sensible.  Some of our "for()" statements use the value
>    of the variable after iteration, for which this new construct
>    cannot be used, though.

I might send a test balloon for this.

>    . // comments

I would think these are useful for two reasons:
(a) these seem to be used widely outside of these old-style project
  (git, gcc, kernel), such that most new contributors need to be told
  to avoid these which adds to the entry burden.
(b) recursive nesting of comments is possible. We may not need this
  in the final patch, but I sure use that in development. To comment out
  a whole function containing multiple comments I have to select all
  lines and prefix them with // currently. If all comments were //, I could
  put /* .. */ around the function, which seems easier. I just realize
  I can use #if 0 .. #endif though.

  (a) may be a concern, (b) not so much from the projects point of view.

>    . restricted pointers

That sounds like the ultimate micro optimization, but may be hard to reason about after a years of churn, so it may become hard to maintain.

>    . static and type qualifiers in parameter array declarators

That sounds equally arcane.

Show 5 quoted lines

> -- >8 --
> Subject: [PATCH] clean.c: use designated initializer
>
> This is another test balloon to see if we get complaints from people
> whose compilers do not support designated initializer for arrays.

This sounds as if it is applied on top of Jeffs test balloon patch, such that we do not need to re-iterate the criteria why to do it here, e.g. this code is always compiled and did not change a lot over the last couple month, so potentially easy to revert.

Thanks, Stefan