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

[Foundation] add additional conformances and functionality to NSRange #7433

Merged
merged 2 commits into from
May 5, 2017

Conversation

phausler
Copy link
Contributor

NSRange is used widely in a number of Cocoa APIs; it has a few free-floating functions used to manipulate and parse ranges. These functiosn should be hoisted into NSRange and protocols should be adopted where appropriate.

NSRange can be converted to a string; therefore it should adopt CustomStringConvertible and CustomDebugStringConvertible. It also has a free floating function NSEqualRanges which can more appropriately be expressed as adopting Equatable. Furthermore NSMaxRange can be exposed as upperBound to fall in line with Range. Also it can move NSUnionRange to be a member function to create new and mutate NSRanges.

Resolves:
rdar://problem/21626767

@phausler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 22289411841195b2b2b4e14ce52fac198aa11876
Test requested by - @phausler


// FIXME(ABI)#75 (Conditional Conformance): this API should be an extension on Range.
// Can't express it now because the compiler does not support conditional
// extensions with type equality constraints.
Copy link
Contributor

@karwa karwa Feb 13, 2017

Choose a reason for hiding this comment

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

The compiler does now support extensions with same-type constraints. The FIXME hint here looks wrong: this has nothing to do with conditional conformances.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That comment was not part of the additions here; it just got moved around

@phausler
Copy link
Contributor Author

@swift-ci please test and merge

1 similar comment
@phausler
Copy link
Contributor Author

@swift-ci please test and merge

@phausler
Copy link
Contributor Author

The previous build that failed was still building 2228941 instead of 526ac56

@phausler
Copy link
Contributor Author

@swift-ci test and merge

@phausler
Copy link
Contributor Author

phausler commented May 2, 2017

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 526ac56c3342659b6bd711b2303f3ee6b73f3ac3
Test requested by - @phausler

@phausler
Copy link
Contributor Author

phausler commented May 2, 2017

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 526ac56c3342659b6bd711b2303f3ee6b73f3ac3
Test requested by - @phausler

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 526ac56c3342659b6bd711b2303f3ee6b73f3ac3
Test requested by - @phausler

@@ -12,33 +12,117 @@

@_exported import Foundation // Clang module

extension NSRange : Hashable {
public var hashValue: Int {
return Int(bitPattern: UInt(location) ^ UInt(location))
Copy link
Contributor

Choose a reason for hiding this comment

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

length?

(Also, XOR is a pretty bad hash-combine for values that are likely to be close.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea that should have been length, I guess I could bit shift them by 32 bits and OR since NSRange (excluding NSNotFound locations) are usually very low bit values.

@@ -12,33 +12,117 @@

@_exported import Foundation // Clang module

extension NSRange : Hashable {
public var hashValue: Int {
return Int(bitPattern: (UInt(bitPattern: location) | (UInt(bitPattern: length) << 32)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrose-apple that should handle most ranges a bit better than before ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Do we care that this drops the length in 32-bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm good point, I guess I could shift by 16 on 32 bit arches

@phausler
Copy link
Contributor Author

phausler commented May 2, 2017

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 0fd0be8
Test requested by - @phausler

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 0fd0be8
Test requested by - @phausler

@phausler
Copy link
Contributor Author

phausler commented May 2, 2017

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 82125a62bce45f7c067b030cac0106a6683f322f
Test requested by - @phausler

@swift-ci
Copy link
Contributor

swift-ci commented May 2, 2017

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 82125a62bce45f7c067b030cac0106a6683f322f
Test requested by - @phausler

@phausler
Copy link
Contributor Author

phausler commented May 5, 2017

@swift-ci please test

@phausler phausler merged commit 0540022 into swiftlang:master May 5, 2017
@phausler
Copy link
Contributor Author

phausler commented May 6, 2017

@gparker42 is there a hash combine function available in the standard library? if so I would rather defer that type of thing to a single location. (granted most NSRanges are probably small values)

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.

5 participants