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.

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

[wasm] Mark errno as nonisolated(unsafe) in wasi-libc #75821

Merged
merged 2 commits into from
Aug 17, 2024
Merged

[wasm] Mark errno as nonisolated(unsafe) in wasi-libc #75821

merged 2 commits into from
Aug 17, 2024

Conversation

kateinoigakukun
Copy link
Member

This patch is a workaround to reflect the fact that errno is thread-local and can be accessed from any actor. This is a temporary solution until we have __swift_attr__ support in apinotes or thread_local support in ClangImporter.

This change is required to build swift-corelibs-foundation with Swift 6 language mode.

@kateinoigakukun
Copy link
Member Author

@swift-ci test WebAssembly

@kateinoigakukun kateinoigakukun mentioned this pull request Aug 10, 2024
Closed
33 tasks
@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@kateinoigakukun kateinoigakukun marked this pull request as ready for review August 12, 2024 02:24
@kateinoigakukun kateinoigakukun requested a review from a team as a code owner August 12, 2024 02:24
@kateinoigakukun kateinoigakukun requested review from MaxDesiatov and removed request for a team August 12, 2024 02:24
@MaxDesiatov MaxDesiatov requested a review from al45tair August 13, 2024 10:44
@al45tair
Copy link
Contributor

An alternative might be to do what every other platform does and wrap it with a Swift wrapper that calls some inline C functions that access errno? That would avoid having to patch __errno.h, I think.

@kateinoigakukun
Copy link
Member Author

@al45tair We had been taking that approach, but unfortunately providing a Swift wrapper for an importable C declaration results in ambiguous identifier resolution, so we removed the shim #75747

@al45tair
Copy link
Contributor

We had been taking that approach, but unfortunately providing a Swift wrapper for an importable C declaration results in ambiguous identifier resolution, so we removed the shim #75747

You should be able to use API Notes to fix that already; if you mark the errno declaration from the C library as SwiftPrivate (aka NS_REFINED_FOR_SWIFT in other contexts), I think you should find that the compiler always uses the Swift definition instead.

@kateinoigakukun
[wasm] Annotate errno as SwiftPrivate by apinotes
be08ebb 
This patch adds an apinotes file for SwiftWASILibc clang module to mark
`errno` macro hidden from Swift code. This resolves ambiguity between
the C macro definition and the Swift wrapper in WASILibc overlay module.

This change installs the apinotes file to the resource directories for
both lib/swift/apinotes and lib/swift_static/apinotes.
@kateinoigakukun kateinoigakukun force-pushed the pr-680047d2b871e809cf13f27c8a4a5abc2f5c31d2 branch from 1fa232d to be08ebb Compare August 16, 2024 11:00
@kateinoigakukun
Copy link
Member Author

Thanks @al45tair! SwiftPrivate is what I was looking for. (not sure why I couldn't find it)

@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@kateinoigakukun
Copy link
Member Author

@swift-ci test WebAssembly

@kateinoigakukun kateinoigakukun mentioned this pull request Aug 16, 2024
Merged
@kateinoigakukun kateinoigakukun merged commit ba63b9f into swiftlang:main Aug 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

@al45tair al45tair al45tair approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@kateinoigakukun @al45tair @MaxDesiatov