Skip to content

Replace implementation of AtomicBool etc. by atomic from the standard library #1949

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
ahoppen opened this issue Jan 22, 2025 · 8 comments · May be fixed by #1969
Open

Replace implementation of AtomicBool etc. by atomic from the standard library #1949

ahoppen opened this issue Jan 22, 2025 · 8 comments · May be fixed by #1969

Comments

@ahoppen
Copy link
Member

ahoppen commented Jan 22, 2025

We should be able to raise the deployment target of SourceKit-LSP to macOS 14.0 and thus use the atomics from the standard library.

@ahoppen
Copy link
Member Author

ahoppen commented Jan 23, 2025

Synced to Apple’s issue tracker as rdar://143438616

@mustiikhalil
Copy link
Contributor

I wonder if replacing it with a ManagedAtomic<Type> would be more than enough or does it have to be UnsafeAtomic?

@ahoppen
Copy link
Member Author

ahoppen commented Jan 27, 2025

I meant ManagedAtomic by the atomic types in the standard library.

@mustiikhalil
Copy link
Contributor

Perfect! I can take a look at this by the end of this week

@mustiikhalil
Copy link
Contributor

Do we want to always import swift-atomics in each and every framework? or should we write a wrapper around it?

example:

struct Atomic<T> {
  let lock: ManagedAtomic<T>

var value: T? {
  get { lock.load(ordering: ....)  }
  set { lock.store(newValue, ordering: ....) }
}

The code above will replace somthing like this:

let lock = ManagedAtomic(false)
/// some code
lock.store(v, ordering: ...)
/// more code
if lock.load(ordering: ....) {

}

@ahoppen
Copy link
Member Author

ahoppen commented Feb 4, 2025

I was thinking to use the Atomic type that’s available in the standard library: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0410-atomics.md

@mustiikhalil
Copy link
Contributor

mustiikhalil commented Feb 4, 2025

Oh I didn't know that existed within the Synchronization framework.

At the same time wouldn't that only work on swift 6 and above? Is deprecating swift 5.10 support in the libraries roadmap?

https://github.com/swiftlang/swift/blob/c14561fba3c057a247f5bcaa514536cd0d22102b/stdlib/public/Synchronization/Atomics/Atomic.swift#L20

(And it seems that that Atomics is based on macOS 15, not sure yet. Still trying to figure out why Xcode is misbehaving)

@ahoppen
Copy link
Member Author

ahoppen commented Feb 4, 2025

Yes, we can raise the deployment target to macOS 15 on main, so we can use Synchronization.

@mustiikhalil mustiikhalil linked a pull request Feb 5, 2025 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants