Skip to content

Use NSArray/NSMutableArray and NSSet/NSMutableSet in NSOrderedSet #1816

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

Merged
merged 2 commits into from
Feb 14, 2019
Merged

Use NSArray/NSMutableArray and NSSet/NSMutableSet in NSOrderedSet #1816

merged 2 commits into from
Feb 14, 2019

Conversation

amorde
Copy link
Contributor

@amorde amorde commented Jan 6, 2019

Taking another stab at #1367

This cleans up a lot of the implementation since we can defer work to NS{Mutable}Array and NS{Mutable}Set

NSRequiresConcreteImplementation()
}
return NSGeneratorEnumerator(_orderedStorage.map { __SwiftValue.fetch(nonOptional: $0) }.makeIterator())
return _orderedStorage.objectEnumerator()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the interesting side effect of making these enumerators behave similar to Darwin - they will now break if mutation occurs during iteration. The previous implementation made a copy of the all the objects using map and therefore did not have this issue.

@@ -62,6 +61,126 @@ class TestNSOrderedSet : XCTestCase {
index += 1
}
}

func test_enumerationUsingBlock() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was copied from NSArray tests for coverage on concurrent enumeration

@@ -544,7 +581,8 @@ extension NSMutableOrderedSet {
}

open func intersectSet(_ other: Set<AnyHashable>) {
for case let item as AnyHashable in self where !other.contains(item) {
let objects = Array(self)
for case let item as AnyHashable in objects where !other.contains(item) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the enumerator now breaks if mutation occurs, we have to make a copy here

@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

@swift-ci swift-ci merged commit 3c41fb2 into swiftlang:master Feb 14, 2019
@amorde amorde deleted the nsorderedset-revisited branch February 14, 2019 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants