Using F#'s hash function inside GetHashCode() evil?

Question

I encountered a couple of places online where code looked something like this:

[<CustomEquality;NoComparison>]
type Test =
    | Foo
    | Bar
    override x.Equals y = 
        match y with
        | :? Test as y' ->
            match y' with
            | Foo -> false
            | Bar -> true    // silly, I know, but not the question here
        | _ -> failwith "error"   // don't do this at home

    override x.GetHashCode() = hash x

But when I run the above in FSI, the prompt does not return when I either call hash foo on an instance of Test or when I call foo.GetHashCode() directly.

let foo = Test.Foo;;
hash foo;;   // no returning to the console until Ctrl-break
foo.GetHashCode();;  // no return

I couldn't readily proof it, but it suggests that hash x calls GetHashCode() on the object, which means the above code is dangerous. Or is it just FSI playing up?

I thought code like the above just means "please implement custom equality, but leave the hash function as default".

I have meanwhile implemented this pattern differently, but am still wondering whether I am correct in assuming that hash just calls GetHashCode(), leading to an eternal loop.

~~As an aside, using equality inside FSI returns immediately, suggesting that it either does not call GetHashCode() prior to comparison, or it does something else.~~ Update: this makes sense as in the example above x.Equals does not call GetHashCode(), and the equality operator calls into Equals, not into GetHashCode().

score 5 · Answer 1 · answered Sep 25 '16 at 17:46

5

If the GetHashCode() method is overridden, then the hash operator will use that:

[The hash operator is a] generic hash function, designed to return equal hash values for items that are equal according to the = operator. By default it will use structural hashing for F# union, record and tuple types, hashing the complete contents of the type. The exact behavior of the function can be adjusted on a type-by-type basis by implementing System.Object.GetHashCode for each type.

So yes, this is a bad idea and it makes sense that it would lead to an infinite loop.

answered Sep 25 '16 at 17:46

JLRishe

99,490
19
131
169

_"The exact behavior of the function can be adjusted on a type-by-type basis by implementing System.Object.GetHashCode for each type."_, I noticed that too, but it doesn't say it will call `GetHashCode()`. I.e., it could be entirely reasonable to write `x.GetHashCode() = if x < 0 then 0 else hash x` (i.e., if anything below zero is to be considered equal). – Abel Sep 25 '16 at 17:57
Actually, I would have expected `hash` to call into `base.GetHashCode()` instead, which would _not_ lead to infinite recursion. – Abel Sep 25 '16 at 17:58
@Abel It says that you can change how the `hash` function behaves for a type by overriding `GetHashCode` how would its behavior change if it didn't actually call `GetHashCode()`? It sounds like you're interpreting that sentence to mean "You can use the `hash` function in overriding `GetHashCode`", which is the _opposite_ of what it says. – JLRishe Sep 25 '16 at 18:02
Not quite. I was interpreting it that it allowed special-case override, defaulting back to the original for other cases. But I see your point. – Abel Sep 25 '16 at 18:12

score 5 · Accepted Answer · answered Sep 25 '16 at 18:01

It's not quite as simple as the hash function simply being a wrapper for GetHashCode but I can comfortably tell you that it's definitely not safe to use the implementation : override x.GetHashCode() = hash x.

If you trace the hash function through, you end up here:

let rec GenericHashParamObj (iec : System.Collections.IEqualityComparer) (x: obj) : int =
    match x with 
    | null -> 0 
    | (:? System.Array as a) -> 
        match a with 
        | :? (obj[]) as oa -> GenericHashObjArray iec oa 
        | :? (byte[]) as ba -> GenericHashByteArray ba 
        | :? (int[]) as ba -> GenericHashInt32Array ba 
        | :? (int64[]) as ba -> GenericHashInt64Array ba 
        | _ -> GenericHashArbArray iec a 
    | :? IStructuralEquatable as a ->    
        a.GetHashCode(iec)
    | _ -> 
        x.GetHashCode()

You can see here that the wild-card case calls x.GetHashCode(), hence it's very possible to find yourself in an infinite recursion.

The only case I can see where you might want to use hash inside an implementation of GetHashCode() would be when you are manually hashing some of an object's members to produce a hash code.

There is a (very old) example of using hash inside GetHashCode() in this way in Don Syme's WebLog.

By the way, that's not the only thing unsafe about the code you posted.

Overrides for object.Equals absolutely must not throw exceptions. If the types do not match, they are to return false. This is clearly documented in System.Object.

Implementations of Equals must not throw exceptions; they should always return a value. For example, if obj is null, the Equals method should return false instead of throwing an ArgumentNullException.

(Source)

_"would be when you are manually hashing some of an object's members"_, yes, this actually started with creating a comparable function type, similar to `string * ('T -> 'U)`, where in the hash override I called `hash s` on the string (so, no infinite recursion there). But when I saw those threads online, I figured, hey, let's try it out... Leading to this question. — Abel, Sep 25 '16 at 18:04
Thanks for the pointer to the source code. And about your comment on the exception: you are right, bad example code... I was fooling around. — Abel, Sep 25 '16 at 18:05
@Abel Yep, I thought you probably knew this but I thought it worth putting out there for other people who view this question/answer because it's one of those common little mistakes that's quite easy to make. — TheInnerLight, Sep 25 '16 at 18:12
You could say the same for me making `foo = bar` being _true_ and `bar = foo` being _false_, which I think would also be really bad practice ;). — Abel, Sep 25 '16 at 18:14

Using F#'s hash function inside GetHashCode() evil?

2 Answers2