Address @Shared sendability. #3329

mbrandonw · 2024-08-30T15:29:45Z

No description provided.

mbrandonw · 2024-08-30T15:30:24Z

Package.swift

@@ -87,7 +87,7 @@ let package = Package(
 )

 #if compiler(>=6)
-  for target in package.targets where target.type != .system {
+  for target in package.targets where target.type != .system && target.type != .test {


I added this temporarily so that we don't get flooded with warnings in test targets, but maybe we should keep this around until we've addressed all/most warnings in TCA proper, and then we can later concentrate on tests?

mbrandonw · 2024-08-30T15:32:37Z

Sources/ComposableArchitecture/SharedState/PersistenceKey.swift

@@ -7,9 +7,9 @@
 ///
 /// See the article <doc:SharingState> for more information, in particular the
 /// <doc:SharingState#Custom-persistence> section.
-public protocol PersistenceReaderKey<Value> {
+public protocol PersistenceReaderKey<Value>: Sendable {


Persistence keys need to be sendable because they are captured in some LockIsolated's, which currently require Sendable values and @Sendable transaction closures. If we weaken that, then perhaps this can be weakened too?

mbrandonw · 2024-08-30T15:34:33Z

Sources/ComposableArchitecture/SharedState/PersistenceKey.swift

  /// A type that can be loaded or subscribed to in an external system.
-  associatedtype Value
+  associatedtype Value: Sendable


Value is captured in a variety of sendable contexts, such as in the subscription endpoint for persistence strategies. So I think this does really need to be full blown Sendable.

mbrandonw · 2024-08-30T15:35:20Z

Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift

@@ -154,7 +154,7 @@ extension PersistenceReaderKey {
 /// A type defining a user defaults persistence strategy.
 ///
 /// See ``PersistenceReaderKey/appStorage(_:)-4l5b`` to create values of this type.
-public struct AppStorageKey<Value> {
+public struct AppStorageKey<Value: Sendable>: Sendable {
  private let lookup: any Lookup<Value>
  private let key: String
  private let store: UserDefaults


Technically UserDefaults is not Sendable even though the docs specifically say "User defaults is thread-safe". So, it should be ok to make this UncheckedSendable, but it was strangely causing some test failures. So I need to look at this again.

mbrandonw · 2024-08-30T15:37:58Z

Sources/ComposableArchitecture/SharedState/References/ValueReference.swift

-  private var subscription: Shared<Value>.Subscription?
+  private var subscription: Shared<Value>.Subscription!


we'd prefer for this to be a let so that we don't have to do @unchecked Sendable above, but we need lazy initialization. And to make that clear I went ahead and made it an implicitly unwrapped optional.

mbrandonw · 2024-08-30T15:45:36Z

Sources/ComposableArchitecture/SharedState/Shared.swift

+public struct Shared<Value: Sendable> {
  private let reference: any Reference
  private let keyPath: AnyKeyPath


Technically this should now be:

public struct Shared<Value: Sendable> {

…and we should drop the conditional conformance below. But there are some weird interactions with _AnyKeyPath. Let's chat about this today.

mbrandonw · 2024-08-30T15:46:29Z

Sources/ComposableArchitecture/SharedState/Shared.swift

+      fatalError("Conformance of 'Shared<Value>' to 'MutableCollection' is unavailable.")
    }
    set {
-      self._wrappedValue[position] = newValue.wrappedValue
+      fatalError("Conformance of 'Shared<Value>' to 'MutableCollection' is unavailable.")


This subscript is unavailable so it should be ok to fatalError. If we don't fatal error, then we get sendability warnings that can't be fixed because you can't conditionally conform to a protocol with a constraint against a marker protocol, like Sendable.

stephencelis · 2024-08-30T16:51:18Z

Sources/ComposableArchitecture/Internal/DefaultSubscript.swift

@@ -1,7 +1,7 @@
-final class DefaultSubscript<Value>: Hashable {
-  var value: Value
+final class DefaultSubscript<Value: Sendable>: Hashable, Sendable {


We know we want to remove these helpers in the future, but since they're use by store.scope let's split off a non-sendable version for that.

* Address effect cancellation sendability * fix * wip * wip

mbrandonw added 2 commits August 30, 2024 11:03

Address @shared sendability.

70cf0bd

Undo UncheckedSendable<UserDefaults>.

68023fa

mbrandonw requested a review from stephencelis August 30, 2024 15:29

mbrandonw commented Aug 30, 2024

View reviewed changes

clean up

17eed0c

mbrandonw commented Aug 30, 2024

View reviewed changes

mbrandonw added 3 commits August 30, 2024 12:06

wip

4b38430

drop AnySendable.

2dac7a3

wip

d277743

stephencelis reviewed Aug 30, 2024

View reviewed changes

stephencelis approved these changes Aug 30, 2024

View reviewed changes

stephencelis and others added 10 commits August 30, 2024 15:20

Address Effect.throttle sendability (#3325)

53d5389

Address effect cancellation sendability (#3326)

414e4fa

* Address effect cancellation sendability * fix * wip * wip

Merge remote-tracking branch 'origin/main' into shared-sendability

0020365

Separate SendableDefaultSubscript from DefaultSubscript.

36d2323

fix test

2faa246

drop escaping

85354f4

switch on swift 6 language mode

e77c431

xcode 16

93e858d

update test

833be15

wip

d185c0a

mbrandonw merged commit 7c6fb26 into main Sep 6, 2024
8 checks passed

mbrandonw deleted the shared-sendability branch September 6, 2024 20:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address @Shared sendability. #3329

Address @Shared sendability. #3329

mbrandonw commented Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

mbrandonw Aug 30, 2024

stephencelis Aug 30, 2024

		private var subscription: Shared<Value>.Subscription?
		private var subscription: Shared<Value>.Subscription!

Address @Shared sendability. #3329

Address @Shared sendability. #3329

Conversation

mbrandonw commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment