Locked shouldn't be a property wrapper. (#185)

* `Locked` shouldn't be a property wrapper.

Property wrappers, when used as static properties, are inherently concurrency-unsafe (because they must be `var`, not `let`.) Discussed with the core Swift folks; this PR removes property wrapper "conformance" from `Locked`, adds `RawRepresentable` to replace it, and modifies call sites as appropriate.

Resolves rdar://121054518.

Author: grynspan

Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift

@@ -61,7 +61,7 @@ extension Event {
 
     /// This event recorder's mutable context about events it has received,
     /// which may be used to inform how subsequent events are written.
-    @Locked private var _context = _Context()
+    private var _context = Locked(rawValue: _Context())
 
     /// Initialize a new human-readable event recorder.
     ///
@@ -186,7 +186,7 @@ extension Event.HumanReadableOutputRecorder {
 
     switch event.kind {
     case .runStarted:
-      $_context.withLock { context in
+      _context.withLock { context in
         context.runStartInstant = instant
       }
       var comments: [Comment] = [
@@ -213,7 +213,7 @@ extension Event.HumanReadableOutputRecorder {
 
     case .testStarted:
       let test = test!
-      $_context.withLock { context in
+      _context.withLock { context in
         context.testData[test.id.keyPathRepresentation] = .init(startInstant: instant)
         if test.isSuite {
           context.suiteCount += 1
@@ -231,7 +231,7 @@ extension Event.HumanReadableOutputRecorder {
     case .testEnded:
       let test = test!
       let id = test.id
-      let testDataGraph = _context.testData.subgraph(at: id.keyPathRepresentation)
+      let testDataGraph = _context.rawValue.testData.subgraph(at: id.keyPathRepresentation)
       let testData = testDataGraph?.value ?? .init(startInstant: instant)
       let issues = _issueCounts(in: testDataGraph)
       let duration = testData.startInstant.descriptionOfDuration(to: instant)
@@ -253,7 +253,7 @@ extension Event.HumanReadableOutputRecorder {
 
     case let .testSkipped(skipInfo):
       let test = test!
-      $_context.withLock { context in
+      _context.withLock { context in
         if test.isSuite {
           context.suiteCount += 1
         } else {
@@ -278,7 +278,7 @@ extension Event.HumanReadableOutputRecorder {
     case let .issueRecorded(issue):
       if let test {
         let id = test.id.keyPathRepresentation
-        $_context.withLock { context in
+        _context.withLock { context in
           var testData = context.testData[id] ?? .init(startInstant: instant)
           if issue.isKnown {
             testData.knownIssueCount += 1
@@ -344,7 +344,7 @@ extension Event.HumanReadableOutputRecorder {
       break
 
     case .runEnded:
-      let context = $_context.wrappedValue
+      let context = _context.rawValue
 
       let testCount = context.testCount
       let issues = _issueCounts(in: context.testData)

Sources/Testing/Events/Recorder/Event.JUnitXMLRecorder.swift

@@ -51,7 +51,7 @@ extension Event {
 
     /// This event recorder's mutable context about events it has received,
     /// which may be used to inform how subsequent events are written.
-    @Locked private var _context = _Context()
+    private var _context = Locked(rawValue: _Context())
 
     /// Initialize a new event recorder.
     ///
@@ -82,7 +82,7 @@ extension Event.JUnitXMLRecorder {
 
     switch event.kind {
     case .runStarted:
-      $_context.withLock { context in
+      _context.withLock { context in
         context.runStartInstant = instant
       }
       return #"""
@@ -93,14 +93,14 @@ extension Event.JUnitXMLRecorder {
     case .testStarted where false == test?.isSuite:
       let id = test!.id
       let keyPath = id.keyPathRepresentation
-      $_context.withLock { context in
+      _context.withLock { context in
         context.testData[keyPath] = _Context.TestData(id: id, startInstant: instant)
       }
       return nil
     case .testEnded where false == test?.isSuite:
       let id = test!.id
       let keyPath = id.keyPathRepresentation
-      $_context.withLock { context in
+      _context.withLock { context in
         context.testData[keyPath]?.endInstant = instant
       }
       return nil
@@ -114,12 +114,12 @@ extension Event.JUnitXMLRecorder {
         return nil // FIXME: handle issues without known tests
       }
       let keyPath = id.keyPathRepresentation
-      $_context.withLock { context in
+      _context.withLock { context in
         context.testData[keyPath]?.issues.append(issue)
       }
       return nil
     case .runEnded:
-      return $_context.withLock { context in
+      return _context.withLock { context in
         let issueCount = context.testData
           .compactMap(\.value?.issues.count)
           .reduce(into: 0, +=)

Sources/Testing/Issues/Confirmation.swift

@@ -14,8 +14,7 @@ public struct Confirmation: Sendable {
   ///
   /// This property is fileprivate because it may be mutated asynchronously and
   /// callers may be tempted to use it in ways that result in data races.
-  @Locked
-  fileprivate var count = 0
+  fileprivate var count = Locked(rawValue: 0)
 
   /// Confirm this confirmation.
   ///
@@ -26,7 +25,7 @@ public struct Confirmation: Sendable {
   /// directly.
   public func confirm(count: Int = 1) {
     precondition(count > 0)
-    $count.add(count)
+    self.count.add(count)
   }
 }
 
@@ -101,7 +100,7 @@ public func confirmation<R>(
 ) async rethrows -> R {
   let confirmation = Confirmation()
   defer {
-    let actualCount = confirmation.count
+    let actualCount = confirmation.count.rawValue
     if actualCount != expectedCount {
       Issue.record(
         .confirmationMiscounted(actual: actualCount, expected: expectedCount),

Sources/Testing/Issues/KnownIssue.swift

@@ -64,7 +64,7 @@ private func _matchError(_ error: any Error, using issueMatcher: KnownIssueMatch
 ///   - sourceLocation: The source location to which the issue should be
 ///     attributed.
 private func _handleMiscount(by matchCounter: Locked<Int>, comment: Comment?, sourceLocation: SourceLocation) {
-  if matchCounter.wrappedValue == 0 {
+  if matchCounter.rawValue == 0 {
     Issue.record(
       .knownIssueNotRecorded,
       comments: Array(comment),
@@ -182,12 +182,12 @@ public func withKnownIssue(
   guard precondition() else {
     return try body()
   }
-  @Locked var matchCounter = 0
+  let matchCounter = Locked(rawValue: 0)
   let sourceLocation = SourceLocation(fileID: fileID, filePath: filePath, line: line, column: column)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: $matchCounter)
+  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
   defer {
     if !isIntermittent {
-      _handleMiscount(by: $matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
   try Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
@@ -297,12 +297,12 @@ public func withKnownIssue(
   guard await precondition() else {
     return try await body()
   }
-  @Locked var matchCounter = 0
+  let matchCounter = Locked(rawValue: 0)
   let sourceLocation = SourceLocation(fileID: fileID, filePath: filePath, line: line, column: column)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: $matchCounter)
+  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
   defer {
     if !isIntermittent {
-      _handleMiscount(by: $matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
   try await Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {

Sources/Testing/Running/EntryPoint.swift

@@ -21,7 +21,7 @@ private import TestingInternals
 /// - Warning: This function is used by Swift Package Manager. Do not call it
 ///   directly.
 @_disfavoredOverload public func __swiftPMEntryPoint() async -> CInt {
-  @Locked var exitCode = EXIT_SUCCESS
+  let exitCode = Locked(rawValue: EXIT_SUCCESS)
 
   do {
     let args = CommandLine.arguments()
@@ -34,7 +34,7 @@ private import TestingInternals
       let oldEventHandler = configuration.eventHandler
       configuration.eventHandler = { event, context in
         if case let .issueRecorded(issue) = event.kind, !issue.isKnown {
-          $exitCode.withLock { exitCode in
+          exitCode.withLock { exitCode in
             exitCode = EXIT_FAILURE
           }
         }
@@ -48,12 +48,12 @@ private import TestingInternals
     fputs(String(describing: error), stderr)
     fflush(stderr)
 
-    $exitCode.withLock { exitCode in
+    exitCode.withLock { exitCode in
       exitCode = EXIT_FAILURE
     }
   }
 
-  return exitCode
+  return exitCode.rawValue
 }
 
 /// The entry point to the testing library used by Swift Package Manager.

Sources/Testing/Running/Runner.RuntimeState.swift

@@ -27,7 +27,7 @@ extension Runner {
 
     /// The runtime state related to the runner running on the current task.
     @TaskLocal
-    static var current: Self = .init()
+    static var current = Self()
   }
 }
 
@@ -91,24 +91,23 @@ extension Configuration {
   }
 
   /// Mutable storage for ``Configuration/all``.
-  @Locked
-  private static var _all = _All()
+  private static let _all = Locked(rawValue: _All())
 
   /// A collection containing all instances of this type that are currently set
   /// as the current configuration for a task.
   ///
   /// This property is used when an event is posted in a context where the value
   /// of ``Configuration/current`` is `nil`, such as from a detached task.
   static var all: some Collection<Self> {
-    _all.instances.values
+    _all.rawValue.instances.values
   }
 
   /// Add this instance to ``Configuration/all``.
   ///
   /// - Returns: A unique number identifying `self` that can be
   ///   passed to `_removeFromAll(identifiedBy:)`` to unregister it.
   private func _addToAll() -> UInt64 {
-    Self.$_all.withLock { all in
+    Self._all.withLock { all in
       let id = all.nextID
       all.nextID += 1
       all.instances[id] = self
@@ -123,7 +122,7 @@ extension Configuration {
   ///     `_addToAll()`. If `nil`, this function has no effect.
   private func _removeFromAll(identifiedBy id: UInt64?) {
     if let id {
-      Self.$_all.withLock { all in
+      Self._all.withLock { all in
         _ = all.instances.removeValue(forKey: id)
       }
     }

Sources/Testing/SourceAttribution/Backtrace.swift

@@ -143,12 +143,10 @@ extension Backtrace {
   /// same location.)
   ///
   /// Access to this dictionary is guarded by a lock.
-  @Locked
-  private static var _errorMappingCache = [ObjectIdentifier: _ErrorMappingCacheEntry]()
+  private static let _errorMappingCache = Locked<[ObjectIdentifier: _ErrorMappingCacheEntry]>()
 
   /// The previous `swift_willThrow` handler, if any.
-  @Locked
-  private static var _oldWillThrowHandler: SWTWillThrowHandler?
+  private static let _oldWillThrowHandler = Locked<SWTWillThrowHandler?>()
 
   /// Handle a thrown error.
   ///
@@ -157,14 +155,14 @@ extension Backtrace {
   ///     refers to an instance of `SwiftError` or (on platforms with
   ///     Objective-C interop) an instance of `NSError`.
   @Sendable private static func _willThrow(_ errorAddress: UnsafeMutableRawPointer) {
-    _oldWillThrowHandler?(errorAddress)
+    _oldWillThrowHandler.rawValue?(errorAddress)
 
     let errorObject = unsafeBitCast(errorAddress, to: (any AnyObject & Sendable).self)
     let errorID = ObjectIdentifier(errorObject)
     let backtrace = Backtrace.current()
     let newEntry = _ErrorMappingCacheEntry(errorObject: errorObject, backtrace: backtrace)
 
-    Self.$_errorMappingCache.withLock { cache in
+    _errorMappingCache.withLock { cache in
       let oldEntry = cache[errorID]
       if oldEntry?.errorObject == nil {
         // Either no entry yet, or its weak reference was zeroed.
@@ -176,7 +174,7 @@ extension Backtrace {
   /// The implementation of ``Backtrace/startCachingForThrownErrors()``, run
   /// only once.
   private static let _startCachingForThrownErrors: Void = {
-    $_oldWillThrowHandler.withLock { oldWillThrowHandler in
+    _oldWillThrowHandler.withLock { oldWillThrowHandler in
       oldWillThrowHandler = swt_setWillThrowHandler { _willThrow($0) }
     }
   }()
@@ -196,7 +194,7 @@ extension Backtrace {
   /// Call this function periodically to ensure that errors do not continue to
   /// take up space in the cache after they have been deinitialized.
   static func flushThrownErrorCache() {
-    Self.$_errorMappingCache.withLock { cache in
+    _errorMappingCache.withLock { cache in
       cache = cache.filter { $0.value.errorObject != nil }
     }
   }
@@ -218,7 +216,7 @@ extension Backtrace {
   @inline(never)
   init?(forFirstThrowOf error: any Error) {
     let errorID = ObjectIdentifier(unsafeBitCast(error, to: AnyObject.self))
-    let entry = Self.$_errorMappingCache.withLock { cache in
+    let entry = Self._errorMappingCache.withLock { cache in
       cache[errorID]
     }
     if let entry, entry.errorObject != nil {

Sources/Testing/Support/Environment.swift

@@ -21,8 +21,7 @@ enum Environment {
   ///
   /// The mechanism by which this dictionary is initially populated depends on
   /// platform-specific implementation details.
-  @Locked
-  private static var _environment = [String: String]()
+  private static let _environment = Locked<[String: String]>()
 #endif
 
   /// Get the environment variable with the specified name.
@@ -34,7 +33,7 @@ enum Environment {
   ///   is not set for the current process.
   static func variable(named name: String) -> String? {
 #if SWT_NO_ENVIRONMENT_VARIABLES
-    _environment[name]
+    _environment.rawValue[name]
 #elseif SWT_TARGET_OS_APPLE || os(Linux)
     getenv(name).flatMap { String(validatingUTF8: $0) }
 #elseif os(Windows)