Skip to content

Add support for the libswiftCompatibilitySpan.dylib backward deployment library #359

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

slavapestov
Copy link

No description provided.

@slavapestov
Copy link
Author

This is a work-in-progress; I still want to add tests. Also the whole thing feels like a bunch of copy and paste. The problem is that "Swift in the OS" and "Swift concurrency" are two concepts plumbed through all these layers, and now I'm adding a third. Suggestions for improving this are welcome.

fileprivate(set) var minimumOSForSwiftConcurrency: Version? = nil

/// Minimum OS version for Span in the standard library (Swift 6.2). If this is `nil`, the platform does not support Swift concurrency at all.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"the platform does not support Swift concurrency at all."

Think this needs to be updated to reflect Span, rather than Swift Concurrency

@@ -287,6 +295,29 @@ extension Platform {

return version >= minimumSwiftConcurrencyVersion
}

/// Determines if the platform natively supports Swift 6.2's Span type. If `false`, then the Swift Span back-compat concurrency libs needs to be copied into the app/framework's bundle.
public func supportsSwiftSpanNatively(_ scope: MacroEvaluationScope, forceNextMajorVersion: Bool = false, considerTargetDeviceOSVersion: Bool = true) -> Bool? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why add default values for the arguments? This is only ever called in one place.

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.

2 participants