That is great news!
I didn’t associate operator overloading with Equatable and Hashable so didn’t realize this is now a non-issue.
How is this implemented? Does it work for a stored static property, or a property with a private setter, when comparing a key path referenced in the same module vs. one referenced from outside? We generally need a property descriptor to do that correctly, and static properties would not have one unless built with a new compiler.
It works with a stored static property and property with a private setter when comparing a key path references in the same module vs one referenced outside. Here is the test case for that example in the implementation.
Here is what I understood:
In GenKeyPath.cpp, emitKeyPathComponent treats externally referenced properties as KeyPathPatternComponent::Kind::GettableProperty/SettableProperty (so stored properties also follow this logic). This encodes the descriptor with an external header, followed by the settability and id ref/kind/value which all get stored in the "local" KeyPathComponent Header.
In KeyPath.swift, if the descriptor is missing, then the descriptor header is also unavailable. In the implementation, I can skip the nil descriptor and reference the "local candidate header" to check mutability and id ref/kind/value via case computed: .... walker.visitComputedComponent(...) instead.
KeyPath's == implementation only looks at the component's type and header information, so this approach seems to work.
Please let me know if there are any other test cases I should consider.
1 Like
It looks like there was a recent regression in key path equality that might've caused us to inappropriately consider key paths equal if their components are equal but their root, value, or capability types are different. With this bug, it's possible that we'd treat a read-only key path created without visibility of a component's setter as being equal to a writable key path created with visibility of the setter. You might double-check that the behavior with static properties doesn't change with the equality implementation fixed.
3 Likes