From: Jeff Hostetler <git@jeffhostetler.com>
Date: Thu, 31 Jan 2019 19:45:38 GMT
Subject: Re: [PATCH v3 00/14] Trace2 tracing facility
Message-ID: <0764948c-f3ac-3bd1-fd82-2bd3f59b463d@jeffhostetler.com>
In-Reply-To: <xmqqbm3w3bjz.fsf@gitster-ct.c.googlers.com>



On 1/31/2019 1:51 PM, Junio C Hamano wrote:
> Jeff Hostetler <git@jeffhostetler.com> 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