more Sendable conformances & Sendable Context #381

weissi · 2023-01-17T17:35:57Z

Technically API breaking:

switches public typealias Context = Dictionary<...> for public struct Context
requires HashingAlgorithm and TracingEvent(Collection) to be Sendable

neonichu · 2023-01-17T17:50:37Z

neonichu · 2023-01-17T17:51:35Z

We don't promise a stable API at this point, so API breakage is acceptable. We'd need to adopt any potential clients in the toolchain, of course.

weissi · 2023-01-17T17:55:19Z

We don't promise a stable API at this point, so API breakage is acceptable. We'd need to adopt any potential clients in the toolchain, of course.

Yes, I'd tag it as 0.5.0 too in which case public API breakage is almost expected for 0.x packages.

neonichu · 2023-01-17T18:34:40Z

@swift-ci please test

jakepetroules · 2023-01-18T08:29:32Z

Sources/TSCUtility/Context.swift

+    }
+
+    @available(*, deprecated, message: "Values should be Sendable")
+    @_disfavoredOverload


one of my favorite tricks :)

@jakepetroules at some point we'll need @_overloadFavouriteness(10) if we have need more than one level ;)

jakepetroules · 2023-01-18T08:34:16Z

Sources/TSCUtility/Tracing.swift

@@ -135,8 +135,24 @@ public struct TracingEvent: TracingEventProtocol, Codable {
    #endif
 }

