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.

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

Rollup of 5 pull requests #98703

Closed
wants to merge 11 commits into from
Closed

Conversation

Dylan-DPC
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

guswynn and others added 11 commits June 7, 2022 13:10
[core] add `Exclusive` to sync

(discussed here: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Adding.20.60SyncWrapper.60.20to.20std)

`Exclusive` is a wrapper that exclusively allows mutable access to the inner value if you have exclusive access to the wrapper. It acts like a compile time mutex, and hold an unconditional `Sync` implementation.

## Justification for inclusion into std
- This wrapper unblocks actual problems:
  - The example that I hit was a vector of `futures::future::BoxFuture`'s causing a central struct in a script to be non-`Sync`. To work around it, you either write really difficult code, or wrap the futures in a needless mutex.
- Easy to maintain: this struct is as simple as a wrapper can get, and its `Sync` implementation has very clear reasoning
- Fills a gap: `&/&mut` are to `RwLock` as `Exclusive` is to `Mutex`

## Public Api
```rust
// core::sync
#[derive(Default)]
struct Exclusive<T: ?Sized> { ... }

impl<T: ?Sized> Sync for Exclusive {}

impl<T> Exclusive<T> {
    pub const fn new(t: T) -> Self;
    pub const fn into_inner(self) -> T;
}

impl<T: ?Sized> Exclusive<T> {
    pub const fn get_mut(&mut self) -> &mut T;
    pub const fn get_pin_mut(Pin<&mut self>) -> Pin<&mut T>;
    pub const fn from_mut(&mut T) -> &mut Exclusive<T>;
    pub const fn from_pin_mut(Pin<&mut T>) -> Pin<&mut Exclusive<T>>;
}

impl<T: Future> Future for Exclusive { ... }

impl<T> From<T> for Exclusive<T> { ... }
impl<T: ?Sized> Debug for Exclusive { ... }
```

## Naming
This is a big bikeshed, but I felt that `Exclusive` captured its general purpose quite well.

## Stability and location
As this is so simple, it can be in `core`. I feel that it can be stabilized quite soon after it is merged, if the libs teams feels its reasonable to add. Also, I don't really know how unstable feature work in std/core's codebases, so I might need help fixing them

## Tips for review
The docs probably are the thing that needs to be reviewed! I tried my best, but I'm sure people have more experience than me writing docs for `Core`

### Implementation:
The API is mostly pulled from https://docs.rs/sync_wrapper/latest/sync_wrapper/struct.SyncWrapper.html (which is apache 2.0 licenesed), and the implementation is trivial:
- its an unsafe justification for pinning
- its an unsafe justification for the `Sync` impl (mostly reasoned about by `@danielhenrymantilla` here: Actyx/sync_wrapper#2)
- and forwarding impls, starting with derivable ones and `Future`
fix data race in thread::scope

Puts the `ScopeData` into an `Arc` so it sticks around as long as we need it.
This means one extra `Arc::clone` per spawned scoped thread, which I hope is fine.

Fixes rust-lang#98498
r? ``@m-ou-se``
…ler-errors

Factor out `hir::Node::Binding`
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jun 30, 2022
@Dylan-DPC
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Jun 30, 2022

📌 Commit db4dd51 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 30, 2022
@bors
Copy link
Contributor

bors commented Jun 30, 2022

⌛ Testing commit db4dd51 with merge 4cd5f0d89087b03ce3508a6544518cb685b43141...

@bors
Copy link
Contributor

bors commented Jun 30, 2022

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 30, 2022
@Dylan-DPC Dylan-DPC closed this Jun 30, 2022
@Dylan-DPC Dylan-DPC deleted the rollup-j95v4t1 branch June 30, 2022 16:33
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants