From c5d42b164ca9a2fde7a147b193a842ffb928507e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Mon, 13 Jan 2020 05:30:42 +0100 Subject: [PATCH 1/3] Ensure all iterations in Rayon iterators run in the presence of panics --- src/librustc/hir/map/hir_id_validator.rs | 4 +- src/librustc/ty/mod.rs | 7 +- src/librustc_codegen_ssa/base.rs | 18 ++-- src/librustc_data_structures/sync.rs | 105 +++++++++++++-------- src/librustc_hir/hir.rs | 8 +- src/librustc_interface/passes.rs | 8 +- src/librustc_lint/late.rs | 4 +- src/librustc_mir/monomorphize/collector.rs | 4 +- 8 files changed, 94 insertions(+), 64 deletions(-) diff --git a/src/librustc/hir/map/hir_id_validator.rs b/src/librustc/hir/map/hir_id_validator.rs index 76e42b8af2874..f347a5373c08e 100644 --- a/src/librustc/hir/map/hir_id_validator.rs +++ b/src/librustc/hir/map/hir_id_validator.rs @@ -1,6 +1,6 @@ use crate::hir::map::Map; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::sync::{par_iter, Lock, ParallelIterator}; +use rustc_data_structures::sync::{par_for_each, Lock}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX}; use rustc_hir::intravisit; @@ -12,7 +12,7 @@ pub fn check_crate(hir_map: &Map<'_>) { let errors = Lock::new(Vec::new()); - par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| { + par_for_each(&hir_map.krate().modules, |(module_id, _)| { let local_def_id = hir_map.local_def_id(*module_id); hir_map.visit_item_likes_in_module( local_def_id, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index e6acb6b74dc63..df1e91e440485 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -30,7 +30,7 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_data_structures::sync::{self, par_iter, Lrc, ParallelIterator}; +use rustc_data_structures::sync::{self, par_for_each, Lrc}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; @@ -2642,8 +2642,9 @@ impl<'tcx> TyCtxt<'tcx> { } pub fn par_body_owners(self, f: F) { - par_iter(&self.hir().krate().body_ids) - .for_each(|&body_id| f(self.hir().body_owner_def_id(body_id))); + par_for_each(&self.hir().krate().body_ids, |&body_id| { + f(self.hir().body_owner_def_id(body_id)) + }); } pub fn provided_trait_methods(self, id: DefId) -> Vec { diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index efd560071202c..d74c1d50e782f 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -40,7 +40,7 @@ use rustc::ty::{self, Instance, Ty, TyCtxt}; use rustc_codegen_utils::{check_for_rustc_errors_attr, symbol_names_test}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::profiling::print_time_passes_entry; -use rustc_data_structures::sync::{par_iter, Lock, ParallelIterator}; +use rustc_data_structures::sync::{par_map, Lock}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_index::vec::Idx; @@ -631,15 +631,13 @@ pub fn codegen_crate( .collect(); // Compile the found CGUs in parallel. - par_iter(cgus) - .map(|(i, _)| { - let start_time = Instant::now(); - let module = backend.compile_codegen_unit(tcx, codegen_units[i].name()); - let mut time = total_codegen_time.lock(); - *time += start_time.elapsed(); - (i, module) - }) - .collect() + par_map(cgus, |(i, _)| { + let start_time = Instant::now(); + let module = backend.compile_codegen_unit(tcx, codegen_units[i].name()); + let mut time = total_codegen_time.lock(); + *time += start_time.elapsed(); + (i, module) + }) }) } else { FxHashMap::default() diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index fa98b4d72dda2..7f09e00738d9b 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -18,14 +18,33 @@ //! depending on the value of cfg!(parallel_compiler). use crate::owning_ref::{Erased, OwningRef}; +use std::any::Any; use std::collections::HashMap; use std::hash::{BuildHasher, Hash}; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; +use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe}; pub use std::sync::atomic::Ordering; pub use std::sync::atomic::Ordering::SeqCst; +pub fn catch( + store: &Lock>>, + f: impl FnOnce() -> R, +) -> Option { + catch_unwind(AssertUnwindSafe(f)) + .map_err(|err| { + *store.lock() = Some(err); + }) + .ok() +} + +pub fn resume(store: Lock>>) { + if let Some(panic) = store.into_inner() { + resume_unwind(panic); + } +} + cfg_if! { if #[cfg(not(parallel_compiler))] { pub auto trait Send {} @@ -42,7 +61,6 @@ cfg_if! { } use std::ops::Add; - use std::panic::{resume_unwind, catch_unwind, AssertUnwindSafe}; /// This is a single threaded variant of AtomicCell provided by crossbeam. /// Unlike `Atomic` this is intended for all `Copy` types, @@ -181,46 +199,40 @@ cfg_if! { ($($blocks:tt),*) => { // We catch panics here ensuring that all the blocks execute. // This makes behavior consistent with the parallel compiler. - let mut panic = None; + let panic = ::rustc_data_structures::sync::Lock::new(None); $( - if let Err(p) = ::std::panic::catch_unwind( - ::std::panic::AssertUnwindSafe(|| $blocks) - ) { - if panic.is_none() { - panic = Some(p); - } - } + ::rustc_data_structures::sync::catch(&panic, || $blocks); )* - if let Some(panic) = panic { - ::std::panic::resume_unwind(panic); - } + ::rustc_data_structures::sync::resume(panic); } } - pub use std::iter::Iterator as ParallelIterator; + use std::iter::{Iterator, IntoIterator, FromIterator}; - pub fn par_iter(t: T) -> T::IntoIter { - t.into_iter() - } - - pub fn par_for_each_in( + pub fn par_for_each( t: T, - for_each: - impl Fn(<::IntoIter as Iterator>::Item) + Sync + Send + mut for_each: impl FnMut(<::IntoIter as Iterator>::Item), ) { // We catch panics here ensuring that all the loop iterations execute. // This makes behavior consistent with the parallel compiler. - let mut panic = None; + let panic = Lock::new(None); t.into_iter().for_each(|i| { - if let Err(p) = catch_unwind(AssertUnwindSafe(|| for_each(i))) { - if panic.is_none() { - panic = Some(p); - } - } + catch(&panic, || for_each(i)); }); - if let Some(panic) = panic { - resume_unwind(panic); - } + resume(panic); + } + + 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 panic = Lock::new(None); + let r = t.into_iter().filter_map(|i| { + catch(&panic, || map(i)) + }).collect(); + resume(panic); + r } pub type MetadataRef = OwningRef, [u8]>; @@ -388,20 +400,39 @@ cfg_if! { pub use rayon_core::WorkerLocal; - pub use rayon::iter::ParallelIterator; - use rayon::iter::IntoParallelIterator; - - pub fn par_iter(t: T) -> T::Iter { - t.into_par_iter() - } + use rayon::iter::{ParallelIterator, FromParallelIterator, IntoParallelIterator}; - pub fn par_for_each_in( + pub fn par_for_each( t: T, for_each: impl Fn( <::Iter as ParallelIterator>::Item ) + Sync + Send ) { - t.into_par_iter().for_each(for_each) + // We catch panics here ensuring that all the loop iterations execute. + let panic = Lock::new(None); + t.into_par_iter().for_each(|i| { + catch(&panic, || for_each(i)); + }); + resume(panic); + } + + pub fn par_map< + T: IntoParallelIterator, + R: Send, + C: FromParallelIterator + >( + t: T, + map: impl Fn( + <::Iter as ParallelIterator>::Item + ) -> R + Sync + Send + ) -> C { + // We catch panics here ensuring that all the loop iterations execute. + let panic = Lock::new(None); + let r = t.into_par_iter().filter_map(|i| { + catch(&panic, || map(i)) + }).collect(); + resume(panic); + r } pub type MetadataRef = OwningRef, [u8]>; diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 550e3654d0800..fc016f579e2da 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -9,7 +9,7 @@ crate use FunctionRetTy::*; crate use UnsafeSource::*; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; +use rustc_data_structures::sync::{par_for_each, Send, Sync}; use rustc_errors::FatalError; use rustc_macros::HashStable_Generic; use rustc_session::node_id::NodeMap; @@ -669,17 +669,17 @@ impl Crate<'_> { { parallel!( { - par_for_each_in(&self.items, |(_, item)| { + par_for_each(&self.items, |(_, item)| { visitor.visit_item(item); }); }, { - par_for_each_in(&self.trait_items, |(_, trait_item)| { + par_for_each(&self.trait_items, |(_, trait_item)| { visitor.visit_trait_item(trait_item); }); }, { - par_for_each_in(&self.impl_items, |(_, impl_item)| { + par_for_each(&self.impl_items, |(_, impl_item)| { visitor.visit_impl_item(impl_item); }); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index c4444fbaa2fc7..58e9a26bc3875 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -21,7 +21,7 @@ use rustc_builtin_macros; use rustc_codegen_ssa::back::link::emit_metadata; use rustc_codegen_utils::codegen_backend::CodegenBackend; use rustc_codegen_utils::link::filename_for_metadata; -use rustc_data_structures::sync::{par_iter, Lrc, Once, ParallelIterator, WorkerLocal}; +use rustc_data_structures::sync::{par_for_each, Lrc, Once, WorkerLocal}; use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel}; use rustc_errors::PResult; use rustc_expand::base::ExtCtxt; @@ -786,7 +786,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> { sess.time("looking_for_derive_registrar", || proc_macro_decls::find(tcx)); }, { - par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + par_for_each(&tcx.hir().krate().modules, |(&module, _)| { let local_def_id = tcx.hir().local_def_id(module); tcx.ensure().check_mod_loops(local_def_id); tcx.ensure().check_mod_attrs(local_def_id); @@ -811,7 +811,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> { }, { sess.time("liveness_and_intrinsic_checking", || { - par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + par_for_each(&tcx.hir().krate().modules, |(&module, _)| { // this must run before MIR dump, because // "not all control paths return a value" is reported here. // @@ -879,7 +879,7 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> { }, { sess.time("privacy_checking_modules", || { - par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + par_for_each(&tcx.hir().krate().modules, |(&module, _)| { tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module)); }); }); diff --git a/src/librustc_lint/late.rs b/src/librustc_lint/late.rs index 30a3788377508..b1467da1bf5e5 100644 --- a/src/librustc_lint/late.rs +++ b/src/librustc_lint/late.rs @@ -17,7 +17,7 @@ use crate::{passes::LateLintPassObject, LateContext, LateLintPass, LintStore}; use rustc::hir::map::Map; use rustc::ty::{self, TyCtxt}; -use rustc_data_structures::sync::{join, par_iter, ParallelIterator}; +use rustc_data_structures::sync::{join, par_for_each}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit as hir_visit; @@ -481,7 +481,7 @@ pub fn check_crate<'tcx, T: for<'a> LateLintPass<'a, 'tcx>>( || { tcx.sess.time("module_lints", || { // Run per-module lints - par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + par_for_each(&tcx.hir().krate().modules, |(&module, _)| { tcx.ensure().lint_mod(tcx.hir().local_def_id(module)); }); }); diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index f5f00c9343561..677a7c64230cc 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -189,7 +189,7 @@ use rustc::ty::print::obsolete::DefPathBasedNames; use rustc::ty::subst::{InternalSubsts, SubstsRef}; use rustc::ty::{self, GenericParamDefKind, Instance, Ty, TyCtxt, TypeFoldable}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::sync::{par_iter, MTLock, MTRef, ParallelIterator}; +use rustc_data_structures::sync::{par_for_each, MTLock, MTRef}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, DefIdMap, LOCAL_CRATE}; use rustc_hir::itemlikevisit::ItemLikeVisitor; @@ -291,7 +291,7 @@ pub fn collect_crate_mono_items( let inlining_map: MTRef<'_, _> = &mut inlining_map; tcx.sess.time("monomorphization_collector_graph_walk", || { - par_iter(roots).for_each(|root| { + par_for_each(roots, |root| { let mut recursion_depths = DefIdMap::default(); collect_items_rec(tcx, root, visited, &mut recursion_depths, inlining_map); }); From 69276ba74f8c27a0fb33b4da2de9b43f2f4ffdf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Mon, 13 Jan 2020 09:57:52 +0100 Subject: [PATCH 2/3] Update tests --- src/test/ui/privacy/privacy2.stderr | 8 +++++++- src/test/ui/privacy/privacy3.stderr | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/test/ui/privacy/privacy2.stderr b/src/test/ui/privacy/privacy2.stderr index 9f2359657bd7c..f55adc8c70110 100644 --- a/src/test/ui/privacy/privacy2.stderr +++ b/src/test/ui/privacy/privacy2.stderr @@ -12,7 +12,13 @@ LL | use bar::glob::foo; error: requires `sized` lang_item -error: aborting due to 3 previous errors +error: requires `sized` lang_item + +error: requires `sized` lang_item + +error: requires `sized` lang_item + +error: aborting due to 6 previous errors Some errors have detailed explanations: E0432, E0603. For more information about an error, try `rustc --explain E0432`. diff --git a/src/test/ui/privacy/privacy3.stderr b/src/test/ui/privacy/privacy3.stderr index 22c1e48b07d94..42ce456d962a1 100644 --- a/src/test/ui/privacy/privacy3.stderr +++ b/src/test/ui/privacy/privacy3.stderr @@ -6,6 +6,12 @@ LL | use bar::gpriv; error: requires `sized` lang_item -error: aborting due to 2 previous errors +error: requires `sized` lang_item + +error: requires `sized` lang_item + +error: requires `sized` lang_item + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0432`. From 934abf2b06ec1e81d4a320bb138b71d007b3d233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 16 Jan 2020 10:58:52 +0100 Subject: [PATCH 3/3] Use $crate --- src/librustc_data_structures/sync.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_data_structures/sync.rs b/src/librustc_data_structures/sync.rs index 7f09e00738d9b..d51fd64e89113 100644 --- a/src/librustc_data_structures/sync.rs +++ b/src/librustc_data_structures/sync.rs @@ -199,11 +199,11 @@ cfg_if! { ($($blocks:tt),*) => { // We catch panics here ensuring that all the blocks execute. // This makes behavior consistent with the parallel compiler. - let panic = ::rustc_data_structures::sync::Lock::new(None); + let panic = $crate::sync::Lock::new(None); $( - ::rustc_data_structures::sync::catch(&panic, || $blocks); + $crate::sync::catch(&panic, || $blocks); )* - ::rustc_data_structures::sync::resume(panic); + $crate::sync::resume(panic); } } @@ -383,7 +383,7 @@ cfg_if! { parallel!(impl $fblock [$block, $($c,)*] [$($rest),*]) }; (impl $fblock:tt [$($blocks:tt,)*] []) => { - ::rustc_data_structures::sync::scope(|s| { + $crate::sync::scope(|s| { $( s.spawn(|_| $blocks); )* @@ -445,7 +445,7 @@ cfg_if! { macro_rules! rustc_erase_owner { ($v:expr) => {{ let v = $v; - ::rustc_data_structures::sync::assert_send_val(&v); + $crate::sync::assert_send_val(&v); v.erase_send_sync_owner() }} }