Re: [MERGE] debugs message improvement: use HERE macro wherever sensible

>> It's meant to be a "just before the next branching point". We have
>> ~1500 calls to debugs() calls with HERE, ~3100 without. Maksing the
>> HERE symbol will make the effect go away, but it won't clean the mess.
>> Such an invasive patch was rejected last week, so I added the TODO to
>> remind to clean up the mess when the right moment in the release cycle
>> will come.
>
> IMHO, there is no right moment for such an invasive patch (the reasons I
> discussed earlier will not change as the release cycle progresses).
> Others may overrule me on that, of course.

I guess that Amos is the one who's most affected, so I would consider
this call as his.

>>>> +#define HERE ""
>>>
>>> Have you checked that the compiler is smart enough to optimize this out?
>>> I suspect it is not. Please find a way to #define HERE so that the
>>> compiler can remove it. I recommend trying an inlined stream manipulator
>>> (that does not do anything).
>>
>> Ok, patch attached.
>
>> === modified file 'src/Debug.h'
>> --- src/Debug.h 2012-11-11 11:32:12 +0000
>> +++ src/Debug.h 2012-11-11 13:25:43 +0000
>> @@ -75,7 +75,6 @@
>>
>> class Debug
>> {
>> -
>> public:
>> static char *debugOptions;
>> static char *cache_log;
>
> Please remove the non-change in the first hunk.

Done.

>> + * HERE is a stream manipulatro which does nothing.
>> + * Its purpose is to inactivate calls made following previous debugs()
>> + * guidelines such as
>> * debugs(1,2, HERE << "some message");
>> + *
>> + * The purpose previously held by HERE is now absorbed in the debugs call itself
>> */
>
> Or you could just say "Legacy code. Do not use in new or updated code."
> or something like that :-).
>
> If you leave the detailed comment, please spellcheck "manipulatro".

How about:
 * HERE is a stream manipulator which does nothing.
 * Do not add it to new code, and remove it from old code as it is changed.
 *
 * Its purpose is to inactivate calls made following previous debugs()
 * guidelines such as
 * debugs(1,2, HERE << "some message");
 *
 * His former objective is now absorbed in the debugs call itself

>
>
>> +static inline std::ostream&
>> +HERE(std::ostream& s)
>> +{
>> + return s;
>> +}
>
> If "static" is not needed, please remove that keyword.

Ok.

> Looks OK to me otherwise. Feel free to commit if there are no other
> opinions. Can you tell whether GCC is optimizing HERE away after these
> changes?

It seems to be optimized away, I couldn't trigger a breakpoint in gdb.

Patch attached, will merge in 12 hours.

--
    /kinkie