Make UnsafeAtomicState safe.
#371
Conversation
| return OSAtomicCompareAndSwap32Barrier(expected.rawValue, | ||
| next.rawValue, | ||
| valuePointer) | ||
| } |
There was a problem hiding this comment.
I'm not sure this is valid.
This:
The pointer argument to body is valid only for the lifetime of the closure. Do not escape it from the closure for later use.
Makes me think that the whole construct may not be atomic.
There was a problem hiding this comment.
I did check the compiler source and generated object code in #342. The compiler differentiates kinds of variable storage, and maps them into different access semantics. The most relevant access semantic to this is DirectToStorage. On top of this, withUnsafeMutablePointer is just the unsafe form of the inout model.
IOW as long as it is stored property that is statically dispatched or a variable, it is atomic.
There was a problem hiding this comment.
But I'm not sure this is something we should rely on. 😕
This is the danger of OSS: can you depend on undocumented implementation details?
There was a problem hiding this comment.
I don't see this bit being dropped in the foreseeable future. It is essential, well-known part of the inout model since Swift 1.0. Better yet, Swift is OSS, so you can also say that the compiler source code already serves as the documentation, and we would always know ahead if anything could break it.
😅
Replace the manual allocation of
UnsafeAtomicStatewithwithUnsafeMutablePointer.