[6.0 required] Remove UncheckedSendable (#429)

This change removes the `UncheckedSendable` type and adopts
`nonisolated(unsafe)`.

This change generates a number of diagnostics when built with the Swift
5.10 toolchain due to swiftlang/swift#72070, so we
will need to hold this change until we drop 5.10 support.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Author: grynspan

Sources/Testing/CMakeLists.txt

@@ -70,7 +70,6 @@ add_library(Testing
   Support/JSON.swift
   Support/Locked.swift
   Support/SystemError.swift
-  Support/UncheckedSendable.swift
   Support/Versions.swift
   Test.ID.Selection.swift
   Test.ID.swift

Sources/Testing/EntryPoints/XCTestScaffold.swift

@@ -139,7 +139,7 @@ public enum XCTestScaffold: Sendable {
 #endif
 #endif
 #else
-    let testCase = UncheckedSendable(rawValue: testCase)
+    nonisolated(unsafe) let testCase = testCase
 #if SWT_TARGET_OS_APPLE
     let isProcessLaunchedByXcode = Environment.variable(named: "XCTestSessionIdentifier") != nil
 #endif
@@ -152,7 +152,7 @@ public enum XCTestScaffold: Sendable {
 
     // Specify the hosting XCTestCase instance. This value is currently only
     // used for exit tests.
-    let typeName = String(reflecting: type(of: testCase.rawValue as Any))
+    let typeName = String(reflecting: type(of: testCase as Any))
     let functionName = if let parenIndex = functionName.lastIndex(of: "(") {
       functionName[..<parenIndex]
     } else {
@@ -168,20 +168,20 @@ public enum XCTestScaffold: Sendable {
 #if SWT_TARGET_OS_APPLE
       if issue.isKnown {
         XCTExpectFailure {
-          testCase.rawValue.record(XCTIssue(issue, processLaunchedByXcode: isProcessLaunchedByXcode))
+          testCase.record(XCTIssue(issue, processLaunchedByXcode: isProcessLaunchedByXcode))
         }
       } else {
-        testCase.rawValue.record(XCTIssue(issue, processLaunchedByXcode: isProcessLaunchedByXcode))
+        testCase.record(XCTIssue(issue, processLaunchedByXcode: isProcessLaunchedByXcode))
       }
 #else
       // NOTE: XCTestCase.recordFailure(withDescription:inFile:atLine:expected:)
       // does not behave as it might appear. The `expected` argument determines
       // if the issue represents an assertion failure or a thrown error.
       if !issue.isKnown {
-        testCase.rawValue.recordFailure(withDescription: String(describing: issue),
-                                        inFile: issue.sourceLocation?._filePath ?? "<unknown>",
-                                        atLine: issue.sourceLocation?.line ?? 0,
-                                        expected: true)
+        testCase.recordFailure(withDescription: String(describing: issue),
+                               inFile: issue.sourceLocation?._filePath ?? "<unknown>",
+                               atLine: issue.sourceLocation?.line ?? 0,
+                               expected: true)
       }
 #endif
     }

Sources/Testing/ExitTests/WaitFor.swift

@@ -50,10 +50,10 @@ private let _childProcessContinuations = Locked<[pid_t: CheckedContinuation<Exit
 
 /// A condition variable used to suspend the waiter thread created by
 /// `_createWaitThread()` when there are no child processes to await.
-private let _waitThreadNoChildrenCondition = {
+private nonisolated(unsafe) let _waitThreadNoChildrenCondition = {
   let result = UnsafeMutablePointer<pthread_cond_t>.allocate(capacity: 1)
   _ = pthread_cond_init(result, nil)
-  return UncheckedSendable(rawValue: result)
+  return result
 }()
 
 /// The implementation of `_createWaitThread()`, run only once.
@@ -89,7 +89,7 @@ private let _createWaitThreadImpl: Void = {
       // outside the lock in case acquiring the lock perturbs it.
       _childProcessContinuations.withUnsafeUnderlyingLock { lock, childProcessContinuations in
         if childProcessContinuations.isEmpty {
-          _ = pthread_cond_wait(_waitThreadNoChildrenCondition.rawValue, lock)
+          _ = pthread_cond_wait(_waitThreadNoChildrenCondition, lock)
         }
       }
     }
@@ -171,7 +171,7 @@ func wait(for pid: pid_t) async throws -> ExitCondition {
       assert(oldContinuation == nil, "Unexpected continuation found for PID \(pid). Please file a bug report at https://github.com/apple/swift-testing/issues/new")
 
       // Wake up the waiter thread if it is waiting for more child processes.
-      _ = pthread_cond_signal(_waitThreadNoChildrenCondition.rawValue)
+      _ = pthread_cond_signal(_waitThreadNoChildrenCondition)
     }
   }
 #endif

Sources/Testing/Running/Configuration.TestFilter.swift

@@ -253,10 +253,10 @@ extension Configuration.TestFilter.Kind {
           throw SystemError(description: "Filtering by regular expression matching is unavailable")
         }
 
-        let regex = UncheckedSendable(rawValue: try Regex(pattern))
+        nonisolated(unsafe) let regex = try Regex(pattern)
         return .function({ test in
           let id = String(describing: test.id)
-          return id.contains(regex.rawValue)
+          return id.contains(regex)
         }, membership: membership)
       case let .combination(lhs, rhs, op):
         return try .combination(lhs.operation, rhs.operation, op)

Sources/Testing/Support/FileHandle.swift

@@ -27,7 +27,7 @@ internal import _TestingInternals
 /// This type is not part of the public interface of the testing library.
 struct FileHandle: ~Copyable, Sendable {
   /// The underlying C file handle.
-  private var _fileHandle: UncheckedSendable<SWT_FILEHandle>
+  private nonisolated(unsafe) var _fileHandle: SWT_FILEHandle
 
   /// Whether or not to close `_fileHandle` when this instance is deinitialized.
   private var _closeWhenDone: Bool
@@ -105,13 +105,13 @@ struct FileHandle: ~Copyable, Sendable {
   ///     instance is deinitialized. The caller is responsible for ensuring that
   ///     there are no other references to `fileHandle` when passing `true`.
   init(unsafeCFILEHandle fileHandle: SWT_FILEHandle, closeWhenDone: Bool) {
-    _fileHandle = UncheckedSendable(rawValue: fileHandle)
+    _fileHandle = fileHandle
     _closeWhenDone = closeWhenDone
   }
 
   deinit {
     if _closeWhenDone {
-      fclose(_fileHandle.rawValue)
+      fclose(_fileHandle)
     }
   }
 
@@ -127,7 +127,7 @@ struct FileHandle: ~Copyable, Sendable {
   /// Use this function when calling C I/O interfaces such as `fputs()` on the
   /// underlying C file handle.
   borrowing func withUnsafeCFILEHandle<R>(_ body: (SWT_FILEHandle) throws -> R) rethrows -> R {
-    try body(_fileHandle.rawValue)
+    try body(_fileHandle)
   }
 
   /// Call a function and pass the underlying POSIX file descriptor to it.

Sources/Testing/Support/Locked.swift

@@ -69,11 +69,11 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   }
 
   /// Storage for the underlying lock and wrapped value.
-  private var _storage: UncheckedSendable<ManagedBuffer<T, _Lock>>
+  private nonisolated(unsafe) var _storage: ManagedBuffer<T, _Lock>
 
   init(rawValue: T) {
-    let storage = _Storage.create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
-    storage.withUnsafeMutablePointerToElements { lock in
+    _storage = _Storage.create(minimumCapacity: 1, makingHeaderWith: { _ in rawValue })
+    _storage.withUnsafeMutablePointerToElements { lock in
 #if SWT_TARGET_OS_APPLE || os(Linux) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
       _ = pthread_mutex_init(lock, nil)
 #elseif os(Windows)
@@ -84,7 +84,6 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
 #warning("Platform-specific implementation missing: locking unavailable")
 #endif
     }
-    _storage = UncheckedSendable(rawValue: storage)
   }
 
   var rawValue: T {
@@ -104,7 +103,7 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   /// synchronous caller. Wherever possible, use actor isolation or other Swift
   /// concurrency tools.
   nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R {
-    try _storage.rawValue.withUnsafeMutablePointers { rawValue, lock in
+    try _storage.withUnsafeMutablePointers { rawValue, lock in
 #if SWT_TARGET_OS_APPLE || os(Linux) || (os(WASI) && compiler(>=6.1) && _runtime(_multithreaded))
       _ = pthread_mutex_lock(lock)
       defer {
@@ -148,7 +147,7 @@ struct Locked<T>: RawRepresentable, Sendable where T: Sendable {
   ///   effect is undefined.
   nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<pthread_mutex_t>, T) throws -> R) rethrows -> R {
     try withLock { value in
-      try _storage.rawValue.withUnsafeMutablePointerToElements { lock in
+      try _storage.withUnsafeMutablePointerToElements { lock in
         try body(lock, value)
       }
     }

Sources/Testing/Support/UncheckedSendable.swift

@@ -70,26 +70,32 @@ public struct Test: Sendable {
 
   /// An enumeration describing the evaluation state of a test's cases.
   ///
-  /// This use of `UncheckedSendable` and of `AnySequence` in this type's cases
-  /// is necessary because it is not currently possible to express
+  /// This use of `@unchecked Sendable` and of `AnySequence` in this type's
+  /// cases is necessary because it is not currently possible to express
   /// `Sequence<Test.Case> & Sendable` as an existential (`any`)
   /// ([96960993](rdar://96960993)). It is also not possible to have a value of
   /// an underlying generic sequence type without specifying its generic
   /// parameters.
-  fileprivate enum TestCasesState: Sendable {
+  fileprivate enum TestCasesState: @unchecked Sendable {
     /// The test's cases have not yet been evaluated.
     ///
     /// - Parameters:
     ///   - function: The function to call to evaluate the test's cases. The
     ///     result is a sequence of test cases.
     case unevaluated(_ function: @Sendable () async throws -> AnySequence<Test.Case>)
 
-    /// The test's cases have been evaluated, and either returned a sequence of
-    /// cases or failed by throwing an error.
+    /// The test's cases have been evaluated.
     ///
     /// - Parameters:
-    ///   - result: The result of having evaluated the test's cases.
-    case evaluated(_ result: Result<UncheckedSendable<AnySequence<Test.Case>>, any Error>)
+    ///   - testCases: The test's cases.
+    case evaluated(_ testCases: AnySequence<Test.Case>)
+
+    /// An error was thrown when the testing library attempted to evaluate the
+    /// test's cases.
+    ///
+    /// - Parameters:
+    ///   - error: The thrown error.
+    case failed(_ error: any Error)
   }
 
   /// The evaluation state of this test's cases, if any.
@@ -108,23 +114,18 @@ public struct Test: Sendable {
   /// test case is synthesized. For test suite types (as opposed to test
   /// functions), the value of this property is `nil`.
   var testCases: (some Sequence<Test.Case> & Sendable)? {
-    guard let testCasesState else {
-      return nil as AnySequence<Test.Case>?
-    }
-    guard case let .evaluated(result) = testCasesState else {
-      // Callers are expected to first attempt to evaluate a test's cases by
-      // calling `evaluateTestCases()`.
-      preconditionFailure("Attempting to access test cases before they have been evaluated.")
-    }
-    guard case let .success(testCases) = result else {
-      // Callers are never expected to access this property after evaluating a
-      // test's cases, if that evaluation threw an error, because if the test
-      // cannot be run. In this scenario, a `Runner.Plan` is expected to record
-      // issue for the test, rather than attempt to run it, and thus never
-      // access this property.
-      preconditionFailure("Attempting to access test cases after evaluating them failed.")
+    testCasesState.map { testCasesState in
+      guard case let .evaluated(testCases) = testCasesState else {
+        // Callers are expected to first attempt to evaluate a test's cases by
+        // calling `evaluateTestCases()`, and are never expected to access this
+        // property after evaluating a test's cases if that evaluation threw an
+        // error (because the test cannot be run.) If an error was thrown, a
+        // `Runner.Plan` is expected to record issue for the test, rather than
+        // attempt to run it, and thus never access this property.
+        preconditionFailure("Attempting to access test cases with invalid state. Please file a bug report at https://github.com/apple/swift-testing/issues/new and include this information: \(String(reflecting: testCasesState))")
+      }
+      return testCases
     }
-    return testCases.rawValue
   }
 
   /// Evaluate this test's cases if they have not been evaluated yet.
@@ -138,20 +139,14 @@ public struct Test: Sendable {
   ///
   /// - Throws: Any error caught while first evaluating the test arguments.
   mutating func evaluateTestCases() async throws {
-    guard let testCasesState else { return }
-
-    do {
-      switch testCasesState {
-      case let .unevaluated(function):
+    if case let .unevaluated(function) = testCasesState {
+      do {
         let sequence = try await function()
-        self.testCasesState = .evaluated(.success(UncheckedSendable(rawValue: sequence)))
-      case .evaluated:
-        // No-op: already evaluated
-        break
+        self.testCasesState = .evaluated(sequence)
+      } catch {
+        self.testCasesState = .failed(error)
+        throw error
       }
-    } catch {
-      self.testCasesState = .evaluated(.failure(error))
-      throw error
     }
   }
 
@@ -242,7 +237,7 @@ public struct Test: Sendable {
     self.sourceLocation = sourceLocation
     self.containingTypeInfo = containingTypeInfo
     self.xcTestCompatibleSelector = xcTestCompatibleSelector
-    self.testCasesState = .evaluated(.success(UncheckedSendable(rawValue: .init(testCases))))
+    self.testCasesState = .evaluated(.init(testCases))
     self.parameters = parameters
   }
 }
@@ -348,14 +343,12 @@ extension Test {
         _timeLimit = test.timeLimit.map(TimeValue.init)
       }
 
-      testCases = switch test.testCasesState {
-      case .unevaluated,
-           .evaluated(.failure):
-        []
-      case let .evaluated(.success(testCases)):
-        testCases.rawValue.map(Test.Case.Snapshot.init(snapshotting:))
-      case nil:
-        nil
+      testCases = test.testCasesState.map { testCasesState in
+        if case let .evaluated(testCases) = testCasesState {
+          testCases.map(Test.Case.Snapshot.init(snapshotting:))
+        } else {
+          []
+        }
       }
     }