Skip to content

Commit 67adcab

Browse files
committed
Apply notes from code review
1 parent 58ab3fe commit 67adcab

File tree

4 files changed

+36
-42
lines changed

4 files changed

+36
-42
lines changed

stdlib/public/core/StringCharacterView.swift

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ extension String: BidirectionalCollection {
132132
/// `index(before:)`.
133133
/// - Complexity: O(*n*), where *n* is the absolute value of `distance`.
134134
public func index(_ i: Index, offsetBy distance: Int) -> Index {
135-
// Note: in Swift 5.6 and below, this method used to be inlinable,
136-
// forwarding to `_index(_:offsetBy:)`.
135+
// Note: prior to Swift 5.7, this method used to be inlinable, forwarding to
136+
// `_index(_:offsetBy:)`.
137137

138138
// TODO: known-ASCII and single-scalar-grapheme fast path, etc.
139139

@@ -194,8 +194,8 @@ extension String: BidirectionalCollection {
194194
public func index(
195195
_ i: Index, offsetBy distance: Int, limitedBy limit: Index
196196
) -> Index? {
197-
// Note: In Swift 5.6 and below, this function used to be inlinable,
198-
// forwarding to `BidirectionalCollection._index(_:offsetBy:limitedBy:)`.
197+
// Note: Prior to Swift 5.7, this function used to be inlinable, forwarding
198+
// to `BidirectionalCollection._index(_:offsetBy:limitedBy:)`.
199199
// Unfortunately, that approach isn't compatible with SE-0180, as it doesn't
200200
// support cases where `i` or `limit` aren't character aligned.
201201

@@ -242,8 +242,8 @@ extension String: BidirectionalCollection {
242242
///
243243
/// - Complexity: O(*n*), where *n* is the resulting distance.
244244
public func distance(from start: Index, to end: Index) -> Int {
245-
// Note: In Swift 5.6 and below, this function used to be inlinable,
246-
// forwarding to `BidirectionalCollection._distance(from:to:)`.
245+
// Note: Prior to Swift 5.7, this function used to be inlinable, forwarding
246+
// to `BidirectionalCollection._distance(from:to:)`.
247247

248248
// FIXME: Due to the `index(after:)` problem above, this function doesn't
249249
// always return consistent results when the given indices fall between
@@ -266,8 +266,7 @@ extension String: BidirectionalCollection {
266266
count += 1
267267
i = _uncheckedIndex(after: i)
268268
}
269-
}
270-
else if i > end {
269+
} else if i > end {
271270
while i > end { // Note `<` instead of `==`
272271
count -= 1
273272
i = _uncheckedIndex(before: i)

stdlib/public/core/StringGutsSlice.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@ internal struct _StringGutsSlice {
3131
internal init(_ guts: _StringGuts, _ offsetRange: Range<Int>) {
3232
_internalInvariant(
3333
offsetRange.lowerBound >= 0 && offsetRange.upperBound <= guts.count)
34-
_internalInvariant(
35-
guts.isOnUnicodeScalarBoundary(offsetRange.lowerBound)
36-
&& guts.isOnUnicodeScalarBoundary(offsetRange.upperBound))
3734
self._guts = guts
3835
self._offsetRange = offsetRange
3936
}

stdlib/public/core/StringUnicodeScalarView.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ extension String.UnicodeScalarView: BidirectionalCollection {
183183
count += 1
184184
i = _uncheckedIndex(after: i)
185185
}
186-
}
187-
else if i > end {
186+
} else if i > end {
188187
while i > end {
189188
count -= 1
190189
i = _uncheckedIndex(before: i)

stdlib/public/core/Substring.swift

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,9 @@ extension Substring: StringProtocol {
200200
public var endIndex: Index { _slice._endIndex }
201201

202202
public func index(after i: Index) -> Index {
203-
// Note: in Swift 5.6 and below, this method used to be inlinable,
204-
// forwarding to `_slice.base.index(after:)`. Unfortunately, that approach
205-
// isn't compatible with SE-0180, as it allows Unicode scalars outside the
203+
// Note: Prior to Swift 5.7, this method used to be inlinable, forwarding to
204+
// `_slice.base.index(after:)`. Unfortunately, that approach isn't
205+
// compatible with SE-0180, as it allows Unicode scalars outside the
206206
// substring to affect grapheme breaking results within the substring. This
207207
// leads to Collection conformance issues when the `Substring`'s bounds do
208208
// not fall on grapheme boundaries in `base`.
@@ -256,9 +256,9 @@ extension Substring: StringProtocol {
256256
}
257257

258258
public func index(before i: Index) -> Index {
259-
// Note: in Swift 5.6 and below, this method used to be inlinable,
260-
// forwarding to `_slice.base.index(before:)`. Unfortunately, that approach
261-
// isn't compatible with SE-0180, as it allows Unicode scalars outside the
259+
// Note: Prior to Swift 5.7, this method used to be inlinable, forwarding to
260+
// `_slice.base.index(before:)`. Unfortunately, that approach isn't
261+
// compatible with SE-0180, as it allows Unicode scalars outside the
262262
// substring to affect grapheme breaking results within the substring. This
263263
// leads to Collection conformance issues when the `Substring`'s bounds do
264264
// not fall on grapheme boundaries in `base`.
@@ -294,26 +294,26 @@ extension Substring: StringProtocol {
294294
_internalInvariant(priorOffset >= startIndex._encodedOffset)
295295

296296
var r = Index(
297-
encodedOffset: priorOffset, characterStride: priorStride)
297+
encodedOffset: priorOffset, characterStride: priorStride
298+
)._scalarAligned
298299

299300
// Don't set the `_isCharacterAligned` bit in indices of exotic substrings
300301
// whose startIndex isn't aligned on a grapheme cluster boundary. (Their
301302
// grapheme breaks may not match with those in `base`.)
302303
if _startIsCharacterAligned {
303304
r = r._characterAligned
304-
} else {
305-
r = r._scalarAligned
306305
}
306+
307307
return r
308308
}
309309

310310
public func index(_ i: Index, offsetBy distance: Int) -> Index {
311-
// Note: in Swift 5.6 and below, this method used to be inlinable,
312-
// forwarding to `_slice.base.index(_:offsetBy:)`. Unfortunately, that
313-
// approach isn't compatible with SE-0180, as it allows Unicode scalars
314-
// outside the substring to affect grapheme breaking results within the
315-
// substring. This leads to Collection conformance issues when the
316-
// `Substring`'s bounds do not fall on grapheme boundaries in `base`.
311+
// Note: Prior to Swift 5.7, this method used to be inlinable, forwarding to
312+
// `_slice.base.index(_:offsetBy:)`. Unfortunately, that approach isn't
313+
// compatible with SE-0180, as it allows Unicode scalars outside the
314+
// substring to affect grapheme breaking results within the substring. This
315+
// leads to Collection conformance issues when the `Substring`'s bounds do
316+
// not fall on grapheme boundaries in `base`.
317317

318318
// TODO: known-ASCII and single-scalar-grapheme fast path, etc.
319319
var i = _wholeGuts.validateInclusiveCharacterIndex(i, in: _bounds)
@@ -334,12 +334,12 @@ extension Substring: StringProtocol {
334334
public func index(
335335
_ i: Index, offsetBy distance: Int, limitedBy limit: Index
336336
) -> Index? {
337-
// Note: in Swift 5.6 and below, this method used to be inlinable,
338-
// forwarding to `_slice.base.index(_:offsetBy:limitedBy:)`. Unfortunately,
339-
// that approach isn't compatible with SE-0180, as it allows Unicode scalars
340-
// outside the substring to affect grapheme breaking results within the
341-
// substring. This leads to Collection conformance issues when the
342-
// `Substring`'s bounds do not fall on grapheme boundaries in `base`.
337+
// Note: Prior to Swift 5.7, this method used to be inlinable, forwarding to
338+
// `_slice.base.index(_:offsetBy:limitedBy:)`. Unfortunately, that approach
339+
// isn't compatible with SE-0180, as it allows Unicode scalars outside the
340+
// substring to affect grapheme breaking results within the substring. This
341+
// leads to Collection conformance issues when the `Substring`'s bounds do
342+
// not fall on grapheme boundaries in `base`.
343343

344344
// Per SE-0180, `i` and `limit` are allowed to fall in between grapheme
345345
// breaks, in which case this function must still terminate without trapping
@@ -372,12 +372,12 @@ extension Substring: StringProtocol {
372372
}
373373

374374
public func distance(from start: Index, to end: Index) -> Int {
375-
// Note: in Swift 5.6 and below, this method used to be inlinable,
376-
// forwarding to `_slice.base.distance(from:to:)`. Unfortunately, that
377-
// approach isn't compatible with SE-0180, as it allows Unicode scalars
378-
// outside the substring to affect grapheme breaking results within the
379-
// substring. This leads to Collection conformance issues when the
380-
// `Substring`'s bounds do not fall on grapheme boundaries in `base`.
375+
// Note: Prior to Swift 5.7, this method used to be inlinable, forwarding to
376+
// `_slice.base.distance(from:to:)`. Unfortunately, that approach isn't
377+
// compatible with SE-0180, as it allows Unicode scalars outside the
378+
// substring to affect grapheme breaking results within the substring. This
379+
// leads to Collection conformance issues when the `Substring`'s bounds do
380+
// not fall on grapheme boundaries in `base`.
381381

382382
// FIXME: Due to the `index(after:)` problem above, this function doesn't
383383
// always return consistent results when the given indices fall between
@@ -400,8 +400,7 @@ extension Substring: StringProtocol {
400400
count += 1
401401
i = _uncheckedIndex(after: i)
402402
}
403-
}
404-
else if i > end {
403+
} else if i > end {
405404
while i > end { // Note `<` instead of `==`
406405
count -= 1
407406
i = _uncheckedIndex(before: i)

0 commit comments

Comments
 (0)