Merge pull request #1447 from ahoppen/atomics-not-unsafe

Change all variables that are `Atomic*` types to not be `nonisolated(unsafe)`

Author: ahoppen

Sources/InProcessClient/InProcessSourceKitLSPClient.swift

@@ -19,8 +19,7 @@ import SourceKitLSP
 public final class InProcessSourceKitLSPClient: Sendable {
   private let server: SourceKitLSPServer
 
-  /// `nonisolated(unsafe)` if fine because `nextRequestID` is atomic.
-  private nonisolated(unsafe) var nextRequestID = AtomicUInt32(initialValue: 0)
+  private let nextRequestID = AtomicUInt32(initialValue: 0)
 
   /// Create a new `SourceKitLSPServer`. An `InitializeRequest` is automatically sent to the server.
   ///

Sources/SKCore/TaskScheduler.swift

@@ -126,7 +126,7 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
   /// Every time `execute` gets called, a new task is placed in this continuation. See comment on `executionTask`.
   private let executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation
 
-  nonisolated(unsafe) private var _priority: AtomicUInt8
+  private let _priority: AtomicUInt8
 
   /// The latest known priority of the task.
   ///
@@ -147,9 +147,9 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
   private var cancelledToBeRescheduled: Bool = false
 
   /// Whether `resultTask` has been cancelled.
-  private nonisolated(unsafe) var resultTaskCancelled: AtomicBool = .init(initialValue: false)
+  private let resultTaskCancelled: AtomicBool = .init(initialValue: false)
 
-  private nonisolated(unsafe) var _isExecuting: AtomicBool = .init(initialValue: false)
+  private let _isExecuting: AtomicBool = .init(initialValue: false)
 
   /// Whether the task is currently executing or still queued to be executed later.
   public nonisolated var isExecuting: Bool {

Sources/SKSupport/Atomics.swift

@@ -16,8 +16,8 @@ import CAtomics
 #warning("We should be able to use atomics in the stdlib when we raise the deployment target to require Swift 6")
 #endif
 
-public class AtomicBool {
-  private let atomic: UnsafeMutablePointer<CAtomicUInt32>
+public final class AtomicBool: Sendable {
+  private nonisolated(unsafe) let atomic: UnsafeMutablePointer<CAtomicUInt32>
 
   public init(initialValue: Bool) {
     self.atomic = atomic_uint32_create(initialValue ? 1 : 0)
@@ -37,8 +37,8 @@ public class AtomicBool {
   }
 }
 
-public class AtomicUInt8 {
-  private let atomic: UnsafeMutablePointer<CAtomicUInt32>
+public final class AtomicUInt8: Sendable {
+  private nonisolated(unsafe) let atomic: UnsafeMutablePointer<CAtomicUInt32>
 
   public init(initialValue: UInt8) {
     self.atomic = atomic_uint32_create(UInt32(initialValue))
@@ -58,8 +58,8 @@ public class AtomicUInt8 {
   }
 }
 
-public class AtomicUInt32 {
-  private let atomic: UnsafeMutablePointer<CAtomicUInt32>
+public final class AtomicUInt32: Sendable {
+  private nonisolated(unsafe) let atomic: UnsafeMutablePointer<CAtomicUInt32>
 
   public init(initialValue: UInt32) {
     self.atomic = atomic_uint32_create(initialValue)

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -87,8 +87,7 @@ fileprivate extension ConfiguredTarget {
   static let forPackageManifest = ConfiguredTarget(targetID: "", runDestinationID: "")
 }
 
-/// `nonisolated(unsafe)` is fine because `preparationTaskID` is atomic.
-fileprivate nonisolated(unsafe) var preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)
+fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0)
 
 /// Swift Package Manager build system and workspace support.
 ///

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -41,8 +41,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
   public typealias RequestHandler<Request: RequestType> = @Sendable (Request) -> Request.Response
 
   /// The ID that should be assigned to the next request sent to the `server`.
-  /// `nonisolated(unsafe)` is fine because `nextRequestID` is atomic.
-  private nonisolated(unsafe) var nextRequestID = AtomicUInt32(initialValue: 0)
+  private let nextRequestID = AtomicUInt32(initialValue: 0)
 
   /// The server that handles the requests.
   public let server: SourceKitLSPServer
@@ -70,7 +69,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
   ///
   /// `isOneShort` if the request handler should only serve a single request and should be removed from
   /// `requestHandlers` after it has been called.
-  private nonisolated(unsafe) var requestHandlers: ThreadSafeBox<[(requestHandler: Sendable, isOneShot: Bool)]> =
+  private let requestHandlers: ThreadSafeBox<[(requestHandler: Sendable, isOneShot: Bool)]> =
     ThreadSafeBox(initialValue: [])
 
   /// A closure that is called when the `TestSourceKitLSPClient` is destructed.

Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift

@@ -20,7 +20,7 @@ import SwiftExtensions
 import struct TSCBasic.AbsolutePath
 import class TSCBasic.Process
 
-private nonisolated(unsafe) var updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1)
+private let updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1)
 
 public enum FileToIndex: CustomLogStringConvertible {
   /// A non-header file

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -40,8 +40,7 @@ fileprivate final class RequestAndReply<Params: RequestType>: Sendable {
   private let replyBlock: @Sendable (LSPResult<Params.Response>) -> Void
 
   /// Whether a reply has been made. Every request must reply exactly once.
-  /// `nonisolated(unsafe)` is fine because `replied` is atomic.
-  private nonisolated(unsafe) var replied: AtomicBool = AtomicBool(initialValue: false)
+  private let replied: AtomicBool = AtomicBool(initialValue: false)
 
   public init(_ request: Params, reply: @escaping @Sendable (LSPResult<Params.Response>) -> Void) {
     self.params = request
@@ -539,8 +538,7 @@ public actor SourceKitLSPServer {
 
 // MARK: - MessageHandler
 
-// nonisolated(unsafe) is fine because `notificationIDForLogging` is atomic.
-private nonisolated(unsafe) var notificationIDForLogging = AtomicUInt32(initialValue: 1)
+private let notificationIDForLogging = AtomicUInt32(initialValue: 1)
 
 extension SourceKitLSPServer: MessageHandler {
   public nonisolated func handle(_ params: some NotificationType) {

Tests/SourceKitDTests/SourceKitDRegistryTests.swift

@@ -59,7 +59,7 @@ final class SourceKitDRegistryTests: XCTestCase {
   }
 }
 
-private nonisolated(unsafe) var nextToken = AtomicUInt32(initialValue: 0)
+private let nextToken = AtomicUInt32(initialValue: 0)
 
 final class FakeSourceKitD: SourceKitD {
   let token: UInt32

Tests/SourceKitLSPTests/PullDiagnosticsTests.swift

@@ -259,7 +259,7 @@ final class PullDiagnosticsTests: XCTestCase {
   }
 
   func testDiagnosticsWaitForDocumentToBePrepared() async throws {
-    nonisolated(unsafe) var diagnosticRequestSent = AtomicBool(initialValue: false)
+    let diagnosticRequestSent = AtomicBool(initialValue: false)
     var serverOptions = SourceKitLSPServer.Options.testDefault
     serverOptions.indexTestHooks.preparationTaskDidStart = { @Sendable taskDescription in
       // Only start preparation after we sent the diagnostic request. In almost all cases, this should not give