[stdlib] String.UTF8View: Review/fix index validation

Also, in UTF-8 slices, forward collection methods to the base view
instead of `Slice`, to make behavior a bit easier to understand.

(There is no need to force readers to page in `Slice`
implementations _in addition to_ whatever the base view is doing.)

Author: lorentey

stdlib/public/core/StringGuts.swift

@@ -456,7 +456,9 @@ extension _StringGuts {
     _precondition(i >= start && i < end, "Substring index is out of bounds")
     return scalarAlign(i)
   }
+}
 
+extension _StringGuts {
   /// Validate `i` and adjust its position toward the start, returning the
   /// resulting index or trapping as appropriate. If this function returns, then
   /// the returned value
@@ -491,7 +493,47 @@ extension _StringGuts {
     _precondition(i >= start && i <= end, "Substring index is out of bounds")
     return scalarAlign(i)
   }
+}
+
+extension _StringGuts {
+  @_alwaysEmitIntoClient
+  internal func validateSubscalarRange(
+    _ range: Range<String.Index>
+  ) -> Range<String.Index> {
+    let upper = ensureMatchingEncoding(range.upperBound)
+    let lower = ensureMatchingEncoding(range.lowerBound)
+
+    // Note: if only `lower` was miscoded, then the range invariant `lower <=
+    // upper` may no longer hold after the above conversions, so we need to
+    // re-check it here.
+    _precondition(upper._encodedOffset <= count && lower <= upper,
+      "String index range is out of bounds")
+
+    return Range(_uncheckedBounds: (lower, upper))
+  }
+
+  @_alwaysEmitIntoClient
+  internal func validateSubscalarRange(
+    _ range: Range<String.Index>,
+    from start: String.Index,
+    to end: String.Index
+  ) -> Range<String.Index> {
+    _internalInvariant(start <= end && end <= endIndex)
+
+    let upper = ensureMatchingEncoding(range.upperBound)
+    let lower = ensureMatchingEncoding(range.lowerBound)
+
+    // Note: if only `lower` was miscoded, then the range invariant `lower <=
+    // upper` may no longer hold after the above conversions, so we need to
+    // re-check it here.
+    _precondition(upper <= end && lower >= start && lower <= upper,
+      "Substring index range is out of bounds")
+
+    return Range(_uncheckedBounds: (lower, upper))
+  }
+}
 
