Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-5521] elementsEqual doesn't work properly for Dictionary #48093

Open
mattneub opened this issue Jul 21, 2017 · 3 comments
Open

[SR-5521] elementsEqual doesn't work properly for Dictionary #48093

mattneub opened this issue Jul 21, 2017 · 3 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@mattneub
Copy link

Previous ID SR-5521
Radar None
Original Reporter @mattneub
Type Bug
Environment

Xcode Version 9.0 beta 3 (9M174d)

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: 031153430bb6642d6654eb26a485b95f

Issue Description:

`elementsEqual` is defined for Dictionary but it appears not to be implemented correctly behind the scenes. Here's my test example:

struct Dog : Equatable {
    let name : String
    static func ==(lhs:Dog,rhs:Dog) -> Bool {
        return lhs.name == rhs.name
    }
}
var d1 = [String:Dog]()
d1["dog"] = Dog(name:"Fido")
d1["doog"] = Dog(name:"Fido")
var d2 = [String:Dog]()
d2["doog"] = Dog(name:"Fido")
d2["dog"] = Dog(name:"Fido")
let ok = d1.elementsEqual(d2) { $0.key == $1.key && $0.value.name == $1.value.name }
ok // false
let ok2 = d1 == d2
ok2 // true

When I test equality using `==`, I get `true`, the correct answer. But when I try to write my own test using `elementsEqual`, I get `false`, even though I am writing the equality test exactly as I believe `==` must be implementing it.

The problem appears to be that I am being handed the elements in order. But Dictionary elements have no order! I need to be handed only pairs whose keys are known to be hash-equivalent. To put it another way, I shouldn't need to compare keys at all, since keys are guaranteed equatable; I should need only to compare values. I did try that, but it didn't work either:

var d1 = [String:Dog]()
d1["dog"] = Dog(name:"Fido")
d1["doog"] = Dog(name:"Fidro")
var d2 = [String:Dog]()
d2["doog"] = Dog(name:"Fidro")
d2["dog"] = Dog(name:"Fido")
let ok = d1.elementsEqual(d2) { return $0.value.name == $1.value.name }
ok // false
let ok2 = d1 == d2
ok2 // true

Thus I would suggest that `elementsEqual` should not even be defined for Dictionary, since it doesn't work properly — or, if it is defined then it should work.

Of course it's perfectly possible that I'm failing to understand what should happen here, so feel free to correct me.

@natecook1000
Copy link
Member

We have a bit of a tricky time talking about sets and dictionaries, because while they don't guarantee a particular ordering (the elements may appear to be in any order regardless of how you add them, and that order may change between different executions of the same program), individual instances are ordered: If you iterate the elements of a single dictionary multiple times, you get the elements in the same order every time.

elementsEqual is a method from the Sequence protocol that simply iterates over two sequences and compares the elements of each as they're produced, and its semantic guarantee is that it returns true when two sequences have the same elements in the same order. It isn't a protocol requirement, so Dictionary and Set can't truly customize their implementations in a way that would work consistently.

The == operator has a different semantic guarantee: it returns true when its two operands are equal, which in Swift means that they are substitutable. Individual types are free to figure out what substitutable means for them, and the order of the elements isn't important when determining whether two sets or dictionaries are equal. Therefore, == for dictionaries is written using a different algorithm than elementsEqual—it iterates over one dictionary's keys, then makes sure that both dictionaries have the same value for that key.

We definitely need better documentation on elementsEqual, clearly indicating the semantics and probably calling out that these aren't a good idea to use for sets and dictionaries. Another step would be to make elementsEqual a protocol requirement, so it could be customized, but that opens up some other questions: Should startsWith and lexicographicallyPrecedes also be customized? Is it a problem if elementsEqual returns true, but the .first element of two dictionaries is different? etc.

cc @airspeedswift

@airspeedswift
Copy link
Member

Agree with everything @natecook1000 is saying here, and believe a documentation fix is the only appropriate fix.

Alternatives would involve constructing a more elaborate scheme where we had an OrderedSequence that declared that ordering was meaningful, and conform some collections to that but not others. But this would lead to all sorts of other complications and I doubt it would be worth it. Dynamically dispatching elementsEqual for Dictionary to perform == also would be inconsistent in other ways – it wouldn't work for comparing an arbitrary "ordered" Sequence to a Dictionary or comparing, say, Dictionary.Keys to a Set.

@mattneub
Copy link
Author

@natecook1000 and @airspeedswift Certainly warning me off in the documentation would have helped, though I still rather wish the compiler would alert me (or even stop me) when I try to use `elementsEqual` on a dictionary (or set). Thanks!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

3 participants