StdlibRef: Sequence.withContiguousStorageIfAvailable(_:) abstract needs a rewrite (#40334) (#40572)

* First draft of incorporating material from #38891

* Apply suggestions from Alex's review

Co-authored-by: Alex Martini <[email protected]>

* Rephrase suggested by Alex.

* Remove redundancy re: "don't replace buffer".

* Apply changes from editorial review.

* Apply Sequence edits (a3a3ff1) to MutableCollect'n

* Remove errant space.

Co-authored-by: Guillaume Lessard <[email protected]>

Co-authored-by: Chris Adamson <[email protected]>
Co-authored-by: Alex Martini <[email protected]>
Co-authored-by: Guillaume Lessard <[email protected]>
(cherry picked from commit 59559ca)

Author: 4 people

stdlib/public/core/MutableCollection.swift

@@ -194,29 +194,46 @@ where SubSequence: MutableCollection
     _ body: (inout UnsafeMutableBufferPointer<Element>) throws -> R
   ) rethrows -> R?
 
-  /// Call `body(buffer)`, where `buffer` provides access to the contiguous
-  /// mutable storage of the entire collection. If no such storage exists, it is
-  /// first created. If the collection does not support an internal
-  /// representation in the form of contiguous mutable storage, `body` is not
-  /// called and `nil` is returned.
+  /// Executes a closure on the collection's contiguous storage.
+  ///
+  /// This method calls `body(buffer)`, where `buffer` provides access to the
+  /// contiguous mutable storage of the entire collection. If the contiguous
+  /// storage doesn't exist, the collection creates it. If the collection
+  /// doesn't support an internal representation in the form of contiguous
+  /// mutable storage, this method doesn't call `body` --- it immediately
+  /// returns `nil`.
   ///
   /// The optimizer can often eliminate bounds- and uniqueness-checking
   /// within an algorithm. When that fails, however, invoking the same
-  /// algorithm on `body`\ 's argument may let you trade safety for speed.
+  /// algorithm on the `buffer` argument may let you trade safety for speed.
+  ///
+  /// Always perform any necessary cleanup in the closure, because the
+  /// method makes no guarantees about the state of the collection if the
+  /// closure throws an error. Your changes to the collection may be absent
+  /// from the collection after throwing the error, because the closure could
+  /// receive a temporary copy rather than direct access to the collection's
+  /// storage.
+  ///
+  /// - Warning: Your `body` closure must not replace `buffer`. This leads
+  ///   to a crash in all implementations of this method within the standard
+  ///   library.
+  ///
+  /// Successive calls to this method may provide a different pointer on each
+  /// call. Don't store `buffer` outside of this method.
   ///
   /// A `Collection` that provides its own implementation of this method
   /// must provide contiguous storage to its elements in the same order
-  /// as they appear in the collection. This guarantees that contiguous
-  /// mutable storage to any of its subsequences can be generated by slicing
+  /// as they appear in the collection. This guarantees that it's possible to
+  /// generate contiguous mutable storage to any of its subsequences by slicing
   /// `buffer` with a range formed from the distances to the subsequence's
   /// `startIndex` and `endIndex`, respectively.
   ///
-  /// - Note: `buffer` must not be replaced by `body`.
-  ///
   /// - Parameters:
-  ///   - body: a closure to be executed using the elements of this collection.
-  ///   - buffer: a buffer to the mutable contiguous storage of this collection.
-  /// - Returns: the value returned by `body`, or `nil`.
+  ///   - body: A closure that receives an in-out
+  ///     `UnsafeMutableBufferPointer` to the collection's contiguous storage.
+  /// - Returns: The value returned from `body`, unless the collection doesn't
+  ///   support contiguous storage, in which case the method ignores `body` and
+  ///   returns `nil`.
   mutating func withContiguousMutableStorageIfAvailable<R>(
     _ body: (_ buffer: inout UnsafeMutableBufferPointer<Element>) throws -> R
   ) rethrows -> R?

stdlib/public/core/Sequence.swift

@@ -384,27 +384,34 @@ public protocol Sequence {
     initializing ptr: UnsafeMutableBufferPointer<Element>
   ) -> (Iterator,UnsafeMutableBufferPointer<Element>.Index)
 
-  /// Call `body(buffer)`, where `buffer` provides access to the contiguous
-  /// storage of the entire collection. If no such storage exists, it is
-  /// first created. If the collection does not support an internal
-  /// representation in the form of contiguous storage, `body` is not
-  /// called and `nil` is returned.
+  /// Executes a closure on the sequence’s contiguous storage.
+  ///
+  /// This method calls `body(buffer)`, where `buffer` is a pointer to the
+  /// collection’s contiguous storage. If the contiguous storage doesn't exist,
+  /// the collection creates it. If the collection doesn’t support an internal
+  /// representation in a form of contiguous storage, the method doesn’t call
+  /// `body` --- it immediately returns `nil`.
   ///
   /// The optimizer can often eliminate bounds- and uniqueness-checking
   /// within an algorithm. When that fails, however, invoking the same
-  /// algorithm on `body`\ 's argument may let you trade safety for speed.
+  /// algorithm on the `buffer` argument may let you trade safety for speed.
+  ///
+  /// Successive calls to this method may provide a different pointer on each
+  /// call. Don't store `buffer` outside of this method.
   ///
   /// A `Collection` that provides its own implementation of this method
   /// must provide contiguous storage to its elements in the same order
-  /// as they appear in the collection. This guarantees that contiguous
-  /// storage to any of its subsequences can be generated by slicing
+  /// as they appear in the collection. This guarantees that it's possible to
+  /// generate contiguous mutable storage to any of its subsequences by slicing
   /// `buffer` with a range formed from the distances to the subsequence's
   /// `startIndex` and `endIndex`, respectively.
   ///
   /// - Parameters:
-  ///   - body: a closure to be executed using the elements of this collection.
-  ///   - buffer: a buffer to the contiguous storage of this collection.
-  /// - Returns: the value returned by `body`, or `nil`.
+  ///   - body: A closure that receives an `UnsafeBufferPointer` to the
+  ///     sequence's contiguous storage.
+  /// - Returns: The value returned from `body`, unless the sequence doesn't
+  ///   support contiguous storage, in which case the method ignores `body` and
+  ///   returns `nil`.
   func withContiguousStorageIfAvailable<R>(
     _ body: (_ buffer: UnsafeBufferPointer<Element>) throws -> R
   ) rethrows -> R?