Open StefanKarpinski opened 11 years ago
Triage prefers the following solution, which was spelt out above in parts, but not in one place
==
, recursively call isequal
This is basically what AutoHashEqual does, except that it seems slightly better in the (somewhat unusual) case that you have type parameters just used for tagging, but not in the fields.
I just remembered the original issue calls for doing this for immutable types, but it seems we're leaning towards using it for everything? That brings up (1) object cycles, (2) strange default behavior for things like Pipes.
The way I see it, there are thousands of julia types out there, call the number N
. Some number E
need to have boilerplate ==
methods defined. There's a cognitive bias: as soon as you've written that boilerplate 3 or 4 times, it seems annoying, but in fact E ≪ N
. If we change the default, then N - E
types need a ==
method that calls ===
to be safe. Otherwise you can get stack overflows or problematic spurious equalities.
So it was not discussed (much?) on the call, but do we want to limit this to immutables?
Pondering this some more after the call, I'm thinking that the current default of not-equal has been a much better default than field-equality (if we're going to define equals at all). For example, it's seems odd that we'll now need to define ==(a::IO, b::IO) = (a === b)
to avoid spurious equivalences, such that (after this change) x in [y]
might return true for x !== y
.
It seems like there's probably many other cases like that too, such as Condition
(will be now considered equal if Tasks with the same starting function + state are waiting on them) and Channel
(will be equal if they contain the same data, for example, if they're both empty).
I would continue to favor doing this for mutable types, though I feel less strongly about that. I'm not really concerned about getting StackOverflowErrors for circular references by default, since e.g. the same happens here:
julia> a = Any[]
0-element Array{Any,1}
julia> push!(a, a)
1-element Array{Any,1}:
Any[Any[#= circular reference @-1 =#]]
julia> a == a
ERROR: StackOverflowError:
But it can cause stack overflows in cases where people were relying on the ===
fallback.
I think this is still controversial and doesn't need to be done right now. Moving to 2.0.
It might be handy to at least have structural equality as a standard library function that can be used for user-defined types. For example, I've been using this generated function for most of my types to avoid manual equality definition:
# ASSUMTION: `e1` and `e2` have the same run-time type
@generated structEqual(e1, e2) = begin
if fieldcount(e1) == 0
return :(true)
end
mkEq = fldName -> :(e1.$fldName == e2.$fldName)
# generate individual equality checks
eqExprs = map(mkEq, fieldnames(e1))
# construct &&-expression for chaining all checks
mkAnd = (expr, acc) -> Expr(:&&, expr, acc)
# no need in initial accumulator because eqExprs is not empty
foldr(mkAnd, eqExprs)
end
And then use it to define equality:
Base.:(==)(x :: MyType, y :: MyType) = structEqual(x, y)
It might be handy to at least have structural equality as a standard library function that can be used for user-defined types. For example, I've been using this generated function for most of my types to avoid manual equality definition:
# ASSUMTION: `e1` and `e2` have the same run-time type @generated structEqual(e1, e2) = begin if fieldcount(e1) == 0 return :(true) end mkEq = fldName -> :(e1.$fldName == e2.$fldName) # generate individual equality checks eqExprs = map(mkEq, fieldnames(e1)) # construct &&-expression for chaining all checks mkAnd = (expr, acc) -> Expr(:&&, expr, acc) # no need in initial accumulator because eqExprs is not empty foldr(mkAnd, eqExprs) end
And then use it to define equality:
Base.:(==)(x :: MyType, y :: MyType) = structEqual(x, y)
awesome idea!
I included it into a related package I myself build, and extended it to similar cases. Enjoy https://github.com/jolin-io/StructEquality.jl
@schlichtanders: I like that you exposed the function versions as part of the API. My problem with macros like @auto_hash_equals
is that they are difficult to compose in case I want multiple extensions to the basic struct
functionality.
This doesn't make much sense:
If the fields are
==
to each other then the objects should be==
to each other.