[PATCH v2 1/3] debug.h: cleanup TRACE macros

[Konstantin Kharlamov]

On 28.01.2019 19:04, Alexandre Julliard wrote:
> Konstantin Kharlamov <hi-angel at yandex.ru> writes:
> 
>> On 28.01.2019 12:24, Alexandre Julliard wrote:
>>> Konstantin Kharlamov <Hi-Angel at yandex.ru> writes:
>>>
>>>> The complex "#ifdef WINE_NO_TRACE_MSGS && __compiler__" ladder below
>>>> reduces to "ignore WINE_NO_TRACE_MSGS when !__GNUC__ && !__SUNPRO_C",
>>>> which is probably a bug.
>>>
>>> No, it's on purpose, because some other compiler may not support varargs
>>> macros.
>>
>> But then the workaround would result in incorrectly configured
>> build. Is the complexity of the original code really worth the not
>> even correct support of an obscure usecase?
> 
> WINE_NO_TRACE_MSGS is simply an optimization, there's not much harm if
> it doesn't work on some obscure compiler. It's better than breaking the
> build.
> 
>> While on it, could you please advice on the naming in the 3-rd patch?
>> Initially I've used "unlikely" because that's how it's used in
>> Mesa. Later then I saw in wine lots of lower-case macros with __
>> prefix, so I changed it to "__unlikely". But then there are generic
>> macros like "TRACE", and I think of renaming it to "UNLIKELY", which
>> is 2 letters shorter, and is upper-case like yet another part of
>> macros.
> 
> Considering that the benchmarks don't show an improvement, I don't think
> it's worth the trouble.

Wait, how so if it does? Besides, even without the benchmark it's 
obvious that the TRACE appear in so many places that there inevitably 
would be branch-misses.

In addition, this macro can be used for further optimization of 
performance critical parts of wine code.

And what you mean by "the trouble" — it's just a few lines of code that 
doesn't add any maintainance burden.

> Also, adding macros that don't exist on Windows
> to standard headers is strongly discouraged.

Ah, I see, you mean that `windef.h` is a standard header from WinAPI. 
Okay, where then should I put it? windef.h just turned out to be one of 
2 headers included by debug.h (the other being stdarg.h). I could put it 
directly into debug.h though.