Add extra access methods for atomic types

[Amanieu]

This RFC adds the following methods to atomic types:

• get_mut
• into_inner
• as_raw
• from_raw

Rendered

[durka]

All the other methods for loading the values of atomic types take an argument to specify the ordering. Why not these?

[durka]

What is required for code to be sound while calling the unsafe from_raw? Just a valid pointer or are there more invariants? If so why not take a reference and make it a safe fn? Same question for as_raw's return value, I guess.

[Amanieu]

All the other methods for loading the values of atomic types take an argument to specify the ordering. Why not these?

Because they are accessing the value non-atomically.

What is required for code to be sound while calling the unsafe from_raw?

The pointer needs to be valid, and you need to make sure that the other thread is not accessing the integer non-atomically while you are performing atomic operations on it.

The reason a raw pointer is used here is because the value is a shared and mutable integer, which can't be expressed using a reference.

[alexcrichton]

The get_mut, into_inner, and as_raw methods seem good to me, but the from_raw method would basically tie our hands into always representing AtomicT as simply UnsafeCell<T>. This is likely to always be the case anyway, but it may mean that it'd be easier to just document that the two are the same and not worry about adding a method to do what to me seems at least a relatively rare operation.

[Amanieu]

@alexcrichton I think simply documenting that AtomicT has the same representation as T would also work fine. This is going to be used in unsafe code anyways, the only thing needed is a guarantee about the layout of atomic types.

[ubsan]

Aren't there platforms where we'd have to use AtomicUsize to emulate smaller atomics?

[alexcrichton]

@Amanieu yeah I might prefer to go that route (a function from *const T to &U seems kinda weird to me) and avoid the as_raw and from_raw methods. I think both are equivalent in terms of the practical and technical implications.

@ubsan yeah but AtomicU8 can always be byte-sized regardless of that, we'd just have to use usize-sized compare-and-swap operations to update the byte we care about (while ignoring all the other bytes)

[ubsan]

@alexcrichton ah, 'k.

[glaebhoerl]

@alexcrichton Hmm wouldn't that misbehave if you have like AtomicU8s in an array or such?

[Amanieu]

@glaebhoerl It isn't a problem since a modification to one AtomicU8 will simply cause a CAS on a nearby one to failed, which will just be retried.

[aturon]

OK, after giving this RFC some though, I'm in favor of the general idea. In particular, I've wanted something like get_mut from time to time, for cases where I'm initializing an atomic data structure.

I personally would prefer to start with just get_mut, since into_inner is expressible in terms of it, but I don't feel too too strongly about that.

[Amanieu]

@aturon The reason for adding into_inner was to be able to consume an atomic that wasn't declared with a mutable binding. In practice I expect 99% of uses of get_mut to be inside drop which gives you a &mut anyways, so it wouldn't be too much of a loss to get rid of it.

[alexcrichton]

🔔 This RFC is now entering its week-long final comment period 🔔

The libs team is leaning towards merging this as is (with get_mut and into_inner), but comments are always appreciated!

[alexcrichton]

Discussed during libs triage the other day, the conclusion was to merge

Thanks again for the RFC @Amanieu!

Tracking issue

[willmo]

I still get an improper_ctypes warning when I try to use an atomic for FFI. Is this expected?

(Relative Rust noob here; apologies if this is a dumb question or an inappropriate place/way to comment.)

[nagisa]

Atomic* are just your average Rust struct, which have unspecified layout, so the error is expected.

What this RFC is proposing is ability to extract plain isize from your AtomicIsize, which you can then put into any of your FFI structs. Alas, the functionality in question does not exist yet, because even though this RFC is accepted, the corresponding functionality is not implemented. You can track the implementation on the tracking issue.

[Amanieu]

Actually, it has already been implemented: rust-lang/rust#35719

[willmo]

@nagisa, it seems to me that with this RFC the layout of Atomic* is now specified:

It also specifies that the layout of an AtomicT type is always the same as the underlying T type. So, for example, AtomicI32 is guaranteed to be transmutable to and from i32.

But the compiler has no way of knowing this.

Apologies that this wasn't clear from my earlier example, but as in the example of Linux futexes, I need to have Rust atomically manipulate a value shared with C. So it seems like the correct solution is a pointer cast, and the effect of this RFC is that this is now defined/correct? (That's another contrived example, but closer to what I need to do.)

But this is not nearly as nice as being able to use Atomic* directly in my #[repr(C)] struct. For one thing, aside from visibility (which I would certainly use in a real example), nothing forces me to manipulate the field atomically. In fact, manipulating it non-atomically (incorrectly) is easy and involves no unsafe, while manipulating it atomically (correctly) requires much more typing and use of unsafe.

I can't even use UnsafeCell<isize>, which would at least force me to use unsafe and indicate to the compiler that my struct has interior mutability, because UnsafeCell is not #[repr(C)]. Nor should it be, of course. But it would be super cool if UnsafeCell had some kind of #[repr(inherit)] or something, that would allow UnsafeCell<T> to be used in a #[repr(C)] struct iff T could be used in a #[repr(C)] struct.

[Amanieu]

My opinion is that structs with only a single member should be defined as having the same representation as that element.

As a workaround, I guess we could just mark the atomic types as #[repr(C)] for now and ignore the warning about UnsafeCell not being #[repr(C)].