paypal
Resolve Compile Errors with Strict Swift Concurrency Checking
Reason for changes
Apple has suggested you work towards complete checking, not just to avoid potential problems, but also in preparation for the expected checking that a future release of Swift 6 will enforce.
In WWDC 2022 video "Eliminate data races using Swift Concurrency", they state "Complete checking approximates the intended Swift 6 semantics to completely eliminate data races".
Summary of changes
- Resolve compile errors with Swift Concurrency Check set to "Complete" in preparation for Swift 6
-
References to properties in UIDevice, UIApplication in structs or classes result in an error unless the struct or classes or specific functions are referencing these properties are also explicitly guaranteed to run on main thread.
-
Unit tests using waitForExpectations function needed to be annotated with @MainActor to resolve compile errors
-
In PaymentButton's subclasses, their Representable structs that wrap the UIKit elements for SwiftUI views, needed to have initializers that are annotated @MainActor to resolve compile errors.
PaymentButton is a subclass of UIButton which is annotated as @MainActor. I think it is not enforced for the UIButton subclass initializers because of backward compatibility.
-
I will address some Sendable warnings in another PR. We can decide when we want to implement these changes. It's good to be proactive in at least spiking potential scope of work before Swift 6.
Checklist
~- [ ] Added a changelog entry~
Authors
- @KunJeongPark
It seems like there is also an intermediary Targeted level of checking before Complete that we can consider for making this move progressively. I found this medium article helpful for talking about these checks. Since we are an SDK we likely won't be able to release with Swift 6 until a bit after the release since that is a breaking change. With that in mind we should consider if we want to jump right to Complete or start with Targeted.
It seems like there is also an intermediary
Targetedlevel of checking beforeCompletethat we can consider for making this move progressively. I found this medium article helpful for talking about these checks. Since we are an SDK we likely won't be able to release with Swift 6 until a bit after the release since that is a breaking change. With that in mind we should consider if we want to jump right toCompleteor start withTargeted.
I checked with Targeted option and we have no errors or warnings other than same old warnings without the concurrency checking. I think for the compile errors with complete option, the only breaking change would be for the buttons if merchant is not instantiating them on main thread.
I think marking AnalyticsEventData @MainActor is ok since the work in there is lightweight. Alternative would be to fetch device information somewhere else and store it for access here, I don't see an obvious place to store the info. I can implement something like DeviceInfoCache. I thought @MainActor was the simplest way to deal with compile errors.
I think it would be surprising if any version of Swift in the future would enforce this one, any reference to UIDevice's read only properties, for example, giving compile errors without explicit main thread context.
all that these would be a breaking change for merchants or are these changes mainly behind the scenes? What changes would this cause for merchant demo apps that also have swift strict concurrency checking enabled?
The AnalyticEventData change, I think that's just us, so no breaking change for merchant. PresentationAnchor change, that function is used behind the scenes as part of ASWebAuthenticationSession, so I don't think it will affect merchants' implementations. It would only be problematic if merchant has the strict checking and we don't. The SwiftUI buttons, the change is only for the Representables, for merchants who are using it in SwiftUI and buttons on SwiftUIViews, I don't think there should be any difference in the way it is instantiated by the merchant.
I didn't submit the complete concurrency checking option but I fixed compile errors that occurred as a result of that option check.
I didn't submit the complete concurrency checking option but I fixed compile errors that occurred as a result of that option check.
Personally, I think if we are adding the code to support complete strict concurrency checking, we should enable it in the SDK at the same time. This way it is clear to merchants/us when we added support and what the necessary code changes were. But curious to hear how other folks feel.
@KunJeongPark I just came across this Swift Forum thread on Swift 6. There are a few things of note with regards to this PR:
Readiness of data-race safety by default There are still a number of bugs and holes in the Sendable checking model that admit data races under strict concurrency checking. Equally important, strict concurrency checking currently issues a significant number of false positive reports of data races, making the complete checking difficult to program against.
The Swift 6 language mode will only be declared ready once the Language Steering Group determines that the programming model is effective and usable. The remaining language work to complete data-race safety for the Swift 6 language mode will fall into two categories:
Fill all holes in the strict concurrency model so that data races are diagnosed statically, or dynamically where static safety cannot be proven. Mitigate false reports of data races on patterns that are proven to be safe. Swift 5.10 contains a number of significant bug fixes in the actor isolation and Sendable checking. In addition, the following language changes are currently being designed and implemented to undergo the Swift evolution review process:
SE-0411: Isolated default value expressions 17 Inferring @Sendable for methods and key-path literals 15 Strict concurrency checking for global and static variables 9 Improved control over closure actor isolation 13 Lifting restrictions on non-Sendable values through isolated value regions 12
Together, these changes fill the remaining major holes in strict concurrency checking and improve the usability of strict concurrency checking by introducing more Sendable inference and enabling safe ways to transfer non-Sendable values across isolation boundaries. The Language Steering Group acknowledges that other language changes in the area of concurrency are important, but the changes above are necessary for defining the Swift 6 language mode. Other concurrency features are additive and can be explored independently.
Based on these comments and since we do not have a major version planned to update to either Swift 5.10 (that includes some of these fixes) or Swift 6 (that does not yet have a targeted release date) it seems like we should hold off on these changes until the lauguage has ironed out some of these bugs. Curious to know what other folks think!
@KunJeongPark I just came across this Swift Forum thread on Swift 6. There are a few things of note with regards to this PR:
Readiness of data-race safety by default There are still a number of bugs and holes in the Sendable checking model that admit data races under strict concurrency checking. Equally important, strict concurrency checking currently issues a significant number of false positive reports of data races, making the complete checking difficult to program against. The Swift 6 language mode will only be declared ready once the Language Steering Group determines that the programming model is effective and usable. The remaining language work to complete data-race safety for the Swift 6 language mode will fall into two categories: Fill all holes in the strict concurrency model so that data races are diagnosed statically, or dynamically where static safety cannot be proven. Mitigate false reports of data races on patterns that are proven to be safe. Swift 5.10 contains a number of significant bug fixes in the actor isolation and Sendable checking. In addition, the following language changes are currently being designed and implemented to undergo the Swift evolution review process: SE-0411: Isolated default value expressions 17 Inferring @Sendable for methods and key-path literals 15 Strict concurrency checking for global and static variables 9 Improved control over closure actor isolation 13 Lifting restrictions on non-Sendable values through isolated value regions 12 Together, these changes fill the remaining major holes in strict concurrency checking and improve the usability of strict concurrency checking by introducing more Sendable inference and enabling safe ways to transfer non-Sendable values across isolation boundaries. The Language Steering Group acknowledges that other language changes in the area of concurrency are important, but the changes above are necessary for defining the Swift 6 language mode. Other concurrency features are additive and can be explored independently.
Based on these comments and since we do not have a major version planned to update to either Swift 5.10 (that includes some of these fixes) or Swift 6 (that does not yet have a targeted release date) it seems like we should hold off on these changes until the lauguage has ironed out some of these bugs. Curious to know what other folks think!
Those are great finds from the forums. I agree, I did have a conversation with some people and consensus is to mark possible issues and keep the branches updated periodically and revisit as warnings and errors in Xcode get refined for next versions.
I will keep making changes, checking forums, keeping the branches updated and I look forward to input from all of you whenever you have a chance. I think it's good for us to be ahead of it and start thinking about possible issues.
Closing due to inactivity, when we are ready to prioritize this work we can reopen this PR or start a new PR using this as a reference