Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

-hash is broken on CPArray

#286

It breaks the contract that two arrays which are -isEqual: also need to report the same -hash value.

Patch and tests included

It's also the same way that GnuStep and Cocotron implement this - I was quite surprised how simple they implemented it.

:)

Reported by Martin Häcker · April 30th, 2009 @ 02:06 AM

State: new
Milestone: none
Assigned to: Ross Boucher Ross Boucher

Activity

  1. Ross Boucher
    Ross Boucher

    Why do both array's need to return the same hash value? That doesn't seem to be documented anywhere in the Cocoa docs.

    This code seems incredibly dangerous to me. You're essentially saying that all kinds of arrays that have nothing to do with each other could be considered equal. This will break anything that uses hash as a unique identifier (for example, KVO/Bindings, and I'm sure other code in Cappuccino).

    April 30th, 2009 @ 08:33 AM

  2. Ross Boucher
    Ross Boucher

    I spoke before I understood what I was talking about.

    From the documentation for hash:

    @"If two objects are equal (as determined by the isEqual: method), they must have the same hash value. This last point is particularly important if you define hash in a subclass and intend to put instances of that subclass into a collection."

    Our implementation of hash does not behave this way. Perhaps we should consider changing our hash to become address, and re-implementing hash the Cocoa way?

    April 30th, 2009 @ 09:05 AM

  3. Martin Häcker
    Martin Häcker

    I'm not sure I understand you correctly, -[CPObject hash] does return __adress, I haven't checked all the other objects, but all objects who redefine -isEqual: need their own implementation that matches the contract.

    The one which I provided for CPArray - and I checked it against the two other Cocoa implementations I know, which do it the same way. (Also it makes sense: The Implementation needs to be FAST, and it needs to be the same when two objects are -isEqual:

    I also checked against some examples on Cocoa and it behaves the same way.

    April 30th, 2009 @ 10:23 AM

  4. Ross Boucher
    Ross Boucher

    My point was that -hash works differently in Cappuccino than in Cocoa (something I previously had not realized).

    We need access to __address as a method, which is what hash has been doing for us. To make -hash actually correct, we'll need to add -address and update all of the cappuccino code that relies on hashes being globally unique.

    April 30th, 2009 @ 10:27 AM

  5. Martin Häcker
    Martin Häcker

    Too late to finish sentences...

    The one which I provided does fit the contract and also behaves the same way that the others do.

    April 30th, 2009 @ 10:30 AM

  6. Ross Boucher
    Ross Boucher

    Yes, but it breaks parts of Cappuccino that expect the current behavior. For example, KeyValueObserving and CPWindow both rely on the value of -hash as a unique index into various data structures.

    April 30th, 2009 @ 10:32 AM

  7. Martin Häcker
    Martin Häcker

    Oh damn.

    I just had a look around in the source and lots of objects like CPDictionary, CPData, CPDate and CPNull don't overide -hash but should. (And most should probably also overide isEqual:)

    Uh oh....

    Yes, this is going to take some work.

    April 30th, 2009 @ 10:54 AM

  8. Martin Häcker
    Martin Häcker

    Can't resist...

    As a good person once said: In God we trust - for everything else we have UnitTests.

    :)

    April 30th, 2009 @ 11:11 PM

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.