Add extra access methods for atomic types #1649

Open
wants to merge 2 commits into from

6 participants

@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 alexcrichton added the T-libs label Jun 20, 2016
@alexcrichton
The Rust Programming Language member

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
The Rust Programming Language member

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

@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 aturon self-assigned this Jul 18, 2016
@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.

@aturon aturon added the I-nominated label Jul 25, 2016
@alexcrichton
The Rust Programming Language member

🔔 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment