Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt "complete" Swift concurrency #123

Closed

Conversation

jszumski
Copy link
Collaborator

@jszumski jszumski commented May 29, 2024

Adopts "complete" Swift concurrency support using Xcode 15.4:

  • types used as static variables are marked Sendable
  • refactored OSLog usage to be Sendable
  • marked protocol methods as @MainActor if any UIKit types conformed to them
  • updated the newly marked @MainActor call sites in sources and tests as @MainActor themselves

For CI, only the Bazel workflow is currently using Swift concurrency due to how the GitHub runners are provisioned and the current iOS 12.0+ deployment target.

@jszumski jszumski force-pushed the jszumski/complete-concurrency branch from afae002 to 4688929 Compare May 29, 2024 19:16
Copy link

@skorulis-ap skorulis-ap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it beneficial to add the @mainactor annotations? I expect these functions are currently used on the main thread but at their core it's only math calculations which aren't thread specific

@@ -19,6 +19,7 @@ import UIKit
/// Defines an object that vends its current user interface layout direction.
public protocol LayoutDirectionProviding {

@MainActor
var effectiveUserInterfaceLayoutDirection: UIUserInterfaceLayoutDirection { get }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skorulis-ap this needs to be MainActor for UIView to have a valid conformance on L27, but that means all the calculations that use LayoutDirectionProviding also need to be marked MainActor

@@ -19,6 +19,7 @@ import UIKit
/// The ratio of pixels to points, either of a UIScreen, a UIView's screen, or an explicit value.
public protocol ScaleFactorProviding {

@MainActor
var pixelsPerPoint: CGFloat { get }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skorulis-ap same story here, it's just math but using scale here requires MainActor:

extension UIScreen: ScaleFactorProviding {
    public var pixelsPerPoint: CGFloat {
        return scale
    }
}

it unfortunately is viral from there and all math that uses pixelsPerPoint then requires MainActor

@jszumski jszumski marked this pull request as ready for review May 30, 2024 13:09
@NickEntin
Copy link
Collaborator

Closing this since it was obviated by #132

@NickEntin NickEntin closed this Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants