Re: [PATCH v3 00/14] Trace2 tracing facility
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- Jan 31, 2019, 19:45 UTC
- 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:
Show 20 quoted lines
> 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.
Show 24 quoted lines
>
>> 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