Merge pull request #40828 from glessard/sort-optimization

[nfc] document legacy ABI in the implementation of sort

Author: glessard

stdlib/public/core/Sort.swift

@@ -329,9 +329,13 @@ extension MutableCollection where Self: BidirectionalCollection {
   }
 }
 
+// FIXME(ABI): unused return value
 /// Merges the elements in the ranges `lo..<mid` and `mid..<hi` using `buffer`
 /// as out-of-place storage. Stable.
 ///
+/// The unused return value is legacy ABI. It was originally added as a
+/// workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+///
 /// - Precondition: `lo..<mid` and `mid..<hi` must already be sorted according
 ///   to `areInIncreasingOrder`.
 /// - Precondition: `buffer` must point to a region of memory at least as large
@@ -506,15 +510,19 @@ internal func _findNextRun<C: RandomAccessCollection>(
 }
 
 extension UnsafeMutableBufferPointer {
+  // FIXME(ABI): unused return value
   /// Merges the elements at `runs[i]` and `runs[i - 1]`, using `buffer` as
   /// out-of-place storage.
   ///
+  /// The unused return value is legacy ABI. It was originally added as a
+  /// workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  ///
   /// - Precondition: `runs.count > 1` and `i > 0`
   /// - Precondition: `buffer` must have at least
   ///   `min(runs[i].count, runs[i - 1].count)` uninitialized elements.
   @discardableResult
   @inlinable
-  public mutating func _mergeRuns(
+  internal mutating func _mergeRuns(
     _ runs: inout [Range<Index>],
     at i: Int,
     buffer: UnsafeMutablePointer<Element>,
@@ -537,17 +545,21 @@ extension UnsafeMutableBufferPointer {
 
     return true
   }
-  
+
+  // FIXME(ABI): unused return value
   /// Merges upper elements of `runs` until the required invariants are
   /// satisfied.
   ///
+  /// The unused return value is legacy ABI. It was originally added as a
+  /// workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  ///
   /// - Precondition: `buffer` must have at least
   ///   `min(runs[i].count, runs[i - 1].count)` uninitialized elements.
   /// - Precondition: The ranges in `runs` must be consecutive, such that for
   ///   any i, `runs[i].upperBound == runs[i + 1].lowerBound`.
   @discardableResult
   @inlinable
-  public mutating func _mergeTopRuns(
+  internal mutating func _mergeTopRuns(
     _ runs: inout [Range<Index>],
     buffer: UnsafeMutablePointer<Element>,
     by areInIncreasingOrder: (Element, Element) throws -> Bool
@@ -611,16 +623,20 @@ extension UnsafeMutableBufferPointer {
 
     return true
   }
-  
+
+  // FIXME(ABI): unused return value
   /// Merges elements of `runs` until only one run remains.
   ///
+  /// The unused return value is legacy ABI. It was originally added as a
+  /// workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  ///
   /// - Precondition: `buffer` must have at least
   ///   `min(runs[i].count, runs[i - 1].count)` uninitialized elements.
   /// - Precondition: The ranges in `runs` must be consecutive, such that for
   ///   any i, `runs[i].upperBound == runs[i + 1].lowerBound`.
   @discardableResult
   @inlinable
-  public mutating func _finalizeRuns(
+  internal mutating func _finalizeRuns(
     _ runs: inout [Range<Index>],
     buffer: UnsafeMutablePointer<Element>,
     by areInIncreasingOrder: (Element, Element) throws -> Bool

validation-test/stdlib/Sort-ABI.swift

@@ -0,0 +1,76 @@
+// RUN: %target-run-stdlib-swift
+// REQUIRES: executable_test
+
+import StdlibUnittest
+
+var SortABITestSuite =  TestSuite("SortABI")
+
+SortABITestSuite.test("_merge-ABI") {
+  let result = withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+    buffer -> Bool in
+    _ = buffer.initialize(from: 0..<10)
+    return withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+      _merge(
+        low: buffer.baseAddress! + 5,
+        mid: buffer.baseAddress! + 8,
+        high: buffer.baseAddress! + 9,
+        buffer: $0.baseAddress!,
+        by: <
+      )
+    }
+  }
+  // The return value is legacy ABI. It was originally added as a
+  // workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  // `_merge` always returns `true`
+  expectEqual(result, true)
+}
+
+SortABITestSuite.test("_mergeRuns-ABI") {
+  let result = withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+    temp -> Bool in
+    _ = temp.initialize(from: 0..<10)
+    var buffer = temp
+    var runs = [5..<8, 8..<9]
+    return withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+      buffer._mergeRuns(&runs, at: 1, buffer: $0.baseAddress!, by: <)
+    }
+  }
+  // The return value is legacy ABI. It was originally added as a
+  // workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  // `_mergeRuns` always returns `true`
+  expectEqual(result, true)
+}
+
+SortABITestSuite.test("_mergeTopRuns-ABI") {
+  let result = withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+    temp -> Bool in
+    _ = temp.initialize(from: 0..<10)
+    var buffer = temp
+    var runs = [5..<8, 8..<9]
+    return withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+      buffer._mergeTopRuns(&runs, buffer: $0.baseAddress!, by: <)
+    }
+  }
+  // The return value is legacy ABI. It was originally added as a
+  // workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  // `_mergeTopRuns` always returns `true`
+  expectEqual(result, true)
+}
+
+SortABITestSuite.test("_finalizeRuns-ABI") {
+  let result = withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+    temp -> Bool in
+    _ = temp.initialize(from: 0..<10)
+    var buffer = temp
+    var runs = [5..<8, 8..<9]
+    return withUnsafeTemporaryAllocation(of: Int.self, capacity: 10) {
+      buffer._finalizeRuns(&runs, buffer: $0.baseAddress!, by: <)
+    }
+  }
+  // The return value is legacy ABI. It was originally added as a
+  // workaround for a compiler bug (now fixed). See SR-14750 (rdar://45044610).
+  // `_finalizeRuns` always returns `true`
+  expectEqual(result, true)
+}
+
+runAllTests()