[SR-6566] Indefinite behavior from Foundation's NSMutableOrderedSet

[swift-ci]

Previous ID	SR-6566
Radar	rdar://35970034
Original Reporter	spc (JIRA User)
Type	Bug

Attachment: Download

Environment

Version 9.2 (9C40b) Apple Swift version 4.0.3 (swiftlang-900.0.74.1 clang-900.0.39.2) Target: x86_64-apple-macosx10.9

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 6ce283b354ae66dca1d41f1417b609eb

Issue Description:

NSMutableOrderedSet from Swift's Foundation library has incorrect behavior.

1. When adding an Any/AnyObject to NSMutableOrderedSet, the set itself should check for existence of duplicated object. Currently it has indefinite behavior.

2. When removing an Any/AnyObject from NSMutableOrderedSet, the set itself should check for existence of duplicated object. Current it has indefinite behavior.

[belkadan]

That's bizarre. I still see it on master too, even changing isEqual to something a little simpler:

    override func isEqual(_ object: Any?) -> Bool {
        guard object != nil else { return false }
        guard type(of: self) == type(of: object!) else { return false }
        return self.i == (object as! SomeClass).i
    }

It looks like this Objective-C program has the same problem, though, which makes me think this is a problem with NSMutableOrderedSet rather than with Swift. Do you want to file a bug through https://bugreport.apple.com or should I?

@import Foundation;

@interface SomeClass: NSObject
@property (readonly) NSInteger i;
@end

@implementation SomeClass
- (nonnull instancetype)init:(NSInteger)i {
  _i = i;
  return self;
}

- (NSInteger)hashValue {
  return self.i;
}

- (BOOL)isEqual:(nullable id)other {
  if (!other) return NO;
  if ([other class] != [self class]) return NO;
  return self.i == ((SomeClass *)other).i;
}

- (NSString *)description {
  return [NSString stringWithFormat: @"<%ld>", (long)self.i];
}
@end

int main(void) {
  @autoreleasepool {
    NSArray *array = @[[[SomeClass alloc] init:1], [[SomeClass alloc] init:2], [[SomeClass alloc] init:3], [[SomeClass alloc] init:4]];
    NSMutableOrderedSet *s = [[NSMutableOrderedSet alloc] initWithArray:array];
    NSLog(@"%@", s);
    [s removeObject:[[SomeClass alloc] init:3]];
    NSLog(@"%@", s);
    [s insertObject:[[SomeClass alloc] init:3] atIndex:0];
    NSLog(@"%@", s);
  }
}

[millenomi]

Filed.

[millenomi]

@belkadan — the ObjC test-case is implementing -hashValue, but NSMutableOrderedSet uses -[NSObject hash] to get a hash value, hence the mismatch.

[belkadan]

Ahh, and the same would be true in Swift. That explains it.

Hm. The right answer would be to make NSObject's hashValue public instead of open (and even final), but that would break source compatibility. Maybe we just need a one-off compiler or linter warning, or something like that.

[belkadan]

(The real right answer would be to have either picked hash for Swift's Hashable protocol instead of hashValue, or to have manually remapped -[NSObject hash] to be named hashValue way back in Swift 1. Unfortunately it's far too late to do either of those.)

[millenomi]

NSMutableOrderedSet in Swift Foundation uses the Hashable protocol directly, unlike the ObjC version, but this should've worked — NSObject should be Hashable. I'll look into this.

[belkadan]

Oh, I didn't try it with the corelibs Foundation. I was talking about the conformance of NSObject to Hashable in the ObjectiveC overlay for Darwin platforms. That said, corelibs NSObject still defines both hash and hashValue and I'm not sure what happens if you override them inconsistently.