-
Notifications
You must be signed in to change notification settings - Fork 10.5k
add _SwiftToolchain availability and add it to the SwiftStdlib macros #81715
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci Please test |
It's a bit unfortunate that we're introducing a new hard-coded availability domain so close to having custom availability domains (https://forums.swift.org/t/pitch-extensible-availability-checking/79308). However, I don't think that will be ready in time to satisfy this use case so I agree that this is probably a reasonable thing to do anyways. I would like to have a path to removing the hard-coded domain in the future when custom domains are fleshed out, though. |
I think this also needs to potentially require an experimental feature to be enabled in order to allow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SwiftifyImport part looks good
9c9ff8e
to
e8f6804
Compare
@swift-ci Please test |
I rebased (a couple times) to deal with merge conflicts, and changed a couple related bits of collateral damage:
Both of these changes were motivated by wanting to lessen the impact on module interface files outside the standard library (and also in tests) by not copying in @tshortli This all seemed unrelated to the work i talked with you about for gating this behind an experimental feature. I could still add that if you'd like. |
e8f6804
to
3daf14a
Compare
@swift-ci Please test |
@tshortli I've added the experimental feature requirement for the new availability domain. This should be the last of your review comments. |
@swift-ci Please test |
@@ -40,7 +40,8 @@ class AvailabilityInference { | |||
/// to ToDecl. | |||
static void | |||
applyInferredAvailableAttrs(Decl *ToDecl, | |||
ArrayRef<const Decl *> InferredFromDecls); | |||
ArrayRef<const Decl *> InferredFromDecls, | |||
bool includeSwiftToolchain = true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defaulting includeSwiftToolchain
to true
makes me a little nervous as it seems easy to make a mistake with - does omitting a default value here cause too much churn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it even need to be a parameter right now? Looking at all the calls to applyInferredAvailableAttrs
that exist today, I'm wondering if there's ever a need to propagate the documentation-only version of this attribute.
aba598a
to
03e830c
Compare
@swift-ci Please test |
@swift-ci Please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we are shipping the SDK as part of the toolchain bundle as a side-effect of the current state of the build system, toolchain versions don't determine what symbols are available from a runtime. Apple platforms build the runtimes separately from the toolchain and we are working toward splitting this for the other platforms as well.
I do think we need something to indicate the stdlib version that is separate from the OS version, but the current name is misleading.
@swift-ci Please test Windows |
@etcwilde Are you suggesting that the current approach is entirely wrong, or do you just have a problem with the name? I will admit that i'm using |
rdar://151390924
The
swift
availability domain is used to determine the language mode that is required to use a particular symbol. However, it is still useful to know what toolchain release introduced a particular API, especially for documentation purposes. This is especially important for the--{build,preview}-stdlib-docs
feature added in #76152, which currently only displays the potentially-misleadingswift
availability domain.Many symbols in the standard library use the
SwiftStdlib
availability macros to indicate which versions of Apple's OSs they can be back-deployed to. While these macros are versioned according to the toolchain release version, this information is not conveyed into the AST at all.This PR adds a
_SwiftToolchain
availability domain that can be used to indicate which version of the Swift toolchain a symbol was released in. It is meant to be added to availability macros like theSwiftStdlib
macros, so it can be added to an@available
attribute alongside other platforms, unlikeswift
. Its active version is set to the release version of the running compiler. The primary purpose for this domain is for documentation, so it's not allowed to be used in#available
checks by itself. (It can be combined with platform domains in these situations, since availability macros are likely to be used in those situations as well.)This PR then adds the
_SwiftToolchain
domain to theSwiftStdlib
availability macros, and ensures that it can be properly rendered in symbol graphs when building the stdlib docs with--{build,preview}-stdlib-docs
. (Right now theSwiftToolchain
token is printed as-is in documentation; a separate PR to swift-docc-symbolkit is needed to render it more nicely.)