From 24fe35a3e1858a6b8441847729e233fa8144996a Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 3 Mar 2022 13:04:14 +0100 Subject: [PATCH 1/4] Remove argument from closure in thread::Scope::spawn. --- library/std/src/thread/scoped.rs | 44 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index 9dd7c15fc5922..e066f97c8e659 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(&'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> 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)) }?)) } } @@ -245,7 +247,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()); @@ -279,7 +281,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()); @@ -299,7 +301,7 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { } } -impl<'env> fmt::Debug for Scope<'env> { +impl<'scope, 'env> fmt::Debug for Scope<'scope, 'env> { 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)) From 6b46a52577e3fb18a45527a105a16b216b0e45f7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 3 Mar 2022 14:58:49 +0100 Subject: [PATCH 2/4] Fix doctests. --- library/core/src/sync/atomic.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 2b8bbe1924450..2da04ab2cea7d 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -352,7 +352,7 @@ impl AtomicBool { /// let a = &*AtomicBool::from_mut_slice(&mut some_bools); /// std::thread::scope(|s| { /// for i in 0..a.len() { - /// s.spawn(move |_| a[i].store(true, Ordering::Relaxed)); + /// s.spawn(move || a[i].store(true, Ordering::Relaxed)); /// } /// }); /// assert_eq!(some_bools, [true; 10]); @@ -984,7 +984,7 @@ impl AtomicPtr { /// let a = &*AtomicPtr::from_mut_slice(&mut some_ptrs); /// std::thread::scope(|s| { /// for i in 0..a.len() { - /// s.spawn(move |_| { + /// s.spawn(move || { /// let name = Box::new(format!("thread{i}")); /// a[i].store(Box::into_raw(name), Ordering::Relaxed); /// }); @@ -1533,7 +1533,7 @@ macro_rules! atomic_int { #[doc = concat!("let a = &*", stringify!($atomic_type), "::from_mut_slice(&mut some_ints);")] /// std::thread::scope(|s| { /// for i in 0..a.len() { - /// s.spawn(move |_| a[i].store(i as _, Ordering::Relaxed)); + /// s.spawn(move || a[i].store(i as _, Ordering::Relaxed)); /// } /// }); /// for (i, n) in some_ints.into_iter().enumerate() { From 9099353ea88c72cb5a62fd7c2af187410295cf5b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 4 Mar 2022 10:41:39 +0000 Subject: [PATCH 3/4] Use '_ for irrelevant lifetimes in Debug impl. Co-authored-by: Daniel Henry-Mantilla --- library/std/src/thread/scoped.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index e066f97c8e659..35082495b18b2 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -301,7 +301,7 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { } } -impl<'scope, 'env> fmt::Debug for Scope<'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)) From a3d269e91cceb4d6234668014930f81fff587ff3 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 7 Mar 2022 22:14:02 +0000 Subject: [PATCH 4/4] Use `f` instead of `|| f()`. Co-authored-by: Mark Rousskov --- library/std/src/thread/scoped.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index 35082495b18b2..ee2d0e243d428 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -231,7 +231,7 @@ impl Builder { F: FnOnce() -> T + Send + 'scope, T: Send + 'scope, { - Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(|| f(), Some(&scope.data)) }?)) + Ok(ScopedJoinHandle(unsafe { self.spawn_unchecked_(f, Some(&scope.data)) }?)) } }