Skip to content

[Concurrency][Stdlib] Add SwiftStdlibCurrentOS availability, use it. #81440

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jun 3, 2025
Merged

[Concurrency][Stdlib] Add SwiftStdlibCurrentOS availability, use it. #81440

merged 6 commits into from
Jun 3, 2025

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented May 12, 2025 •
edited
Loading

If you use SwiftStdlibCurrentOS availability, you will be able to use new types and functions from within the implementation. This works by, when appropriate, building with the CurrentOS availability set to the current deployment target.

rdar://150944675

@al45tair
[Concurrency][Stdlib] Add SwiftStdlibCurrentOS availability, use it.
28f96e6 
If you use SwiftStdlibCurrentOS availability, you will be able to
use new types and functions from within the implementation. This
works by, when appropriate, building with the CurrentOS availability
set to the current deployment target.

rdar://150944675
@al45tair al45tair force-pushed the currentos-availability branch from d36db74 to 28f96e6 Compare May 12, 2025 11:07
@al45tair
Copy link
Contributor Author

@swift-ci Please test

@stephentyrone
Copy link
Contributor

stephentyrone commented May 12, 2025 •
edited
Loading

Love where this is going, but I think the names are more-or-less backwards; we shouldn't reference the "OS" for changes that are just new API additions in the stdlib at all. They should be "SwiftStdlib x.y," and the rare change that does depend on an outside OS feature should either reference the OS version, or reference the feature itself (Foundation does this).

This may make phasing adoption in slightly more complex however.

etcwilde
etcwilde reviewed May 12, 2025
View reviewed changes
Runtimes/Core/cmake/modules/PlatformInfo.cmake Outdated
@@ -36,3 +36,26 @@ if(NOT SwiftCore_ARCH_SUBDIR)

message(CONFIGURE_LOG "Swift Arch: ${arch}")
endif()

if(NOT SwiftCore_SWIFT_AVAILABILITY_PLATFORM)
set(availability_platform_macosx "macOS")
Copy link
Member

Choose a reason for hiding this comment

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

I don't love this bit. We're trying to get away from the hard-coded hand-maintained tables. I wonder if there is any way we could embed it into the existing availability macro file?
CC @compnerd, do you have any ideas?

Maybe this is something we can embed in cache files?

Copy link
Member

Choose a reason for hiding this comment

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

CPP to generate a response file would be my recommendation. I think that we were already doing that somewhere? (Perhaps the new build system?)

Runtimes/Core/cmake/modules/PlatformInfo.cmake
set(availability_platform_bridgeos "bridgeOS")

if(SwiftCore_SWIFT_MODULE_TRIPLE MATCHES ".*-([^-]+)-simulator$")
set(platform "${CMAKE_MATCH_1}")
Copy link
Member

Choose a reason for hiding this comment

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

I think you should be able to get this information from the platform entry in the target info without needing to regex it out of the module triple.

swift/Runtimes/Core/cmake/modules/PlatformInfo.cmake

Line 25 in 3b3d13c

string(JSON platform GET "${target_info_json}" "target" "platform")

Runtimes/Core/cmake/modules/AvailabilityMacros.cmake Outdated
# ON will cause us to use the "proper" availability instead.
string(REPLACE "SwiftStdlib" "SwiftStdlibCurrentOS" current "${def}")
if(NOT SwiftCore_ENABLE_STRICT_AVAILABILITY AND SwiftCore_SWIFT_AVAILABILITY_PLATFORM)
string(REGEX MATCH "${SwiftCore_SWIFT_AVAILABILITY_PLATFORM} ([0-9]+(\.[0-9]+)+)" platform_version "${def}")
Copy link
Member

Choose a reason for hiding this comment

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

AvailabilityMacros.cmake is loaded before platform info, so SwiftCore_SWIFT_AVAILABILITY_PLATFORM won't be set by that here.

@al45tair
Copy link
Contributor Author

Love where this is going, but I think the names are more-or-less backwards

Agreed, but I didn't want to change the meaning of SwiftStdlib; for context, the reason I'm doing this now is that when we tell Swift what SwiftStdlib 6.2 is, a load of Concurrency things will break because Concurrency wants to use new things to implement other things, but that won't work on the CI machines (or in OSS) where the build system is a lower version than whatever SwiftStdlib 6.2 ends up being.

We could make changing the names a separate task, if someone comes up with something better.

@al45tair
[Build] Add catalyst support to SwiftStdlibCurrentOS.
cd7701c 
Add catalyst support for SwiftStdlibCurrentOS.

Also, set a minimum deployment target when building Concurrency;
this stops the build failing when we're trying to build on older
systems where Concurrency didn't really exist yet.

rdar://150966361
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test macOS platform

@al45tair
[Build] Fix variable name in new build system.
c749e64 
It should have been `SwiftCore_MODULE_TRIPLE`, not
`SwiftCore_SWIFT_MODULE_TRIPLE`.
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
[Build] Use FATAL_ERROR, not ERROR.
7e318e2 
We should be using `FATAL_ERROR` for error messages from CMake, not
just `ERROR`.
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
[Build] Fix macos vs macosx issue.
d110464 
The module triple bizarrely omits the "x" from "macosx".

Also disable some of this code on non-Apple platforms.

rdar://150944675
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@stephentyrone
Copy link
Contributor

I don't think you need to fully solve the naming issue now, but please do not merge this under a name that references "OS"; there will be considerable inertia to changing this later, so we should not use a name that we know is bad. If we need a placeholder name, let's call this @available(DeploymentTarget) or similar, which is at least not wrong.

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

I don't think you need to fully solve the naming issue now, but please do not merge this under a name that references "OS"; there will be considerable inertia to changing this later, so we should not use a name that we know is bad. If we need a placeholder name, let's call this @available(DeploymentTarget) or similar, which is at least not wrong.

Let's discuss this offline and agree what we're going to do here.

@al45tair al45tair force-pushed the currentos-availability branch from fca653e to 0b560cb Compare May 30, 2025 15:09
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
[Build] Fix availability platform macros.
467f401 
These need to be defined always; we'll set them to "unknown" and "none"
respectively if they end up being something we don't understand.

rdar://150966361
@al45tair al45tair force-pushed the currentos-availability branch from 0b560cb to 467f401 Compare June 2, 2025 10:30
@al45tair
Copy link
Contributor Author

al45tair commented Jun 2, 2025

@swift-ci Please smoke test

@al45tair al45tair merged commit d5ed335 into swiftlang:main Jun 3, 2025
3 checks passed
@etcwilde etcwilde mentioned this pull request Jun 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etcwilde etcwilde etcwilde left review comments

@ktoso ktoso ktoso approved these changes

@stephentyrone stephentyrone stephentyrone approved these changes

@edymtt edymtt edymtt approved these changes

@justice-adams-apple justice-adams-apple Awaiting requested review from justice-adams-apple justice-adams-apple is a code owner

@shahmishal shahmishal Awaiting requested review from shahmishal shahmishal is a code owner

@compnerd compnerd Awaiting requested review from compnerd compnerd is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@al45tair @stephentyrone @compnerd @ktoso @etcwilde @edymtt