go
go copied to clipboard
golang
reflect: add Value.Equal, Value.Comparable
In Go 1.17 we have introduced a conversion that can panic (#395). This is the first case in which a conversion can panic. This means that code that calls reflect.Type.ConvertibleTo and then, if that returns true, calls reflect.Value.Convert, can see an unexpected panic. (See #46730.)
Separately, for a long time now it has been possible for a comparison to panic, when comparing two interface values that have the same dynamic type but for which the dynamic type is not comparable. Therefore, for a long time code that calls reflect.Type.Comparable and then, if that returns true, uses the == operator can see an unexpected panic. (This is a fairly uncommon case as the problem only arises when working with indirectly accessed interface types, such as pointers to interfaces.)
I propose adding two new methods to reflect.Value.
// ConvertibleTo reports whether v can be converted to type t.
// If this reports true then v.Convert(t) will not panic.
func (v Value) ConvertibleTo(t Type) bool
// Comparable reports whether the type of v is comparable.
// If the type of v is an interface, this checks the dynamic type.
// If this reports true then v.Interface() == x will not panic for any x.
func (v Value) Comparable() bool
One very minor observation:
// If this reports true then v.Interface() == x will not panic for any x.
I'm not sure this is quite right. x could be an interface containing a dynamic type that isn't comparable. In the common case, comparing two reflect.Values, you need to call Comparable on both of them. I don't have better wording to suggest.
I think the statement is still true, because if the dynamic types of v.Interface() and x are different, then the comparison is false, and it doesn't matter whether either or both of the dynamic types are not comparable. In other words, if the dynamic type of v.Interface() is comparable, then either x has a different dynamic type and the result of v.Interface() == x is false, or x has the same dynamic type and the comparison will be run without panicking.
Ah, indeed. Thanks. The relevant sentence from the spec is:
A comparison of two interface values with identical dynamic types causes a run-time panic if values of that type are not comparable.
We have:
func (v Value) Addr() Value
func (v Value) CanAddr() bool
func (v Value) Interface() interface{}
func (v Value) CanInterface() bool
So it sounds like we want to add the second one of these:
func (v Value) Convert(t Type) Value
func (v Value) CanConvert(t Type) bool
And maybe:
func (v Value) Equal(u Value) bool
func (v Value) Comparable(u Value) bool
Comparable seems like a better name than CanEqual here, but Equal seems better than Compare (compare bytes.Compare, bytes.Equal).
We probably want CanConvert at least for Go 1.17.
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group
Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group
If a Value is Comparable, does this mean the return from Interface() can be used as a key in a map? I don't think reflect currently has a way to answer this question.
(this is just a clarifying question, not a comment against the proposal)
We probably want CanConvert at least for Go 1.17.
Is this still true? The window for doing this is pretty small now.
If a Value is Comparable, does this mean the return from Interface() can be used as a key in a map? I don't think reflect currently has a way to answer this question.
Assuming you have a map[interface{}]T (for some value type T), then yes: if v.Comparable() returns true, you can use v.Interface() as a key value for that map, and no panic will occur.
Change https://golang.org/cl/334669 mentions this issue: reflect: add Value.CanConvert
I sent https://golang.org/cl/334669 in case we do want this in 1.17.
No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group
CanConvert has been added for 1.17. This issue remains open for Equal and Comparable in a later release.
Change https://go.dev/cl/423794 mentions this issue: reflect: add Value.Comparable
Reopen for more discussion about the behavior of Value.Equal for non-comparable. At this moment, the implementation always return false, assuming the two values are valid. The point of Value.Equal is to do the equivalent of ==, so what should we do for non-comparable values? Return false? Panic?
cc @cherrymui @ianlancetaylor
I think it's fine to return false.
Seems a bit counter-intuitive that:
x := reflect.ValueOf([]int{1, 2, 3})
x.Equal(x) // reports false
It's unfortunate to lose the reflexive property of equality, but I guess that's already the case for NaNs.
@dsnet Fair point. As far as I can tell the only other option is to panic, or to change the signature of Equal.
Given that the language itself panics for equality comparisons on incomparable types, I think it would make sense for Equal to panic for incomparable types, perhaps with a CanEqual method (akin to CanConvert, CanAddr, CanInterface, etc.) that can be used to easily avoid the panic.
Maybe? I don't know: the proposed doc comment is “Comparable reports whether the type of v is comparable”, but for struct types whether Equal panics depends on the (deep) value, not just its (shallow) type.
@ianlancetaylor
which is why Comparable is a method on Value, not Type.
It is actually the inverse?
@go101 The Value.Comparable method is new on tip, for 1.20, for #46746.
...which is why Comparable is a method on Value, not Type.
In that case, I think the documentation is confusing — it explicitly states that it reports (emphasis mine) “whether the type of v is comparable”, which on its face seems equivalent to v.Type().Comparable().
(The second sentence does state that “v.Interface() == x will not panic for any x”, but I would prefer that the behavior of the method be described accurately in the first sentence as well.)
Change https://go.dev/cl/435277 mentions this issue: reflect: clarify that Value.Comparable checks the value
We definitely explicitly chose above to call CanEqual Comparable instead (because it matches the language spec term).
If CanConvert returns false and you call Convert anyway, that panics. (Same for Addr, Set, Float, and so on.) So if Comparable returns false and you call Equal anyway, it seems like that should panic too.
In any other package panicking would probably not be the answer, but panicking is how reflect signals these kinds of conditions. It would be odd not to panic in this one function.
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group