-public class TracingCollection: TracingCollectionProtocol {
-    public var events: [TracingEventProtocol] = []
+public final class TracingCollection: TracingCollectionProtocol, @unchecked Sendable {


@unchecked because of NSLock? (if so, can add a comment?) Do we have a Lock implementation in TSC which is sendable, that we could use instead?

It's unchecked because it's manually synchronised with the lock. Even if the lock itself is sendable, the compiler can't check that you've used it correctly.

The only way to get that is to use something like NIOLockedValueBox<[TracingEventProtocol>] but that's not available here.

neonichu · 2023-01-18T10:54:36Z

Looks like the macOS failures are all about the changes?

neonichu · 2023-01-18T16:59:51Z

@swift-ci please test

weissi · 2023-01-18T17:02:26Z

@neonichu Yes, that's an old Foundation SDK which doesn't have enough Sendable annotations. Which SDK is this using?

neonichu · 2023-01-18T17:14:42Z

@weissi I believe the Xcode installed on Swift CI is still 13A5201i, so that would mean macOS 12.0 (21A5294d). @shahmishal to double check.

weissi · 2023-01-18T18:08:23Z

@neonichu thanks, yeah, that's old. I removed the Sendables that are currently problematic because that Xcode also doesn't understand @preconcurrency :P

neonichu · 2023-01-18T18:34:14Z

@swift-ci please test

weissi · 2023-01-23T12:26:28Z

@neonichu / @jakepetroules Could we upgrade the CI? It seems ridiculously old:

/Users/ec2-user/jenkins/workspace/pr-swift-tools-support-core-macos/branch-main/swift-tools-support-core/Sources/TSCUtility/Context.swift:13:79: error: unknown attribute 'unchecked'
public struct Context: /* until we can remove the support for 'Any' values */ @unchecked Sendable {
                                                                              ^

it doesn't know about @unchecked??

MaxDesiatov · 2023-01-23T14:05:50Z

So far I've used UnsafeSendable instead of @unchecked Sendable, conditional on #if swift(<5.6) as in PR #382: https://github.com/apple/swift-tools-support-core/pull/382/files#diff-c659355a0fbeab4d2e87c402f625dfcd9d014493b2c5f28a0d45ea722de362ddR13

weissi · 2023-01-23T14:09:55Z

So far I've used UnsafeSendable instead of @unchecked Sendable, conditional on #if swift(<5.6) as in PR #382: https://github.com/apple/swift-tools-support-core/pull/382/files#diff-c659355a0fbeab4d2e87c402f625dfcd9d014493b2c5f28a0d45ea722de362ddR13

Sure, but @unchecked Sendable got introduced with Swift 5.7 (-> https://github.com/apple/swift-evolution/blob/main/proposals/0302-concurrent-value-and-concurrent-closures.md#sendable-and-sendable-closures) and given that SwiftPM is part of the Swift toolchain it should be targeting >= 5.8 by now and not < 5.7.

So yes, I know I could put in hacks to work around this, this really shouldn't be necessary, right?

MaxDesiatov · 2023-01-23T14:21:43Z

AFAIR we're keeping SwiftPM compatible with older Swift versions on purpose, since libSwiftPM clients may use those. @neonichu would be able to clarify this.

Thanks for linking to that proposal, based on that swift(<5.6) check in our codebase should really look like swift(<5.7), right?

neonichu · 2023-01-23T16:21:54Z

AFAIR we're keeping SwiftPM compatible with older Swift versions on purpose, since libSwiftPM clients may use those. @neonichu would be able to clarify this.

Yes, we can definitely not target 5.8 only. It's also not just about libSwiftPM clients, but we want to enable people to contribute with just the stable version of Xcode.

There's also the practical issue that Swift CI has 13A5201i installed, so we have to stay compatible with that for the time being. If that gets upgraded to a 14.x era Xcode, we could discuss requiring 5.7

jakepetroules · 2023-01-25T05:17:57Z

@neonichu / @jakepetroules Could we upgrade the CI? It seems ridiculously old:

/Users/ec2-user/jenkins/workspace/pr-swift-tools-support-core-macos/branch-main/swift-tools-support-core/Sources/TSCUtility/Context.swift:13:79: error: unknown attribute 'unchecked'
public struct Context: /* until we can remove the support for 'Any' values */ @unchecked Sendable {
                                                                              ^

it doesn't know about @unchecked??

Based on @neonichu's comments just above, can you add #if swift(...) as needed to get this past CI checks?

weissi · 2023-01-25T11:39:06Z

@swift-ci please test

weissi · 2023-01-27T13:40:06Z

@neonichu I'm hitting this linker error (CC @jakepetroules / @MaxDesiatov )

[82/84] Merging module TSCBasicTests
[83/84] Merging module TSCUtilityTests
Undefined symbols for architecture x86_64:
  "_$ss8SendableMp", referenced from:
      _$ss8Sendable_pMa in Context.swift.o
ld: symbol(s) not found for architecture x86_64
[83/84] Linking swift-tools-support-corePackageTests
error: fatalError

which is

$ pbpaste | swift demangle
Undefined symbols for architecture x86_64:
  "protocol descriptor for Swift.Sendable", referenced from:
      type metadata accessor for Swift.Sendable in Context.swift.o
ld: symbol(s) not found for architecture x86_64
[83/84] Linking swift-tools-support-corePackageTests

a bug in either Swift or SwiftPM not managing to link libswift_Concurrency.dylib on macOS :|. Not sure how to proceed without upgrading the buiders here...

weissi · 2023-01-30T18:02:30Z

@swift-ci please test

weissi · 2023-01-31T23:00:24Z

@swift-ci please test

weissi · 2023-02-02T12:21:25Z

@swift-ci please test

weissi · 2023-02-07T09:09:28Z

@swift-ci please test

weissi · 2023-02-07T09:11:16Z

@swift-ci please test

weissi · 2023-02-07T11:57:24Z

@swift-ci please test

weissi · 2023-02-07T12:07:38Z

@swift-ci please test

weissi · 2023-02-07T12:17:30Z

@swift-ci please test

weissi · 2023-02-07T12:20:48Z

@swift-ci please test

weissi · 2023-02-07T12:25:24Z

@swift-ci please test

weissi · 2023-02-07T14:04:31Z

@swift-ci please test

weissi · 2023-02-07T23:05:30Z

@swift-ci please test

weissi · 2023-02-08T11:07:10Z

@swift-ci please test

weissi · 2023-02-08T12:28:01Z

@swift-ci please test

weissi · 2023-02-08T14:03:34Z

@swift-ci please test

weissi requested review from aciidgh, abertelrud, neonichu, friedbunny and tomerd as code owners January 17, 2023 17:35

weissi requested a review from jakepetroules January 17, 2023 17:36

weissi force-pushed the jw-sendable-moar branch from 24264ef to 48bcea1 Compare January 17, 2023 17:59

jakepetroules reviewed Jan 18, 2023

View reviewed changes

jakepetroules approved these changes Jan 18, 2023

View reviewed changes

weissi force-pushed the jw-sendable-moar branch from 48bcea1 to e28cadb Compare January 18, 2023 14:45

weissi force-pushed the jw-sendable-moar branch from e28cadb to 2687983 Compare January 18, 2023 18:07

weissi force-pushed the jw-sendable-moar branch from 2687983 to df024d4 Compare January 25, 2023 11:38

weissi force-pushed the jw-sendable-moar branch from 28b332b to fddc6ab Compare January 30, 2023 17:46

weissi force-pushed the jw-sendable-moar branch from fddc6ab to 94f785d Compare January 31, 2023 23:00

weissi force-pushed the jw-sendable-moar branch from 0f6e796 to 5d52087 Compare February 2, 2023 12:21

weissi force-pushed the jw-sendable-moar branch from 5d52087 to 48bcda3 Compare February 7, 2023 09:09

weissi force-pushed the jw-sendable-moar branch from 48bcda3 to 61f074b Compare February 7, 2023 09:11

weissi force-pushed the jw-sendable-moar branch from 61f074b to b4b4ed5 Compare February 7, 2023 11:57

weissi force-pushed the jw-sendable-moar branch from b4b4ed5 to faa5880 Compare February 7, 2023 12:07

weissi force-pushed the jw-sendable-moar branch from faa5880 to f5beeba Compare February 7, 2023 12:17

neonichu approved these changes Feb 7, 2023

View reviewed changes

weissi force-pushed the jw-sendable-moar branch from f5beeba to bba2979 Compare February 8, 2023 12:27

more Sendable conformances & Sendable Context

c9785f8

weissi force-pushed the jw-sendable-moar branch from bba2979 to c9785f8 Compare February 8, 2023 14:03

weissi merged commit add9e15 into swiftlang:main Feb 8, 2023

weissi deleted the jw-sendable-moar branch February 8, 2023 22:38

more Sendable conformances & Sendable Context #381

more Sendable conformances & Sendable Context #381

Uh oh!

Conversation

weissi commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonichu commented Jan 17, 2023

Uh oh!

neonichu commented Jan 17, 2023

Uh oh!

weissi commented Jan 17, 2023

Uh oh!

neonichu commented Jan 17, 2023

Uh oh!

jakepetroules Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

weissi Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

jakepetroules Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weissi Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

neonichu commented Jan 18, 2023

Uh oh!

neonichu commented Jan 18, 2023

Uh oh!

weissi commented Jan 18, 2023

Uh oh!

neonichu commented Jan 18, 2023

Uh oh!

weissi commented Jan 18, 2023

Uh oh!

neonichu commented Jan 18, 2023

Uh oh!

weissi commented Jan 23, 2023

Uh oh!

MaxDesiatov commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weissi commented Jan 23, 2023

Uh oh!

MaxDesiatov commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonichu commented Jan 23, 2023

Uh oh!

jakepetroules commented Jan 25, 2023

Uh oh!

weissi commented Jan 25, 2023

Uh oh!

weissi commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weissi commented Jan 30, 2023

Uh oh!

weissi commented Jan 31, 2023

Uh oh!

weissi commented Feb 2, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

Uh oh!

weissi commented Feb 7, 2023

weissi commented Jan 17, 2023 •

edited

Loading

jakepetroules Jan 18, 2023 •

edited

Loading

MaxDesiatov commented Jan 23, 2023 •

edited

Loading

MaxDesiatov commented Jan 23, 2023 •

edited

Loading

weissi commented Jan 27, 2023 •

edited

Loading