From 51eb8427bf3887323a77d4dd56bf4557f80fdabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 19 Aug 2023 17:26:54 +0200 Subject: [PATCH 1/5] Make parallel! an expression --- compiler/rustc_data_structures/src/sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 083aa6cf69747..c46ae9a65da12 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -203,7 +203,7 @@ cfg_if! { #[macro_export] macro_rules! parallel { - ($($blocks:block),*) => { + ($($blocks:block),*) => {{ // We catch panics here ensuring that all the blocks execute. // This makes behavior consistent with the parallel compiler. let mut panic = None; @@ -219,7 +219,7 @@ cfg_if! { if let Some(panic) = panic { ::std::panic::resume_unwind(panic); } - } + }} } pub fn par_for_each_in(t: T, mut for_each: impl FnMut(T::Item) + Sync + Send) { From b56acac41d2abbc9afc91fec370b574be49d5c6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 3 Jan 2020 01:51:30 +0100 Subject: [PATCH 2/5] Add `ParallelGuard` type to handle unwinding in parallel sections --- compiler/rustc_data_structures/src/sync.rs | 189 ++++++++------------- 1 file changed, 68 insertions(+), 121 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index c46ae9a65da12..8defcd39ecdd6 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -41,6 +41,7 @@ //! [^2] `MTLockRef` is a typedef. pub use crate::marker::*; +use std::any::Any; use std::collections::HashMap; use std::hash::{BuildHasher, Hash}; use std::ops::{Deref, DerefMut}; @@ -103,6 +104,37 @@ mod mode { pub use mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode}; +/// A guard used to hold panics that occur during a parallel section to later by unwound. +/// This is used for the parallel compiler to prevent fatal errors from non-deterministically +/// hiding errors by ensuring that everything in the section has completed executing before +/// continuing with unwinding. It's also used for the non-parallel code to ensure error message +/// output match the parallel compiler for testing purposes. +pub struct ParallelGuard { + panic: Lock>>, +} + +impl ParallelGuard { + #[inline] + pub fn new() -> Self { + ParallelGuard { panic: Lock::new(None) } + } + + pub fn run(&self, f: impl FnOnce() -> R) -> Option { + catch_unwind(AssertUnwindSafe(f)) + .map_err(|err| { + *self.panic.lock() = Some(err); + }) + .ok() + } + + #[inline] + pub fn unwind(self) { + if let Some(panic) = self.panic.into_inner() { + resume_unwind(panic); + } + } +} + cfg_if! { if #[cfg(not(parallel_compiler))] { use std::ops::Add; @@ -198,66 +230,37 @@ cfg_if! { where A: FnOnce() -> RA, B: FnOnce() -> RB { - (oper_a(), oper_b()) + let guard = ParallelGuard::new(); + let a = guard.run(oper_a); + let b = guard.run(oper_b); + guard.unwind(); + (a.unwrap(), b.unwrap()) } #[macro_export] macro_rules! parallel { ($($blocks:block),*) => {{ - // We catch panics here ensuring that all the blocks execute. - // This makes behavior consistent with the parallel compiler. - let mut panic = None; - $( - if let Err(p) = ::std::panic::catch_unwind( - ::std::panic::AssertUnwindSafe(|| $blocks) - ) { - if panic.is_none() { - panic = Some(p); - } - } - )* - if let Some(panic) = panic { - ::std::panic::resume_unwind(panic); - } + let mut guard = $crate::sync::ParallelGuard::new(); + $(guard.run(|| $blocks);)* + guard.unwind(); }} } pub fn par_for_each_in(t: T, mut for_each: impl FnMut(T::Item) + Sync + Send) { - // We catch panics here ensuring that all the loop iterations execute. - // This makes behavior consistent with the parallel compiler. - let mut panic = None; + let guard = ParallelGuard::new(); t.into_iter().for_each(|i| { - if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { - if panic.is_none() { - panic = Some(p); - } - } + guard.run(|| for_each(i)); }); - if let Some(panic) = panic { - resume_unwind(panic); - } + guard.unwind(); } pub fn par_map>( t: T, mut map: impl FnMut(<::IntoIter as Iterator>::Item) -> R, ) -> C { - // We catch panics here ensuring that all the loop iterations execute. - let mut panic = None; - let r = t.into_iter().filter_map(|i| { - match catch_unwind(AssertUnwindSafe(|| map(i))) { - Ok(r) => Some(r), - Err(p) => { - if panic.is_none() { - panic = Some(p); - } - None - } - } - }).collect(); - if let Some(panic) = panic { - resume_unwind(panic); - } + let guard = ParallelGuard::new(); + let r = t.into_iter().filter_map(|i| guard.run(|| map(i))).collect(); + guard.unwind(); r } @@ -380,7 +383,11 @@ cfg_if! { let (a, b) = rayon::join(move || FromDyn::from(oper_a.into_inner()()), move || FromDyn::from(oper_b.into_inner()())); (a.into_inner(), b.into_inner()) } else { - (oper_a(), oper_b()) + let guard = ParallelGuard::new(); + let a = guard.run(oper_a); + let b = guard.run(oper_b); + guard.unwind(); + (a.unwrap(), b.unwrap()) } } @@ -415,28 +422,10 @@ cfg_if! { // of a single threaded rustc. parallel!(impl $fblock [] [$($blocks),*]); } else { - // We catch panics here ensuring that all the blocks execute. - // This makes behavior consistent with the parallel compiler. - let mut panic = None; - if let Err(p) = ::std::panic::catch_unwind( - ::std::panic::AssertUnwindSafe(|| $fblock) - ) { - if panic.is_none() { - panic = Some(p); - } - } - $( - if let Err(p) = ::std::panic::catch_unwind( - ::std::panic::AssertUnwindSafe(|| $blocks) - ) { - if panic.is_none() { - panic = Some(p); - } - } - )* - if let Some(panic) = panic { - ::std::panic::resume_unwind(panic); - } + let guard = $crate::sync::ParallelGuard::new(); + guard.run(|| $fblock); + $(guard.run(|| $blocks);)* + guard.unwind(); } }; } @@ -449,31 +438,17 @@ cfg_if! { ) { if mode::is_dyn_thread_safe() { let for_each = FromDyn::from(for_each); - let panic: Mutex> = Mutex::new(None); - t.into_par_iter().for_each(|i| if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { - let mut l = panic.lock(); - if l.is_none() { - *l = Some(p) - } + let guard = ParallelGuard::new(); + t.into_par_iter().for_each(|i| { + guard.run(|| for_each(i)); }); - - if let Some(panic) = panic.into_inner() { - resume_unwind(panic); - } + guard.unwind(); } else { - // We catch panics here ensuring that all the loop iterations execute. - // This makes behavior consistent with the parallel compiler. - let mut panic = None; + let guard = ParallelGuard::new(); t.into_iter().for_each(|i| { - if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { - if panic.is_none() { - panic = Some(p); - } - } + guard.run(|| for_each(i)); }); - if let Some(panic) = panic { - resume_unwind(panic); - } + guard.unwind(); } } @@ -487,43 +462,15 @@ cfg_if! { map: impl Fn(I) -> R + DynSync + DynSend ) -> C { if mode::is_dyn_thread_safe() { - let panic: Mutex> = Mutex::new(None); let map = FromDyn::from(map); - // We catch panics here ensuring that all the loop iterations execute. - let r = t.into_par_iter().filter_map(|i| { - match catch_unwind(AssertUnwindSafe(|| map(i))) { - Ok(r) => Some(r), - Err(p) => { - let mut l = panic.lock(); - if l.is_none() { - *l = Some(p); - } - None - }, - } - }).collect(); - - if let Some(panic) = panic.into_inner() { - resume_unwind(panic); - } + let guard = ParallelGuard::new(); + let r = t.into_par_iter().filter_map(|i| guard.run(|| map(i))).collect(); + guard.unwind(); r } else { - // We catch panics here ensuring that all the loop iterations execute. - let mut panic = None; - let r = t.into_iter().filter_map(|i| { - match catch_unwind(AssertUnwindSafe(|| map(i))) { - Ok(r) => Some(r), - Err(p) => { - if panic.is_none() { - panic = Some(p); - } - None - } - } - }).collect(); - if let Some(panic) = panic { - resume_unwind(panic); - } + let guard = ParallelGuard::new(); + let r = t.into_iter().filter_map(|i| guard.run(|| map(i))).collect(); + guard.unwind(); r } } From 242805442bb217b0cb8a2b363c3eb301311b33f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 23 Aug 2023 21:13:55 +0200 Subject: [PATCH 3/5] Update failure status --- tests/ui/const-generics/late-bound-vars/in_closure.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/ui/const-generics/late-bound-vars/in_closure.rs b/tests/ui/const-generics/late-bound-vars/in_closure.rs index 4fdf603b05f35..6549cad629b04 100644 --- a/tests/ui/const-generics/late-bound-vars/in_closure.rs +++ b/tests/ui/const-generics/late-bound-vars/in_closure.rs @@ -1,4 +1,4 @@ -// failure-status: 101 +// failure-status: 1 // known-bug: unknown // error-pattern:internal compiler error // normalize-stderr-test "internal compiler error.*" -> "" @@ -22,7 +22,10 @@ #![feature(generic_const_exprs)] #![allow(incomplete_features)] -const fn inner<'a>() -> usize where &'a (): Sized { +const fn inner<'a>() -> usize +where + &'a (): Sized, +{ 3 } From d36393b839719959e1f2ce90d8805e53b131f759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 24 Aug 2023 02:52:16 +0200 Subject: [PATCH 4/5] Use `Mutex` to avoid issue with conditional locks --- compiler/rustc_data_structures/src/sync.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 8defcd39ecdd6..5fe1e8f8c4fdf 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -41,6 +41,7 @@ //! [^2] `MTLockRef` is a typedef. pub use crate::marker::*; +use parking_lot::Mutex; use std::any::Any; use std::collections::HashMap; use std::hash::{BuildHasher, Hash}; @@ -110,13 +111,13 @@ pub use mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode}; /// continuing with unwinding. It's also used for the non-parallel code to ensure error message /// output match the parallel compiler for testing purposes. pub struct ParallelGuard { - panic: Lock>>, + panic: Mutex>>, } impl ParallelGuard { #[inline] pub fn new() -> Self { - ParallelGuard { panic: Lock::new(None) } + ParallelGuard { panic: Mutex::new(None) } } pub fn run(&self, f: impl FnOnce() -> R) -> Option { @@ -316,8 +317,6 @@ cfg_if! { } } } else { - use parking_lot::Mutex; - pub use std::marker::Send as Send; pub use std::marker::Sync as Sync; From c303c8abdddaae5508346f3a31c651744d005dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 25 Aug 2023 22:16:21 +0200 Subject: [PATCH 5/5] Use a `parallel_guard` function to handle the parallel guard --- compiler/rustc_data_structures/src/sync.rs | 115 ++++++++++----------- 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 5fe1e8f8c4fdf..2ec509b91180e 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -115,11 +115,6 @@ pub struct ParallelGuard { } impl ParallelGuard { - #[inline] - pub fn new() -> Self { - ParallelGuard { panic: Mutex::new(None) } - } - pub fn run(&self, f: impl FnOnce() -> R) -> Option { catch_unwind(AssertUnwindSafe(f)) .map_err(|err| { @@ -127,13 +122,18 @@ impl ParallelGuard { }) .ok() } +} - #[inline] - pub fn unwind(self) { - if let Some(panic) = self.panic.into_inner() { - resume_unwind(panic); - } +/// This gives access to a fresh parallel guard in the closure and will unwind any panics +/// caught in it after the closure returns. +#[inline] +pub fn parallel_guard(f: impl FnOnce(&ParallelGuard) -> R) -> R { + let guard = ParallelGuard { panic: Mutex::new(None) }; + let ret = f(&guard); + if let Some(panic) = guard.panic.into_inner() { + resume_unwind(panic); } + ret } cfg_if! { @@ -231,38 +231,38 @@ cfg_if! { where A: FnOnce() -> RA, B: FnOnce() -> RB { - let guard = ParallelGuard::new(); - let a = guard.run(oper_a); - let b = guard.run(oper_b); - guard.unwind(); + let (a, b) = parallel_guard(|guard| { + let a = guard.run(oper_a); + let b = guard.run(oper_b); + (a, b) + }); (a.unwrap(), b.unwrap()) } #[macro_export] macro_rules! parallel { ($($blocks:block),*) => {{ - let mut guard = $crate::sync::ParallelGuard::new(); - $(guard.run(|| $blocks);)* - guard.unwind(); + $crate::sync::parallel_guard(|guard| { + $(guard.run(|| $blocks);)* + }); }} } pub fn par_for_each_in(t: T, mut for_each: impl FnMut(T::Item) + Sync + Send) { - let guard = ParallelGuard::new(); - t.into_iter().for_each(|i| { - guard.run(|| for_each(i)); - }); - guard.unwind(); + parallel_guard(|guard| { + t.into_iter().for_each(|i| { + guard.run(|| for_each(i)); + }); + }) } pub fn par_map>( t: T, mut map: impl FnMut(<::IntoIter as Iterator>::Item) -> R, ) -> C { - let guard = ParallelGuard::new(); - let r = t.into_iter().filter_map(|i| guard.run(|| map(i))).collect(); - guard.unwind(); - r + parallel_guard(|guard| { + t.into_iter().filter_map(|i| guard.run(|| map(i))).collect() + }) } pub use std::rc::Rc as Lrc; @@ -382,10 +382,11 @@ cfg_if! { let (a, b) = rayon::join(move || FromDyn::from(oper_a.into_inner()()), move || FromDyn::from(oper_b.into_inner()())); (a.into_inner(), b.into_inner()) } else { - let guard = ParallelGuard::new(); - let a = guard.run(oper_a); - let b = guard.run(oper_b); - guard.unwind(); + let (a, b) = parallel_guard(|guard| { + let a = guard.run(oper_a); + let b = guard.run(oper_b); + (a, b) + }); (a.unwrap(), b.unwrap()) } } @@ -421,10 +422,10 @@ cfg_if! { // of a single threaded rustc. parallel!(impl $fblock [] [$($blocks),*]); } else { - let guard = $crate::sync::ParallelGuard::new(); - guard.run(|| $fblock); - $(guard.run(|| $blocks);)* - guard.unwind(); + $crate::sync::parallel_guard(|guard| { + guard.run(|| $fblock); + $(guard.run(|| $blocks);)* + }); } }; } @@ -435,20 +436,18 @@ cfg_if! { t: T, for_each: impl Fn(I) + DynSync + DynSend ) { - if mode::is_dyn_thread_safe() { - let for_each = FromDyn::from(for_each); - let guard = ParallelGuard::new(); - t.into_par_iter().for_each(|i| { - guard.run(|| for_each(i)); - }); - guard.unwind(); - } else { - let guard = ParallelGuard::new(); - t.into_iter().for_each(|i| { - guard.run(|| for_each(i)); - }); - guard.unwind(); - } + parallel_guard(|guard| { + if mode::is_dyn_thread_safe() { + let for_each = FromDyn::from(for_each); + t.into_par_iter().for_each(|i| { + guard.run(|| for_each(i)); + }); + } else { + t.into_iter().for_each(|i| { + guard.run(|| for_each(i)); + }); + } + }); } pub fn par_map< @@ -460,18 +459,14 @@ cfg_if! { t: T, map: impl Fn(I) -> R + DynSync + DynSend ) -> C { - if mode::is_dyn_thread_safe() { - let map = FromDyn::from(map); - let guard = ParallelGuard::new(); - let r = t.into_par_iter().filter_map(|i| guard.run(|| map(i))).collect(); - guard.unwind(); - r - } else { - let guard = ParallelGuard::new(); - let r = t.into_iter().filter_map(|i| guard.run(|| map(i))).collect(); - guard.unwind(); - r - } + parallel_guard(|guard| { + if mode::is_dyn_thread_safe() { + let map = FromDyn::from(map); + t.into_par_iter().filter_map(|i| guard.run(|| map(i))).collect() + } else { + t.into_iter().filter_map(|i| guard.run(|| map(i))).collect() + } + }) } /// This makes locks panic if they are already held.