diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2022-03-08 11:04:51 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-03-08 11:04:51 +0100 |
| commit | aec535f8051c7cf00b1e373a05893b2cb580eeac (patch) | |
| tree | f3de1f8f634c093f02e57983faa811a872bd155f /library/std/src/thread | |
| parent | e22331ce02d9847a0837bddc074f0e7b5d34db13 (diff) | |
| parent | a3d269e91cceb4d6234668014930f81fff587ff3 (diff) | |
| download | rust-aec535f8051c7cf00b1e373a05893b2cb580eeac.tar.gz rust-aec535f8051c7cf00b1e373a05893b2cb580eeac.zip | |
Rollup merge of #94559 - m-ou-se:thread-scope-spawn-closure-without-arg, r=Mark-Simulacrum
Remove argument from closure in thread::Scope::spawn.
This implements ```@danielhenrymantilla's``` [suggestion](https://github.com/rust-lang/rust/issues/93203#issuecomment-1040798286) for improving the scoped threads interface.
Summary:
The `Scope` type gets an extra lifetime argument, which represents basically its own lifetime that will be used in `&'scope Scope<'scope, 'env>`:
```diff
- pub struct Scope<'env> { .. };
+ pub struct Scope<'scope, 'env: 'scope> { .. }
pub fn scope<'env, F, T>(f: F) -> T
where
- F: FnOnce(&Scope<'env>) -> T;
+ F: for<'scope> FnOnce(&'scope Scope<'scope, 'env>) -> T;
```
This simplifies the `spawn` function, which now no longer passes an argument to the closure you give it, and now uses the `'scope` lifetime for everything:
```diff
- pub fn spawn<'scope, F, T>(&'scope self, f: F) -> ScopedJoinHandle<'scope, T>
+ pub fn spawn<F, T>(&'scope self, f: F) -> ScopedJoinHandle<'scope, T>
where
- F: FnOnce(&Scope<'env>) -> T + Send + 'env,
+ F: FnOnce() -> T + Send + 'scope,
- T: Send + 'env;
+ T: Send + 'scope;
```
The only difference the user will notice, is that their closure now takes no arguments anymore, even when spawning threads from spawned threads:
```diff
thread::scope(|s| {
- s.spawn(|_| {
+ s.spawn(|| {
...
});
- s.spawn(|s| {
+ s.spawn(|| {
...
- s.spawn(|_| ...);
+ s.spawn(|| ...);
});
});
```
<details><summary>And, as a bonus, errors get <em>slightly</em> better because now any lifetime issues point to the outermost <code>s</code> (since there is only one <code>s</code>), rather than the innermost <code>s</code>, making it clear that the lifetime lasts for the entire <code>thread::scope</code>.
</summary>
```diff
error[E0373]: closure may outlive the current function, but it borrows `a`, which is owned by the current function
--> src/main.rs:9:21
|
- 7 | s.spawn(|s| {
- | - has type `&Scope<'1>`
+ 6 | thread::scope(|s| {
+ | - lifetime `'1` appears in the type of `s`
9 | s.spawn(|| println!("{:?}", a)); // might run after `a` is dropped
| ^^ - `a` is borrowed here
| |
| may outlive borrowed value `a`
|
note: function requires argument type to outlive `'1`
--> src/main.rs:9:13
|
9 | s.spawn(|| println!("{:?}", a)); // might run after `a` is dropped
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to force the closure to take ownership of `a` (and any other referenced variables), use the `move` keyword
|
9 | s.spawn(move || println!("{:?}", a)); // might run after `a` is dropped
| ++++
"
```
</details>
The downside is that the signature of `scope` and `Scope` gets slightly more complex, but in most cases the user wouldn't need to write those, as they just use the argument provided by `thread::scope` without having to name its type.
Another downside is that this does not work nicely in Rust 2015 and Rust 2018, since in those editions, `s` would be captured by reference and not by copy. In those editions, the user would need to use `move ||` to capture `s` by copy. (Which is what the compiler suggests in the error.)
Diffstat (limited to 'library/std/src/thread')
| -rw-r--r-- | library/std/src/thread/scoped.rs | 44 |
1 files changed, 23 insertions, 21 deletions
diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index ea9623be63b..4af58f1a380 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -9,23 +9,24 @@ use crate::sync::Arc; /// A scope to spawn scoped threads in. /// /// See [`scope`] for details. -pub struct Scope<'env> { +pub struct Scope<'scope, 'env: 'scope> { data: ScopeData, - /// Invariance over 'env, to make sure 'env cannot shrink, + /// Invariance over 'scope, to make sure 'scope cannot shrink, /// which is necessary for soundness. /// /// Without invariance, this would compile fine but be unsound: /// - /// ```compile_fail + /// ```compile_fail,E0373 /// #![feature(scoped_threads)] /// /// std::thread::scope(|s| { - /// s.spawn(|s| { + /// s.spawn(|| { /// let a = String::from("abcd"); - /// s.spawn(|_| println!("{:?}", a)); // might run after `a` is dropped + /// s.spawn(|| println!("{:?}", a)); // might run after `a` is dropped /// }); /// }); /// ``` + scope: PhantomData<&'scope mut &'scope ()>, env: PhantomData<&'env mut &'env ()>, } @@ -88,12 +89,12 @@ impl ScopeData { /// let mut x = 0; /// /// thread::scope(|s| { -/// s.spawn(|_| { +/// s.spawn(|| { /// println!("hello from the first scoped thread"); /// // We can borrow `a` here. /// dbg!(&a); /// }); -/// s.spawn(|_| { +/// s.spawn(|| { /// println!("hello from the second scoped thread"); /// // We can even mutably borrow `x` here, /// // because no other threads are using it. @@ -109,7 +110,7 @@ impl ScopeData { #[track_caller] pub fn scope<'env, F, T>(f: F) -> T where - F: FnOnce(&Scope<'env>) -> T, + F: for<'scope> FnOnce(&'scope Scope<'scope, 'env>) -> T, { let scope = Scope { data: ScopeData { @@ -118,6 +119,7 @@ where a_thread_panicked: AtomicBool::new(false), }, env: PhantomData, + scope: PhantomData, }; // Run `f`, but catch panics so we can make sure to wait for all the threads to join. @@ -138,7 +140,7 @@ where } } -impl<'env> Scope<'env> { +impl<'scope, 'env> Scope<'scope, 'env> { /// Spawns a new thread within a scope, returning a [`ScopedJoinHandle`] for it. /// /// Unlike non-scoped threads, threads spawned with this function may @@ -163,10 +165,10 @@ impl<'env> Scope<'env> { /// to recover from such errors. /// /// [`join`]: ScopedJoinHandle::join - pub fn spawn<'scope, F, T>(&'scope self, f: F) -> ScopedJoinHandle<'scope, T> + pub fn spawn<F, T>(&'scope self, f: F) -> ScopedJoinHandle<'scope, T> where - F: FnOnce(&Scope<'env>) -> T + Send + 'env, - T: Send + 'env, + F: FnOnce() -> T + Send + 'scope, + T: Send + 'scope, { Builder::new().spawn_scoped(self, f).expect("failed to spawn thread") } @@ -196,7 +198,7 @@ impl Builder { /// thread::scope(|s| { /// thread::Builder::new() /// .name("first".to_string()) - /// .spawn_scoped(s, |_| + /// .spawn_scoped(s, || /// { /// println!("hello from the {:?} scoped thread", thread::current().name()); /// // We can borrow `a` here. @@ -205,7 +207,7 @@ impl Builder { /// .unwrap(); /// thread::Builder::new() /// .name("second".to_string()) - /// .spawn_scoped(s, |_| + /// .spawn_scoped(s, || /// { /// println!("hello from the {:?} scoped thread", thread::current().name()); /// // We can even mutably borrow `x` here, @@ -222,14 +224,14 @@ impl Builder { /// ``` pub fn spawn_scoped<'scope, 'env, F, T>( self, - scope: &'scope Scope<'env>, + scope: &'scope Scope<'scope, 'env>, f: F, ) -> io::Result<ScopedJoinHandle<'scope, T>> where - F: FnOnce(&Scope<'env>) -> T + Send + 'env, - T: Send + 'env, + F: FnOnce() -> T + Send + 'scope, + T: Send + 'scope, { - Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(|| f(scope), Some(&scope.data)) }?)) + Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(f, Some(&scope.data)) }?)) } } @@ -244,7 +246,7 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { /// use std::thread; /// /// thread::scope(|s| { - /// let t = s.spawn(|_| { + /// let t = s.spawn(|| { /// println!("hello"); /// }); /// println!("thread id: {:?}", t.thread().id()); @@ -277,7 +279,7 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { /// use std::thread; /// /// thread::scope(|s| { - /// let t = s.spawn(|_| { + /// let t = s.spawn(|| { /// panic!("oh no"); /// }); /// assert!(t.join().is_err()); @@ -302,7 +304,7 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { } } -impl<'env> fmt::Debug for Scope<'env> { +impl fmt::Debug for Scope<'_, '_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Scope") .field("num_running_threads", &self.data.num_running_threads.load(Ordering::Relaxed)) |
