On 1/31/2019 1:51 PM, Junio C Hamano wrote:
Jeff Hostetler <[email protected]> writes:

       -+       for_each_builtin(j, tgt_j) {
       ++       for_each_builtin(j, tgt_j)
       ++       {
        +               tgt_j->pfn_term();
        +       }

Our CodingGuidelines prefer the opening brace on the same line after
the if/for/while/struct/etc. statement, and even omitting the braces
if the if arm or loop body consists of a single statement.  So
unfortunately a considerable part of this range diff goes in the wrong
direction.

I know they do and I had them on the same line originally.

Clang-format was complaining about every use of the for_each_builtin
macro, so I changed them to be on the next line to quiet it.

Well, clang-format is wrong then ;-)

Ok.  I had never even heard of clang-format until Josh suggested
that it flagged several commits in my series.  The last thing I wanted
to do was to start hacking up its config file (in the dark) assuming
that is even an option.

I can undo my formatting changes if we want to update the settings.
I'll give that a try if there are no objections.


I hesitate to remove braces around a statement adjacent to a
for_each macro trick for the usual safety reasons.

Sorry, but what's "usual safety reasons"?  Isn't a macro that
requires {} in order to work correctly simply broken?

I see (from a previous iteration---sorry, but I haven't caught up)

#define for_each_builtin(j, tgt_j)                      \
         for (j = 0, tgt_j = tr2_tgt_builtins[j];        \
              tgt_j;                                     \
              j++, tgt_j = tr2_tgt_builtins[j])
and I do not think

        for (j = 0, tgt_j = ...; tgt_j; j++, tgt_j = ...)
                statement;

is unsafe (iow, your macro is not broken).

Puzzled.


Right, I don't think my macro is broken either.  It is more
my personal paranoia about preventing accidents (like wrapping
macro parameters in parens when using them in the macro body).

I can remove them if we want.

Jeff

Reply via email to