Skip to content

[stdlib] Fix issue with UTF16 index(_:offsetBy:limitedBy) #12378

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 16, 2017
Merged

[stdlib] Fix issue with UTF16 index(_:offsetBy:limitedBy) #12378

merged 2 commits into from
Oct 16, 2017

Conversation

airspeedswift
Copy link
Member

@airspeedswift airspeedswift commented Oct 10, 2017 •
edited
Loading

When the string is empty, the code unit distance to the limit can be zero.

Manifests in this failing:

let empty = ""
// should be nil but instead traps
Range(NSRange(location: 1, length: 0), in: empty)

rdar://problem/34551055

@airspeedswift airspeedswift requested a review from moiseev October 10, 2017 23:02
@airspeedswift
Copy link
Member Author

@swift-ci please test

@moiseev
Copy link
Contributor

moiseev commented Oct 12, 2017

This seems to be fixing https://bugs.swift.org/browse/SR-4657.
Can we have a test case that proves that? Otherwise LGTM!

@airspeedswift
Copy link
Member Author

@swift-ci please test and merge

@airspeedswift
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - bc300aa6e698246fdb19c0f1ac579c5da0688107

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - bc300aa6e698246fdb19c0f1ac579c5da0688107

moiseev
moiseev reviewed Oct 13, 2017
View reviewed changes
test/stdlib/StringTraps.swift
}

StringTraps.test("UTF16ViewIndex/offsetLimited")
.code {
Copy link
Contributor

Choose a reason for hiding this comment

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

.code is not necessary.

moiseev
moiseev reviewed Oct 13, 2017
View reviewed changes
test/stdlib/StringTraps.swift

let iaBegin = u16a.index(sa.startIndex, offsetBy: 99, limitedBy: sa.endIndex)
expectNil(iaBegin)
let iaEnd = u16a.index(sa.endIndex, offsetBy: 99, limitedBy: sa.endIndex)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be interesting to test for non-positive offsets starting from an endIndex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, good point!

@tkremenek
Copy link
Member

@swift-ci test source compatibility

@airspeedswift airspeedswift merged commit a70e857 into swiftlang:master Oct 16, 2017
@airspeedswift airspeedswift deleted the nsrange-in-string branch October 16, 2017 20:44
@moiseev moiseev mannequin mentioned this pull request Oct 13, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moiseev moiseev moiseev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@airspeedswift @moiseev @swift-ci @tkremenek