Make `ReentrantMutex` movable and `const`
As `MovableMutex` is now `const`, it can be used to simplify the implementation and interface of the internal reentrant mutex type. Consequently, the standard error stream does not need to be wrapped in `OnceLock` and `OnceLock::get_or_init_pin()` can be removed.
Uplift the `let_underscore` lints from clippy into rustc.
This PR resolves#97241.
This PR adds three lints from clippy--`let_underscore_drop`, `let_underscore_lock`, and `let_underscore_must_use`, which are meant to capture likely-incorrect uses of `let _ = ...` bindings (in particular, doing this on a type with a non-trivial `Drop` causes the `Drop` to occur immediately, instead of at the end of the scope. For a type like `MutexGuard`, this effectively releases the lock immediately, which is almost certainly the wrong behavior)
In porting the lints from clippy I had to copy over a bunch of utility functions from `clippy_util` that these lints also relied upon. Is that the right approach?
Note that I've set the `must_use` and `drop` lints to Allow by default and set `lock` to Deny by default (this matches the same settings that clippy has). In talking with `@estebank` he informed me to do a Crater run (I am not sure what type of Crater run to request here--I think it's just "check only"?)
On the linked issue, there's some discussion about using `must_use` and `Drop` together as a heuristic for when to warn--I did not implement this yet.
r? `@estebank`
[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: https://github.com/Actyx/sync_wrapper/pull/2)
- and forwarding impls, starting with derivable ones and `Future`
Make RwLockReadGuard covariant
Hi, first time contributor here, if anything is not as expected, please let me know.
`RwLockReadGoard`'s type constructor is invariant. Since it behaves like a smart pointer to an immutable reference, there is no reason that it should not be covariant. Take e.g.
```
fn test_read_guard_covariance() {
fn do_stuff<'a>(_: RwLockReadGuard<'_, &'a i32>, _: &'a i32) {}
let j: i32 = 5;
let lock = RwLock::new(&j);
{
let i = 6;
do_stuff(lock.read().unwrap(), &i);
}
drop(lock);
}
```
where the compiler complains that &i doesn't live long enough. If `RwLockReadGuard` is covariant, then the above code is accepted because the lifetime can be shorter than `'a`.
In order for `RwLockReadGuard` to be covariant, it can't contain a full reference to the `RwLock`, which can never be covariant (because it exposes a mutable reference to the underlying data structure). By reducing the data structure to the required pieces of `RwLock`, the rest falls in place.
If there is a better way to do a test that tests successful compilation, please let me know.
Fixes#80392
once cell renamings
This PR does the renamings proposed in https://github.com/rust-lang/rust/issues/74465#issuecomment-1153703128
- Move/rename `lazy::{OnceCell, Lazy}` to `cell::{OnceCell, LazyCell}`
- Move/rename `lazy::{SyncOnceCell, SyncLazy}` to `sync::{OnceLock, LazyLock}`
(I used `Lazy...` instead of `...Lazy` as it seems to be more consistent, easier to pronounce, etc)
```@rustbot``` label +T-libs-api -T-libs
`Mutex::lock()` and `RwLock::write()` are poison-guarded against panics,
in that they set the poison flag if a panic occurs while they're locked.
But if we're already in a panic (`thread::panicking()`), they leave the
poison flag alone.
That check is a bit of a waste for methods that never set the poison
flag though, namely `get_mut()`, `into_inner()`, and `RwLock::read()`.
These use-cases are now split to avoid that unnecessary call.
Remove `#[rustc_deprecated]`
This removes `#[rustc_deprecated]` and introduces diagnostics to help users to the right direction (that being `#[deprecated]`). All uses of `#[rustc_deprecated]` have been converted. CI is expected to fail initially; this requires #95958, which includes converting `stdarch`.
I plan on following up in a short while (maybe a bootstrap cycle?) removing the diagnostics, as they're only intended to be short-term.
Since https://github.com/rust-lang/rust/pull/95340 landed, Miri with
-Zmiri-check-number-validity produces an error on the test suites of
some crates which implement concurrency tools, because it seems like
such crates tend to use std::sync::mpsc in their tests. This fixes the
problem by storing pointer bytes in a pointer.
This updates the standard library's documentation to use the new syntax. The
documentation is worthwhile to update as it should be more idiomatic
(particularly for features like this, which are nice for users to get acquainted
with). The general codebase is likely more hassle than benefit to update: it'll
hurt git blame, and generally updates can be done by folks updating the code if
(and when) that makes things more readable with the new format.
A few places in the compiler and library code are updated (mostly just due to
already having been done when this commit was first authored).
The documentation on `std::sync::mpsc::Iter` and `std::sync::mpsc::TryIter` provides links to the corresponding `Receiver` methods, unlike `std::sync::mpsc::IntoIter` does.
This was left out in c59b188aae
Related to #29377
Remove unnecessary condition in Barrier::wait()
This is my first pull request for Rust, so feel free to call me out if anything is amiss.
After some examination, I realized that the second condition of the "spurious-wakeup-handler" loop in ``std::sync::Barrier::wait()`` should always evaluate to ``true``, making it redundant in the ``&&`` expression.
Here is the affected function before the fix:
```rust
#[stable(feature = "rust1", since = "1.0.0")]
pub fn wait(&self) -> BarrierWaitResult {
let mut lock = self.lock.lock().unwrap();
let local_gen = lock.generation_id;
lock.count += 1;
if lock.count < self.num_threads {
// We need a while loop to guard against spurious wakeups.
// https://en.wikipedia.org/wiki/Spurious_wakeup
while local_gen == lock.generation_id && lock.count < self.num_threads { // fixme
lock = self.cvar.wait(lock).unwrap();
}
BarrierWaitResult(false)
} else {
lock.count = 0;
lock.generation_id = lock.generation_id.wrapping_add(1);
self.cvar.notify_all();
BarrierWaitResult(true)
}
}
```
At first glance, it seems that the check that ``lock.count < self.num_threads`` would be necessary in order for a thread A to detect when another thread B has caused the barrier to reach its thread count, making thread B the "leader".
However, the control flow implicitly results in an invariant that makes observing ``!(lock.count < self.num_threads)``, i.e. ``lock.count >= self.num_threads`` impossible from thread A.
When thread B, which will be the leader, calls ``.wait()`` on this shared instance of the ``Barrier``, it locks the mutex in the first line and saves the ``MutexGuard`` in the ``lock`` variable. It then increments the value of ``lock.count``. However, it then proceeds to check if ``lock.count < self.num_threads``. Since it is the leader, it is the case that (after the increment of ``lock.count``), the lock count is *equal* to the number of threads. Thus, the second branch is immediately taken and ``lock.count`` is zeroed. Additionally, the generation ID is incremented (with wrap). Then, the condition variable is signalled. But, the other threads are waiting at the line ``lock = self.cvar.wait(lock).unwrap();``, so they cannot resume until thread B's call to ``Barrier::wait()`` returns, which drops the ``MutexGuard`` acquired in the first ``let`` statement and unlocks the mutex.
The order of events is thus:
1. A thread A calls `.wait()`
2. `.wait()` acquires the mutex, increments `lock.count`, and takes the first branch
3. Thread A enters the ``while`` loop since the generation ID has not changed and the count is less than the number of threads for the ``Barrier``
3. Spurious wakeups occur, but both conditions hold, so the thread A waits on the condition variable
4. This process repeats for N - 2 additional times for non-leader threads A'
5. *Meanwhile*, Thread B calls ``Barrier::wait()`` on the same barrier that threads A, A', A'', etc. are waiting on. The thread count reaches the number of threads for the ``Barrier``, so all threads should now proceed, with B being the leader. B acquires the mutex and increments the value ``lock.count`` only to find that it is not less than ``self.num_threads``. Thus, it immediately clamps ``self.num_threads`` back down to 0 and increments the generation. Then, it signals the condvar to tell the A (prime) threads that they may continue.
6. The A, A', A''... threads wake up and attempt to re-acquire the ``lock`` as per the internal operation of a condition variable. When each A has exclusive access to the mutex, it finds that ``lock.generation_id`` no longer matches ``local_generation`` **and the ``&&`` expression short-circuits -- and even if it were to evaluate it, ``self.count`` is definitely less than ``self.num_threads`` because it has been reset to ``0`` by thread B *before* B dropped its ``MutexGuard``**.
Therefore, it my understanding that it would be impossible for the non-leader threads to ever see the second boolean expression evaluate to anything other than ``true``. This PR simply removes that condition.
Any input would be appreciated. Sorry if this is terribly verbose. I'm new to the Rust community and concurrency can be hard to explain in words. Thanks!
