Skip to content

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

@ahoppen

Description

@ahoppen
Member

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.

Activity

ahoppen

ahoppen commented on Jan 23, 2025

@ahoppen
MemberAuthor

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

mustiikhalil

mustiikhalil commented on Jan 25, 2025

@mustiikhalil
Contributor

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

ahoppen

ahoppen commented on Jan 27, 2025

@ahoppen
MemberAuthor

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

mustiikhalil

mustiikhalil commented on Jan 27, 2025

@mustiikhalil
Contributor

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

mustiikhalil

mustiikhalil commented on Feb 4, 2025

@mustiikhalil
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

ahoppen commented on Feb 4, 2025

@ahoppen
MemberAuthor

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

mustiikhalil commented on Feb 4, 2025

@mustiikhalil
Contributor

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

ahoppen commented on Feb 4, 2025

@ahoppen
MemberAuthor

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

linked a pull request that will close this issue on Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @ahoppen@mustiikhalil

      Issue actions

        Replace implementation of `AtomicBool` etc. by atomic from the standard library · Issue #1949 · swiftlang/sourcekit-lsp