Merge pull request #38509 from glessard/sr14850

[stdlib] prevent MutableCollections from inappropriately inheriting a Slice<Self> subscript

Author: glessard

stdlib/public/core/MutableCollection.swift

@@ -238,6 +238,7 @@ extension MutableCollection {
   ///   the range must be valid indices of the collection.
   ///
   /// - Complexity: O(1)
+  @available(*, unavailable)
   @inlinable
   public subscript(bounds: Range<Index>) -> Slice<Self> {
     get {
@@ -249,6 +250,19 @@ extension MutableCollection {
     }
   }
 
+  // This unavailable default implementation of `subscript(bounds: Range<_>)`
+  // prevents incomplete MutableCollection implementations from satisfying the
+  // protocol through the use of the generic convenience implementation
+  // `subscript<R: RangeExpression>(r: R)`. If that were the case, at
+  // runtime the generic implementation would call itself
+  // in an infinite recursion due to the absence of a better option.
+  @available(*, unavailable)
+  @_alwaysEmitIntoClient
+  public subscript(bounds: Range<Index>) -> SubSequence {
+    get { fatalError() }
+    set { fatalError() }
+  }
+
   /// Exchanges the values at the specified indices of the collection.
   ///
   /// Both parameters must be valid indices of the collection that are not
@@ -269,6 +283,45 @@ extension MutableCollection {
   }
 }
 
+extension MutableCollection where SubSequence == Slice<Self> {
+
+  /// Accesses a contiguous subrange of the collection's elements.
+  ///
+  /// The accessed slice uses the same indices for the same elements as the
+  /// original collection. Always use the slice's `startIndex` property
+  /// instead of assuming that its indices start at a particular value.
+  ///
+  /// This example demonstrates getting a slice of an array of strings, finding
+  /// the index of one of the strings in the slice, and then using that index
+  /// in the original array.
+  ///
+  ///     var streets = ["Adams", "Bryant", "Channing", "Douglas", "Evarts"]
+  ///     let streetsSlice = streets[2 ..< streets.endIndex]
+  ///     print(streetsSlice)
+  ///     // Prints "["Channing", "Douglas", "Evarts"]"
+  ///
+  ///     let index = streetsSlice.firstIndex(of: "Evarts")    // 4
+  ///     streets[index!] = "Eustace"
+  ///     print(streets[index!])
+  ///     // Prints "Eustace"
+  ///
+  /// - Parameter bounds: A range of the collection's indices. The bounds of
+  ///   the range must be valid indices of the collection.
+  ///
+  /// - Complexity: O(1)
+  @inlinable
+  @_alwaysEmitIntoClient
+  public subscript(bounds: Range<Index>) -> Slice<Self> {
+    get {
+      _failEarlyRangeCheck(bounds, bounds: startIndex..<endIndex)
+      return Slice(base: self, bounds: bounds)
+    }
+    set {
+      _writeBackMutableSlice(&self, bounds: bounds, slice: newValue)
+    }
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // _rotate(in:shiftingToStart:)
 //===----------------------------------------------------------------------===//

stdlib/public/core/SmallString.swift

@@ -191,11 +191,17 @@ extension _SmallString: RandomAccessCollection, MutableCollection {
 
   @inlinable  @inline(__always)
   internal subscript(_ bounds: Range<Index>) -> SubSequence {
-    // TODO(String performance): In-vector-register operation
-    return self.withUTF8 { utf8 in
-      let rebased = UnsafeBufferPointer(rebasing: utf8[bounds])
-      return _SmallString(rebased)._unsafelyUnwrappedUnchecked
+    get {
+      // TODO(String performance): In-vector-register operation
+      return self.withUTF8 { utf8 in
+        let rebased = UnsafeBufferPointer(rebasing: utf8[bounds])
+        return _SmallString(rebased)._unsafelyUnwrappedUnchecked
+      }
     }
+    // This setter is required for _SmallString to be a valid MutableCollection.
+    // Since _SmallString is internal and this setter unused, we cheat.
+    @_alwaysEmitIntoClient set { fatalError() }
+    @_alwaysEmitIntoClient _modify { fatalError() }
   }
 }
 

test/Constraints/members.swift

@@ -622,7 +622,6 @@ func rdar_50467583_and_50909555() {
     // expected-note@-2 {{found candidate with type '(Int) -> Int'}}
     // expected-note@-3 {{found candidate with type '(Range<Int>) -> ArraySlice<Int>'}}
     // expected-note@-4 {{found candidate with type '((UnboundedRange_) -> ()) -> ArraySlice<Int>'}}
-    // expected-note@-5 {{found candidate with type '(Range<Array<Int>.Index>) -> Slice<[Int]>' (aka '(Range<Int>) -> Slice<Array<Int>>')}}
   }
 
   // rdar://problem/50909555

test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected

@@ -1,4 +1,7 @@
+Accessor MutableCollection.subscript(_:).Get() has generic signature change from <Self where Self : Swift.MutableCollection> to <Self where Self : Swift.MutableCollection, Self.SubSequence == Swift.Slice<Self>>
+Accessor MutableCollection.subscript(_:).Set() has generic signature change from <Self where Self : Swift.MutableCollection> to <Self where Self : Swift.MutableCollection, Self.SubSequence == Swift.Slice<Self>>
 Protocol CodingKey has added inherited protocol Sendable
 Protocol CodingKey has generic signature change from <Self : Swift.CustomDebugStringConvertible, Self : Swift.CustomStringConvertible> to <Self : Swift.CustomDebugStringConvertible, Self : Swift.CustomStringConvertible, Self : Swift.Sendable>
 Protocol Error has added inherited protocol Sendable
 Protocol Error has generic signature change from to <Self : Swift.Sendable>
+Subscript MutableCollection.subscript(_:) has generic signature change from <Self where Self : Swift.MutableCollection> to <Self where Self : Swift.MutableCollection, Self.SubSequence == Swift.Slice<Self>>

validation-test/stdlib/CollectionDiagnostics.swift

@@ -154,6 +154,52 @@ struct CollectionWithNonDefaultSubSequence: Collection {
   public subscript(position: Int) -> Int { position }
 }
 
+// expected-error@+2 {{type 'MutableCollectionWithNonDefaultSubSequence' does not conform to protocol 'MutableCollection'}}
+// expected-error@+1 {{unavailable subscript 'subscript(_:)' was used to satisfy a requirement of protocol 'MutableCollection'}}
+struct MutableCollectionWithNonDefaultSubSequence: MutableCollection {
+  public var startIndex: Int
+  public var endIndex: Int
+
+  public typealias SubSequence = Self
+
+  public func index(after i: Int) -> Int { i+1 }
+  public subscript(position: Int) -> Int {
+    get { position }
+    set { _ = newValue }
+  }
+
+  public subscript(bounds: Range<Index>) -> Self {
+    Self(startIndex: bounds.startIndex, endIndex: bounds.endIndex)
+  }
+}
+
+struct MutableCollectionWithDefaultSubSequence: MutableCollection {
+  public var startIndex: Int
+  public var endIndex: Int
+
+  public func index(after i: Int) -> Int { i+1 }
+  public subscript(position: Int) -> Int {
+    get { position }
+    set { _ = newValue }
+  }
+}
+
+func subscriptMutableCollectionIgnored() {
+  let cs: MutableCollectionWithNonDefaultSubSequence
+  cs = .init(startIndex: 0, endIndex: 10)
+
+  let badSlice: Slice<MutableCollectionWithNonDefaultSubSequence>
+  badSlice = cs[0..<2] // expected-error {{'subscript(_:)' is unavailable}}
+  cs[3..<5] = badSlice // expected-error {{'subscript(_:)' is unavailable}}
+
+  let ds: MutableCollectionWithDefaultSubSequence
+  ds = .init(startIndex: 0, endIndex: 10)
+
+  let goodSlice: Slice<MutableCollectionWithDefaultSubSequence>
+  goodSlice = ds[0..<2]
+  ds[3..<5] = goodSlice
+}
+
 // FIXME: Remove -verify-ignore-unknown.
 // <unknown>:0: error: unexpected note produced: possibly intended match
 // <unknown>:0: error: unexpected note produced: possibly intended match