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.

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

[_AtomicShims] On Darwin, ensure we link against libswiftCore using assembly shenanigans #97

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

lorentey
Copy link
Member

Like #95 and #96, the intention here is to avoid a linker failure when the build system decides to build this module as a standalone library.

Hopefully this will all go away soon, when swift_retain_n/swift_release_n will become primitives exposed by the stdlib.

Resolves #55

Checklist

  • I've read the Contribution Guidelines
  • My contributions are licensed under the Swift license.
  • I've followed the coding style of the rest of the project.
  • I've added tests covering all new code paths my change adds to the project (if appropriate).
  • I've verified that my change does not break any existing tests.
  • I've updated the documentation if necessary.

…ssembly shenanigans

This avoids a linker failure when the build system decides to build this module as a standalone library.

Hopefully this will all go away soon, when swift_retain_n/swift_release_n will become primitives exposed by the stdlib.
@lorentey
Copy link
Member Author

@swift-ci test

@lorentey lorentey requested a review from glessard August 11, 2023 23:08
@lorentey
Copy link
Member Author

Local testing confirms that this resolves the issue that's plaguing #55 (and rdar://108390931).

@lorentey
Copy link
Member Author

Local testing confirms that this resolves the issue that's plaguing #55 (and rdar://108390931).

Not so fast! It looks like the method I'm using to override the swift-atomics release tag with a local checkout also resolves it, by itself. Grr.

// Ensure we link with libswiftCore.dylib even when the build system decides
// to build this module as a standalone library.
// (See https://github.com/apple/swift-atomics/issues/55)
__asm__(".linker_option \"-L/usr/lib/swift\"\n");
Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, -L doesn't actually work, but (for some reason) it also seems unnecessary, at least on 5.9.

This emits a warning and it seems to be unnecessary.
@lorentey
Copy link
Member Author

@swift-ci test

@lorentey lorentey merged commit 38a8ccd into apple:main Aug 15, 2023
@lorentey lorentey deleted the autolink-libswiftCore branch August 15, 2023 00:00
@rvsrvs
Copy link

rvsrvs commented Sep 13, 2023

@lorentey does Swift 5.9 expose swift_retain_n and swift_release_n? Thinking this would fix all my problems with playgrounds!

@lorentey lorentey added this to the 1.2.0 milestone Sep 22, 2023
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.

import Atomics won't compile in Playgrounds
3 participants