docs: clarify Equal method is skipped when either value is nil - #397
Open
Solaris-star wants to merge 1 commit into
Open
docs: clarify Equal method is skipped when either value is nil#397Solaris-star wants to merge 1 commit into
Solaris-star wants to merge 1 commit into
Conversation
The package docs claimed x.Equal(y) runs "even if x or y is nil", but pointers/interfaces short-circuit on nil before tryMethod, so a nil receiver is never used. Document the actual behavior. Fixes google#363
dsnet
requested changes
Jul 30, 2026
| // x.Equal(y) when both values are non-nil. If either value is nil, Equal | ||
| // reports whether both are nil without calling the method (so a nil | ||
| // receiver never panics). Otherwise, no such method exists and evaluation | ||
| // proceeds to the next rule. |
Collaborator
There was a problem hiding this comment.
Thanks for the PR, but I don't think this is quite correct.
The difference of whether Equal is called on nil is whether it is declared on a value receiver or not. Perhaps this is more clear:
If the values have an Equal method of the form "(T) Equal(T) bool" or
"(T) Equal(I) bool" where T is assignable to I, then use the result of
x.Equal(y). The method is called even if x or y are nil and Equal
is declared on a pointer receiver. Otherwise, no such method exists and
evaluation proceeds to the next rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Package docs for
Equalsaid the method is used even if x or y is nil.Actual behavior:
comparePtr/compareInterfacereport nil equality first and return, sotryMethodnever runs when either side is nil. Non-nil values still callEqualas documented.This matches issue #363 and avoids implying a nil receiver would be invoked (and potentially panic).
Test plan
*Twith value-receiverEqualdoes not print / call methodgo test ./cmp/Fixes #363