+extension _StringGuts {
   /// Validate `range` and adjust the position of its bounds, returning the
   /// resulting range or trapping as appropriate. If this function returns, then
   /// the bounds of the returned value

stdlib/public/core/StringUTF8View.swift

@@ -136,28 +136,37 @@ extension String.UTF8View: BidirectionalCollection {
   /// - Precondition: The next position is representable.
   @inlinable @inline(__always)
   public func index(after i: Index) -> Index {
+    let i = _guts.ensureMatchingEncoding(i)
     if _fastPath(_guts.isFastUTF8) {
+      // Note: deferred bounds check
       return i.strippingTranscoding.nextEncoded._knownUTF8
     }
-
+    _precondition(i._encodedOffset < _guts.count,
+      "String index is out of bounds")
     return _foreignIndex(after: i)
   }
 
   @inlinable @inline(__always)
   public func index(before i: Index) -> Index {
-    _precondition(!i.isZeroPosition)
+    let i = _guts.ensureMatchingEncoding(i)
+    _precondition(!i.isZeroPosition, "String index is out of bounds")
     if _fastPath(_guts.isFastUTF8) {
       return i.strippingTranscoding.priorEncoded._knownUTF8
     }
 
+    _precondition(i._encodedOffset <= _guts.count,
+      "String index is out of bounds")
     return _foreignIndex(before: i)
   }
 
   @inlinable @inline(__always)
   public func index(_ i: Index, offsetBy n: Int) -> Index {
+    let i = _guts.ensureMatchingEncoding(i)
     if _fastPath(_guts.isFastUTF8) {
-      _precondition(n + i._encodedOffset <= _guts.count)
-      return i.strippingTranscoding.encoded(offsetBy: n)
+      let offset = n + i._encodedOffset
+      _precondition(offset >= 0 && offset <= _guts.count,
+        "String index is out of bounds")
+      return Index(_encodedOffset: offset)._knownUTF8
     }
 
     return _foreignIndex(i, offsetBy: n)
@@ -167,6 +176,7 @@ extension String.UTF8View: BidirectionalCollection {
   public func index(
     _ i: Index, offsetBy n: Int, limitedBy limit: Index
   ) -> Index? {
+    let i = _guts.ensureMatchingEncoding(i)
     if _fastPath(_guts.isFastUTF8) {
       // Check the limit: ignore limit if it precedes `i` (in the correct
       // direction), otherwise must not be beyond limit (in the correct
@@ -179,6 +189,8 @@ extension String.UTF8View: BidirectionalCollection {
       } else {
         guard limitOffset > iOffset || result >= limitOffset else { return nil }
       }
+      _precondition(result >= 0 && result <= _guts.count,
+        "String index is out of bounds")
       return Index(_encodedOffset: result)
     }
 
@@ -187,9 +199,14 @@ extension String.UTF8View: BidirectionalCollection {
 
   @inlinable @inline(__always)
   public func distance(from i: Index, to j: Index) -> Int {
+    let i = _guts.ensureMatchingEncoding(i)
+    let j = _guts.ensureMatchingEncoding(j)
     if _fastPath(_guts.isFastUTF8) {
       return j._encodedOffset &- i._encodedOffset
     }
+    _precondition(
+      i._encodedOffset <= _guts.count && j._encodedOffset <= _guts.count,
+      "String index is out of bounds")
     return _foreignDistance(from: i, to: j)
   }
 
@@ -207,7 +224,14 @@ extension String.UTF8View: BidirectionalCollection {
   ///   must be less than the view's end index.
   @inlinable @inline(__always)
   public subscript(i: Index) -> UTF8.CodeUnit {
-    String(_guts)._boundsCheck(i)
+    let i = _guts.ensureMatchingEncoding(i)
+    _precondition(i._encodedOffset < _guts.count,
+      "String index is out of bounds")
+    return self[_unchecked: i]
+  }
+
+  @_alwaysEmitIntoClient @inline(__always)
+  internal subscript(_unchecked i: Index) -> UTF8.CodeUnit {
     if _fastPath(_guts.isFastUTF8) {
       return _guts.withFastUTF8 { utf8 in utf8[_unchecked: i._encodedOffset] }
     }
@@ -373,6 +397,7 @@ extension String.UTF8View {
   @inlinable
   @available(swift, introduced: 4)
   public subscript(r: Range<Index>) -> String.UTF8View.SubSequence {
+    let r = _guts.validateSubscalarRange(r)
     return Substring.UTF8View(self, _bounds: r)
   }
 }
@@ -422,6 +447,7 @@ extension String.UTF8View {
   @_effects(releasenone)
   internal func _foreignIndex(after idx: Index) -> Index {
     _internalInvariant(_guts.isForeign)
+    _internalInvariant(idx._encodedOffset < _guts.count)
 
     let idx = _utf8AlignForeignIndex(idx)
 
@@ -448,6 +474,7 @@ extension String.UTF8View {
   @_effects(releasenone)
   internal func _foreignIndex(before idx: Index) -> Index {
     _internalInvariant(_guts.isForeign)
+    _internalInvariant(idx._encodedOffset <= _guts.count)
 
     let idx = _utf8AlignForeignIndex(idx)
 

stdlib/public/core/Substring.swift

@@ -731,6 +731,12 @@ extension Substring {
         base: String(base._guts).utf8,
         bounds: _bounds)
     }
+
+    @_alwaysEmitIntoClient @inline(__always)
+    internal var _wholeGuts: _StringGuts { _slice._base._guts }
+
+    @_alwaysEmitIntoClient @inline(__always)
+    internal var _base: String.UTF8View { _slice._base }
   }
 }
 
@@ -740,48 +746,52 @@ extension Substring.UTF8View: BidirectionalCollection {
   public typealias Element = String.UTF8View.Element
   public typealias SubSequence = Substring.UTF8View
 
-  //
-  // Plumb slice operations through
-  //
   @inlinable
-  public var startIndex: Index { return _slice.startIndex }
+  public var startIndex: Index { _slice._startIndex }
 
   @inlinable
-  public var endIndex: Index { return _slice.endIndex }
+  public var endIndex: Index { _slice._endIndex }
 
   @inlinable
-  public subscript(index: Index) -> Element { return _slice[index] }
+  public subscript(index: Index) -> Element {
+    let index = _wholeGuts.ensureMatchingEncoding(index)
+    _precondition(index >= startIndex && index < endIndex,
+      "String index is out of bounds")
+    return _base[_unchecked: index]
+  }
 
   @inlinable
   public var indices: Indices { return _slice.indices }
 
   @inlinable
-  public func index(after i: Index) -> Index { return _slice.index(after: i) }
+  public func index(after i: Index) -> Index {
+    // Note: deferred bounds check
+    return _base.index(after: i)
+  }
 
   @inlinable
   public func formIndex(after i: inout Index) {
-    // TODO(lorentey): Review index validation
-    _slice.formIndex(after: &i)
+    // Note: deferred bounds check
+    _base.formIndex(after: &i)
   }
 
   @inlinable
   public func index(_ i: Index, offsetBy n: Int) -> Index {
-    // TODO(lorentey): Review index validation
-    return _slice.index(i, offsetBy: n)
+    // Note: deferred bounds check
+    return _base.index(i, offsetBy: n)
   }
 
   @inlinable
   public func index(
     _ i: Index, offsetBy n: Int, limitedBy limit: Index
   ) -> Index? {
-    // TODO(lorentey): Review index validation
-    return _slice.index(i, offsetBy: n, limitedBy: limit)
+    // Note: deferred bounds check
+    return _base.index(i, offsetBy: n, limitedBy: limit)
   }
 
   @inlinable
   public func distance(from start: Index, to end: Index) -> Int {
-    // TODO(lorentey): Review index validation
-    return _slice.distance(from: start, to: end)
+    return _base.distance(from: start, to: end)
   }
 
   @_alwaysEmitIntoClient
@@ -794,36 +804,36 @@ extension Substring.UTF8View: BidirectionalCollection {
 
   @inlinable
   public func _failEarlyRangeCheck(_ index: Index, bounds: Range<Index>) {
-    // TODO(lorentey): Review index validation
-    _slice._failEarlyRangeCheck(index, bounds: bounds)
+    // FIXME: This probably ought to ensure that all three indices have matching
+    // encodings.
+    _base._failEarlyRangeCheck(index, bounds: bounds)
   }
 
   @inlinable
   public func _failEarlyRangeCheck(
     _ range: Range<Index>, bounds: Range<Index>
   ) {
-    // TODO(lorentey): Review index validation
-    _slice._failEarlyRangeCheck(range, bounds: bounds)
+    // FIXME: This probably ought to ensure that all three indices have matching
+    // encodings.
+    _base._failEarlyRangeCheck(range, bounds: bounds)
   }
 
   @inlinable
   public func index(before i: Index) -> Index {
-    // TODO(lorentey): Review index validation
-    return _slice.index(before: i)
+    // Note: deferred bounds check
+    return _base.index(before: i)
   }
 
   @inlinable
   public func formIndex(before i: inout Index) {
-    // TODO(lorentey): Review index validation
-    _slice.formIndex(before: &i)
+    // Note: deferred bounds check
+    _base.formIndex(before: &i)
   }
 
   @inlinable
   public subscript(r: Range<Index>) -> Substring.UTF8View {
-    // TODO(lorentey): Review index validation
     // FIXME(strings): tests.
-    _precondition(r.lowerBound >= startIndex && r.upperBound <= endIndex,
-      "UTF8View index range out of bounds")
+    let r = _wholeGuts.validateSubscalarRange(r, from: startIndex, to: endIndex)
     return Substring.UTF8View(_slice.base, _bounds: r)
   }
 }