rs / zerolog

Zero Allocation JSON Logger
MIT License
10.41k stars 567 forks source link

Invalid condition in WithContext method. #498

Closed tep closed 1 year ago

tep commented 1 year ago

Since non-pointer receivers are passed by value, once the receiver for WithContext was reverted to a non-pointer, this condition is guaranteed to always be false.

In other words, the receiver (l) is a copy of the Logger struct from the caller and its address can never be the same as the logger attached to the Context.

Either the receiver should be converted back to a pointer (and different fixes devised for #116 and #400) -- or, this condition should simply be removed and the docs updated to say that the Context will always be replaced no matter what (because that's the actual behavior with the current logic).

[Here's a Go Playground link proving that non-pointer receivers have different addresses from those of their caller]

rs commented 1 year ago

This is a regression from this commit: https://github.com/rs/zerolog/commit/588a61c2df4bf7b92e12b826b226059b9f262190

rs commented 1 year ago

Perhaps we should remove the condition altogether.

tep commented 1 year ago

That's what I'd originally done in PR #499 but then changed it to add back a similar condition using reflect.DeepEqual but, I can always pull it out again.

rs commented 1 year ago

Ok, I'll prefer removing the condition then.

tep commented 1 year ago

Done.

499 should be ready to go.