Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all iterations in Rayon iterators run in the presence of panics #68171

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc/hir/map/hir_id_validator.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -2642,8 +2642,9 @@ impl<'tcx> TyCtxt<'tcx> {
}

pub fn par_body_owners<F: Fn(DefId) + sync::Sync + sync::Send>(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<AssocItem> {
Expand Down
18 changes: 8 additions & 10 deletions src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -631,15 +631,13 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
.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()
Expand Down
109 changes: 70 additions & 39 deletions src/librustc_data_structures/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<R>(
store: &Lock<Option<Box<dyn Any + std::marker::Send + 'static>>>,
f: impl FnOnce() -> R,
) -> Option<R> {
catch_unwind(AssertUnwindSafe(f))
.map_err(|err| {
*store.lock() = Some(err);
})
.ok()
}

pub fn resume(store: Lock<Option<Box<dyn Any + std::marker::Send + 'static>>>) {
if let Some(panic) = store.into_inner() {
resume_unwind(panic);
}
}

cfg_if! {
if #[cfg(not(parallel_compiler))] {
pub auto trait Send {}
Expand All @@ -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,
Expand Down Expand Up @@ -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 = $crate::sync::Lock::new(None);
$(
if let Err(p) = ::std::panic::catch_unwind(
::std::panic::AssertUnwindSafe(|| $blocks)
) {
if panic.is_none() {
panic = Some(p);
}
}
$crate::sync::catch(&panic, || $blocks);
)*
if let Some(panic) = panic {
::std::panic::resume_unwind(panic);
}
$crate::sync::resume(panic);
}
}

pub use std::iter::Iterator as ParallelIterator;
use std::iter::{Iterator, IntoIterator, FromIterator};

pub fn par_iter<T: IntoIterator>(t: T) -> T::IntoIter {
t.into_iter()
}

pub fn par_for_each_in<T: IntoIterator>(
pub fn par_for_each<T: IntoIterator>(
t: T,
for_each:
impl Fn(<<T as IntoIterator>::IntoIter as Iterator>::Item) + Sync + Send
mut for_each: impl FnMut(<<T as IntoIterator>::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: IntoIterator, R, C: FromIterator<R>>(
t: T,
mut map: impl FnMut(<<T as IntoIterator>::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
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
}

pub type MetadataRef = OwningRef<Box<dyn Erased>, [u8]>;
Expand Down Expand Up @@ -371,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);
)*
Expand All @@ -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: IntoParallelIterator>(t: T) -> T::Iter {
t.into_par_iter()
}
use rayon::iter::{ParallelIterator, FromParallelIterator, IntoParallelIterator};

pub fn par_for_each_in<T: IntoParallelIterator>(
pub fn par_for_each<T: IntoParallelIterator>(
t: T,
for_each: impl Fn(
<<T as IntoParallelIterator>::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<R>
>(
t: T,
map: impl Fn(
<<T as IntoParallelIterator>::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<Box<dyn Erased + Send + Sync>, [u8]>;
Expand All @@ -414,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()
}}
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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.
//
Expand Down Expand Up @@ -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));
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
Expand Down
8 changes: 7 additions & 1 deletion src/test/ui/privacy/privacy2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
8 changes: 7 additions & 1 deletion src/test/ui/privacy/privacy3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.