Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Add Atomic*::from_mut_slice #94384

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Add Atomic*::from_mut_slice #94384

merged 1 commit into from
Mar 1, 2022

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Feb 25, 2022

Tracking issue #76314 for from_mut has a question about the possibility of from_mut_slice, and I found a real case for it. A user in the forum had a parallelism problem that could be solved by open-indexing updates to a vector of atomics, but they didn't want to affect the other code using that vector. Using from_mut_slice, they could borrow that data as atomics just long enough for their parallel loop.

ref: https://users.rust-lang.org/t/sharing-vector-with-rayon-par-iter-correctly/72022

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2022
@Mark-Simulacrum
Copy link
Member

Generally makes sense to me, though feels a bit unfortunate that we need a separate function - it seems like if you have stateless &mut T -> &mut U, you can do &mut [T] -> &mut [U], right?

Probably not worth not doing this, but it feels like maybe there's a missing abstraction of some kind here - feels similar to https://doc.rust-lang.org/nightly/std/cell/struct.Cell.html#method.as_slice_of_cells in some ways.

r? @dtolnay for libs-api sign off

@cuviper
Copy link
Member Author

cuviper commented Feb 26, 2022

it seems like if you have stateless &mut T -> &mut U, you can do &mut [T] -> &mut [U], right?

Yes, the constraint is almost like T: AsMut<U>, but they have to be the same size and return the same address, no dereferencing. It's similar to coercion too. Maybe something like unsafe trait CoerceMut<T> with a blanket impl for slices?

dtolnay
dtolnay approved these changes Feb 28, 2022
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@dtolnay
Copy link
Member

dtolnay commented Feb 28, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Feb 28, 2022

📌 Commit d3d2a27 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2022
@dtolnay dtolnay mentioned this pull request Feb 28, 2022
Open
6 tasks
@Dylan-DPC Dylan-DPC mentioned this pull request Mar 1, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2022
@bors
Auto merge of rust-lang#94469 - Dylan-DPC:rollup-2tcq6s5, r=Dylan-DPC
6343edf 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#91545 (Generalize "remove `&`"  and "add `*`" suggestions to more than one deref)
 - rust-lang#93385 (Rustdoc ty consistency fixes)
 - rust-lang#93926 (Lint against more useless `#[must_use]` attributes)
 - rust-lang#94094 (use BOOL for TCP_NODELAY setsockopt value on Windows)
 - rust-lang#94384 (Add Atomic*::from_mut_slice)
 - rust-lang#94448 (5 - Make more use of `let_chains`)
 - rust-lang#94452 (Sync portable-simd for bitmasks &c.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5bd119d into rust-lang:master Mar 1, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 1, 2022
@m-ou-se
Copy link
Member

m-ou-se commented Mar 3, 2022

:D Coincidentally I just got to my to do list item that says "Add atomic from_mut_slice" and noticed you just added it a few days ago. Nice!

@WaffleLapkin
Copy link
Member

Should we also introduce the inverse operation? i.e. &mut [Atomic*] -> &mut [*]?

just like Atomic*::from_mut has an inverse Atomic*::get_mut.

@cuviper
Copy link
Member Author

cuviper commented Mar 10, 2022

@WaffleLapkin that's a bit awkward because I don't think we can have a &mut [self] method. So it would either have to be a method on actual slice impls, or else an explicit associated method like Atomic*::get_mut_slice(this: &mut [Self]).

@WaffleLapkin
Copy link
Member

For me an explicit associated method seems fine. Can make a PR 👀

@WaffleLapkin WaffleLapkin mentioned this pull request Mar 10, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2022
@matthiaskrgr
Rollup merge of rust-lang#94816 - WaffleLapkin:atomic_get_mut_slice, …
8dad2d1 
…r=Mark-Simulacrum

Add `Atomic*::get_mut_slice`

This PR adds the inverse of `Atomic*::from_mut_slice` introduced in rust-lang#94384 with the following API:
```rust
// core::sync::atomic

impl Atomic* {
    fn get_mut_slice(this: &mut [Self]) -> &mut [*];
}
```

cc `@cuviper`

-----

For now I've used the same tracking issue as `Atomic*::from_mut_slice`, should I open a new one?
@cuviper cuviper deleted the atomic-slice branch April 12, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@cuviper @rust-highfive @Mark-Simulacrum @dtolnay @bors @m-ou-se @WaffleLapkin @rustbot