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

On 06.11.2012 04:47, Kinkie wrote:
> On Mon, Nov 5, 2012 at 4:19 PM, Alex Rousskov
> <rousskov_at_measurement-factory.com> wrote:
>> On 11/05/2012 02:36 AM, Kinkie wrote:
>>> Hi all,
>>> at lp:~kinkie/squid/sourceformat I have a work branch aiming at
>>> using HERE in debugs() messages wherever sensible.
>>> Apart from the main goal, there's a few typo fixes and a few
>>> wording
>>> improvements. No functional changes.
>>> The patch is ~11k lines so far, with ~1400 changes, I'm about
>>> halfway through.
>>> Any preference on how to review? Or may I just commit once it
>>> passes a
>>> full testsuite build?
>>
>> These kind of widespread changes will cause a lot of conflicts with
>> pending patches but will not significantly improve code quality or
>> functionality in most cases. Is it wise to merge this at all?
>
> There's two benefits IMO:
> - in many cases the function names displayed in the messages are just
> plain wrong (obsolete) or confusing - there's functions which hop
> from
> HERE to hand-coded wrong function names.
> - it should reduce the memory footprint as HERE strings can get
> aliased by the compiler
>
> Note that almost all changes are one-liners.
>
>> If the decision is to merge, please merge into trunk _and_ v3.3 (at
>> the
>> very least) to minimize porting overheads.
>
> Fine with me. I'll suspend making further changes until a consensus
> is reached.
>
>> My current preference is to change debugs() to use HERE when the
>> surrounding code has to be changed anyway OR when there is a good
>> reason
>> to add HERE to that specific debugs() line.
>
> I'm aiming at "let's get rid of the pain now" approach, but I don't
> want to cause more pain than I'm aiming to solve :)

Like Alex is hinting at if we do it *now* there is more pain than
necessary. This type of change does have "windows of least pain".
Somewhere around Feb next year would be better timing IMO, when we are
about to branch 3.4 for beta. Just like we did massive polish right
before staring 3.3 on its beta testing.

>
>> Long-term, we should consider making HERE in debugs() at level 2 or
>> higher an automatic/default behavior. This will require some work,
>> but I
>> think it is possible, and I think it can even accommodate existing
>> debugs() statements (with or without HERE) _without_ changing them.
>
> It's hairy at best, if it can even be done, as it depends on cpp
> predefined macro magic (__LINE__, __FILE__ etc).

Good point. Testing needed.

Amos
Received on Tue Nov 06 2012 - 00:32:03 MST