Make the codebase clean under SWIFT_STRICT_CONCURRENCY=complete#1
Conversation
Clears the data-race-checking warnings that blocked enabling complete concurrency checking (a prerequisite for the Swift 6 language mode) and turns the setting on in project.yml so any new issue surfaces immediately. - Theme and its token types are now Sendable; Typography's font closures are marked @sendable (the one non-Sendable member that broke the chain), which also makes the Theme statics and ThemeEnvironment.defaultValue safe. - Catalog.isoTimestamp is a computed property rather than a shared, non-Sendable ISO8601DateFormatter static. - The KVO status/rate observers re-capture [weak self] on the inner DispatchQueue.main.async closure and run their body in MainActor.assumeIsolated, so self is no longer captured from the outer closure's weak var in concurrently-executing code. - The didPlayToEndTime handler sends an ObjectIdentifier across the main hop instead of the non-Sendable Notification / AVPlayerItem. Verified: a full from-scratch build under SWIFT_STRICT_CONCURRENCY=complete is warning-free, and a SWIFT_STRICT_CONCURRENCY=minimal build still succeeds (no regression to the non-strict build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tsvb
left a comment
There was a problem hiding this comment.
Reviewed and verified — ready to merge. (Posting as a comment since GitHub blocks approving one's own PR.)
Independently confirmed the central claim: a fresh clean build in an isolated worktree with SWIFT_STRICT_CONCURRENCY=complete (fresh -derivedDataPath so incremental can't fake it) produced zero concurrency warnings, zero errors, BUILD SUCCEEDED.
Per file
- Theme.swift —
Sendableon the token types plus@Sendable (CGFloat) -> Fonton theTypographyclosures. The closure annotation is the load-bearing fix (Themecan't beSendableuntil its closures are); the font-builders capture no mutable state, so it's sound. - Catalog.swift —
isoTimestampmoved fromstatic letto a computed property becauseISO8601DateFormatterisn'tSendable. Correct. Minor, non-blocking: a formatter is now allocated per call (~N per livestream load); fine for that rare path, worth revisiting only if it ever gets hot. - project.yml —
SWIFT_STRICT_CONCURRENCY: completeunder Swift 5 mode: data-race issues surface as warnings now and become errors at the eventual Swift-6 flip. Right call. - PlayerService / VideoPlayerService observers — behaviorally identical; the changes are isolation hygiene. The inner
DispatchQueue.main.asyncre-captures[weak self](so@Sendablechecking doesn't flag closing over the outer capture) andMainActor.assumeIsolatedsupplies the isolation — valid because the hop always lands on the main actor. The end-observer'sObjectIdentifierhop correctly avoids sending the non-SendableNotification/AVPlayerItemacross the boundary while preserving the exact identity match (itemID == ObjectIdentifier(currentItem)). No retain cycles introduced, no logic change.
Optional nits (non-blocking): the per-call formatter allocation above, and the async { [weak self] in assumeIsolated { … } } pattern now repeats across four observers — a small onMain { } helper could DRY it. Pure style.
The note about running xcodegen generate after pulling (the .xcodeproj is generated/git-ignored and is what picks up the project.yml setting) is exactly right and worth keeping prominent for anyone testing the branch.
What
Resolves every data-race-checking warning surfaced by building with
SWIFT_STRICT_CONCURRENCY=complete(still in the Swift 5 language mode), and turns the setting on permanently inproject.yml.Why
Complete concurrency checking is the prerequisite for eventually adopting the Swift 6 language mode. The codebase had ~11 pre-existing warnings blocking it. With them cleared and the flag enabled in the project base settings, any new concurrency issue now shows up as a warning immediately (and as an error once
SWIFT_VERSIONis raised to 6).Changes
Theme.swift—Themeand its token types (TransportSignature,AccentMode,WashStyle,Capabilities,Palette,Typography,IdleCopy) now conform toSendable. The actual blocker wasTypography's(CGFloat) -> Fontclosures, now marked@Sendable. This also makes the fourThemestatics andThemeEnvironment.defaultValueconcurrency-safe.Catalog.swift—isoTimestampis a computed property instead of a shared, non-SendableISO8601DateFormatterstatic. It's only hit a handful of times per livestream load, and the retry path right below already created a formatter per call.PlayerService.swift/VideoPlayerService.swift— the KVOstatus/timeControlStatusobservers re-capture[weak self]on the innerDispatchQueue.main.asyncclosure and run their body insideMainActor.assumeIsolated. Previously the inner@Sendableclosure closed over the outer closure's weak-selfcapture, which strict concurrency flags as "reference to captured var 'self' in concurrently-executing code".PlayerService.swift— thedidPlayToEndTimehandler now derives aSendableObjectIdentifierand sends only that across the main-actor hop, instead of capturing the non-SendableNotification/AVPlayerItem.itemDidEndcompares by identity.project.yml— base settings setSWIFT_STRICT_CONCURRENCY: complete.Note on the observer fix
I used
MainActor.assumeIsolated+ an inner[weak self]rather thanTask { @MainActor in … }. ATask's pure-Swift@Sendableclosure would have introduced new warnings for capturing the non-SendableAVPlayerItem/AVPlayer, whereas keeping theDispatchQueue.main.asynchop preserves the existing semantics and matches the idiom already used elsewhere in these files.Verification
-derivedDataPath) underSWIFT_STRICT_CONCURRENCY=complete: 46 Swift files compiled, 0 errors, 0 warnings,BUILD SUCCEEDED.SWIFT_STRICT_CONCURRENCY=minimal: also clean — the non-strict build is not regressed.The
.xcodeprojis generated by xcodegen (git-ignored); runxcodegen generateafter pulling to pick up theproject.ymlchange.🤖 Generated with Claude Code