diff --git a/crates/compiler/build/src/program.rs b/crates/compiler/build/src/program.rs index c8e6fd2496d..ee3a70fb106 100644 --- a/crates/compiler/build/src/program.rs +++ b/crates/compiler/build/src/program.rs @@ -41,7 +41,6 @@ pub struct CodeGenTiming { pub fn report_problems_monomorphized(loaded: &mut MonomorphizedModule) -> Problems { report_problems( - loaded.total_problems(), &loaded.sources, &loaded.interns, &mut loaded.can_problems, @@ -51,7 +50,6 @@ pub fn report_problems_monomorphized(loaded: &mut MonomorphizedModule) -> Proble pub fn report_problems_typechecked(loaded: &mut LoadedModule) -> Problems { report_problems( - loaded.total_problems(), &loaded.sources, &loaded.interns, &mut loaded.can_problems, diff --git a/crates/compiler/load/build.rs b/crates/compiler/load/build.rs index 6655800ad31..bb963c50b48 100644 --- a/crates/compiler/load/build.rs +++ b/crates/compiler/load/build.rs @@ -105,7 +105,6 @@ fn write_types_for_module_real(module_id: ModuleId, filename: &str, output_path: }; let problems = report_problems( - module.total_problems(), &module.sources, &module.interns, &mut module.can_problems, diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index 4a112656756..2063746d7e3 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/crates/compiler/load_internal/src/file.rs @@ -954,7 +954,6 @@ pub enum LoadingProblem<'a> { ParsingFailed(FileError<'a, SyntaxError<'a>>), UnexpectedHeader(String), - MsgChannelDied, ErrJoiningWorkerThreads, TriedToImportAppModule, @@ -964,6 +963,27 @@ pub enum LoadingProblem<'a> { ImportCycle(PathBuf, Vec), IncorrectModuleName(FileError<'a, IncorrectModuleName<'a>>), CouldNotFindCacheDir, + ChannelProblem(ChannelProblem), +} + +#[derive(Debug)] +pub enum ChannelProblem { + FailedToEnqueueTask(Box), + FailedToSendRootMsg, + FailedToSendWorkerShutdownMsg, + ChannelDisconnected, + FailedToSendManyMsg, + FailedToSendFinishedSpecializationsMsg, + FailedToSendTaskMsg, + FailedToSendFinishedTypeCheckingMsg, +} + +#[derive(Debug)] +pub struct PanicReportInfo { + can_problems: MutMap>, + type_problems: MutMap>, + sources: MutMap)>, + interns: Interns, } pub enum Phases { @@ -980,13 +1000,35 @@ fn enqueue_task<'a>( injector: &Injector>, listeners: &[Sender], task: BuildTask<'a>, + state: &State<'a>, ) -> Result<(), LoadingProblem<'a>> { injector.push(task); for listener in listeners { - listener - .send(WorkerMsg::TaskAdded) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + listener.send(WorkerMsg::TaskAdded).map_err(|_| { + let module_ids = { (*state.arc_modules).lock().clone() }.into_module_ids(); + + let interns = Interns { + module_ids, + all_ident_ids: state.constrained_ident_ids.clone(), + }; + + LoadingProblem::ChannelProblem(ChannelProblem::FailedToEnqueueTask(Box::new( + PanicReportInfo { + can_problems: state.module_cache.can_problems.clone(), + type_problems: state.module_cache.type_problems.clone(), + interns, + sources: state + .module_cache + .sources + .iter() + .map(|(key, (path, str_ref))| { + (*key, (path.clone(), str_ref.to_string().into_boxed_str())) + }) + .collect(), + }, + ))) + })?; } Ok(()) @@ -1030,7 +1072,7 @@ pub fn load_and_typecheck_str<'a>( roc_cache_dir, load_config, )? { - Monomorphized(_) => unreachable!(""), + Monomorphized(_) => unreachable!(), TypeChecked(module) => Ok(module), } } @@ -1325,7 +1367,7 @@ pub fn load_single_threaded<'a>( msg_tx .send(root_msg) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + .map_err(|_| LoadingProblem::ChannelProblem(ChannelProblem::FailedToSendRootMsg))?; let number_of_workers = 1; let mut state = State::new( @@ -1575,7 +1617,9 @@ fn state_thread_step<'a>( } Err(err) => match err { crossbeam::channel::TryRecvError::Empty => Ok(ControlFlow::Continue(state)), - crossbeam::channel::TryRecvError::Disconnected => Err(LoadingProblem::MsgChannelDied), + crossbeam::channel::TryRecvError::Disconnected => Err(LoadingProblem::ChannelProblem( + ChannelProblem::ChannelDisconnected, + )), }, } } @@ -1647,7 +1691,7 @@ fn load_multi_threaded<'a>( let (msg_tx, msg_rx) = bounded(1024); msg_tx .send(root_msg) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + .map_err(|_| LoadingProblem::ChannelProblem(ChannelProblem::FailedToSendRootMsg))?; // Reserve one CPU for the main thread, and let all the others be eligible // to spawn workers. @@ -1705,10 +1749,15 @@ fn load_multi_threaded<'a>( // Get a reference to the completed stealers, so we can send that // reference to each worker. (Slices are Sync, but bumpalo Vecs are not.) let stealers = stealers.into_bump_slice(); - let it = worker_arenas.iter_mut(); + + let mut can_problems_recorded = MutMap::default(); + let mut type_problems_recorded = MutMap::default(); + let mut sources_recorded = MutMap::default(); + let mut interns_recorded = Interns::default(); + { - thread::scope(|thread_scope| { + let thread_result = thread::scope(|thread_scope| { let mut worker_listeners = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); @@ -1744,7 +1793,9 @@ fn load_multi_threaded<'a>( ) }); - res_join_handle.unwrap(); + res_join_handle.unwrap_or_else(|_| { + panic!("Join handle panicked!"); + }); } // We've now distributed one worker queue to each thread. @@ -1760,9 +1811,13 @@ fn load_multi_threaded<'a>( macro_rules! shut_down_worker_threads { () => { for listener in worker_listeners { - listener - .send(WorkerMsg::Shutdown) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + // We intentionally don't propagate this Result, because even if + // shutting down a worker failed (which can happen if a a panic + // occurred on that thread), we want to continue shutting down + // the others regardless. + if listener.send(WorkerMsg::Shutdown).is_err() { + log!("There was an error trying to shutdown a worker thread. One reason this can happen is if the thread panicked."); + } } }; } @@ -1788,6 +1843,36 @@ fn load_multi_threaded<'a>( state = new_state; continue; } + Err(LoadingProblem::ChannelProblem(ChannelProblem::FailedToEnqueueTask( + info, + ))) => { + let PanicReportInfo { + can_problems, + type_problems, + sources, + interns, + } = *info; + + // Record these for later. + can_problems_recorded = can_problems; + type_problems_recorded = type_problems; + sources_recorded = sources; + interns_recorded = interns; + + shut_down_worker_threads!(); + + return Err(LoadingProblem::ChannelProblem( + ChannelProblem::FailedToEnqueueTask(Box::new(PanicReportInfo { + // This return value never gets used, so don't bother + // cloning these in order to be able to return them. + // Really, anything could go here. + can_problems: Default::default(), + type_problems: Default::default(), + sources: Default::default(), + interns: Default::default(), + })), + )); + } Err(e) => { shut_down_worker_threads!(); @@ -1795,9 +1880,33 @@ fn load_multi_threaded<'a>( } } } + }); + + thread_result.unwrap_or_else(|_| { + // This most likely means a panic occurred in one of the threads. + // Therefore, print all the error info we've accumulated, and note afterwards + // that there was a compiler crash. + // + // Unfortunately, this often has no information to report if there's a panic in mono. + // Consequently, the following ends up being more misleading than helpful. + // + // roc_reporting::cli::report_problems( + // &sources_recorded, + // &mut interns_recorded, + // &mut can_problems_recorded, + // &mut type_problems_recorded, + // ) + // .print_to_stdout(Duration::default()); // TODO determine total elapsed time and use it here + + Err(LoadingProblem::FormattedReport( + concat!( + "\n\nThere was an unrecoverable error in the Roc compiler. The `roc check` ", + "command can sometimes give a more helpful error report than other commands.\n\n" + ) + .to_string(), + )) }) } - .unwrap() } fn worker_task_step<'a>( @@ -1843,8 +1952,8 @@ fn worker_task_step<'a>( match result { Ok(()) => {} - Err(LoadingProblem::MsgChannelDied) => { - panic!("Msg channel closed unexpectedly.") + Err(LoadingProblem::ChannelProblem(problem)) => { + panic!("Channel problem: {problem:?}"); } Err(LoadingProblem::ParsingFailed(problem)) => { msg_tx.send(Msg::FailedToParse(problem)).unwrap(); @@ -1942,8 +2051,8 @@ fn worker_task<'a>( match result { Ok(()) => {} - Err(LoadingProblem::MsgChannelDied) => { - panic!("Msg channel closed unexpectedly.") + Err(LoadingProblem::ChannelProblem(problem)) => { + panic!("Channel problem: {problem:?}"); } Err(LoadingProblem::ParsingFailed(problem)) => { msg_tx.send(Msg::FailedToParse(problem)).unwrap(); @@ -1976,8 +2085,10 @@ fn start_tasks<'a>( worker_listeners: &'a [Sender], ) -> Result<(), LoadingProblem<'a>> { for (module_id, phase) in work { - for task in start_phase(module_id, phase, arena, state) { - enqueue_task(injector, worker_listeners, task)? + let tasks = start_phase(module_id, phase, arena, state); + + for task in tasks { + enqueue_task(injector, worker_listeners, task, state)? } } @@ -2086,9 +2197,9 @@ fn update<'a>( Many(messages) => { // enqueue all these message for msg in messages { - msg_tx - .send(msg) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + msg_tx.send(msg).map_err(|_| { + LoadingProblem::ChannelProblem(ChannelProblem::FailedToSendManyMsg) + })?; } Ok(state) @@ -2552,7 +2663,11 @@ fn update<'a>( #[cfg(debug_assertions)] checkmate, }) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + .map_err(|_| { + LoadingProblem::ChannelProblem( + ChannelProblem::FailedToSendFinishedTypeCheckingMsg, + ) + })?; // bookkeeping state.declarations_by_id.insert(module_id, decls); @@ -2874,7 +2989,11 @@ fn update<'a>( exposed_to_host: state.exposed_to_host.clone(), module_expectations, }) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + .map_err(|_| { + LoadingProblem::ChannelProblem( + ChannelProblem::FailedToSendFinishedSpecializationsMsg, + ) + })?; Ok(state) } @@ -6308,7 +6427,7 @@ fn run_task<'a>( msg_tx .send(msg) - .map_err(|_| LoadingProblem::MsgChannelDied)?; + .map_err(|_| LoadingProblem::ChannelProblem(ChannelProblem::FailedToSendTaskMsg))?; Ok(()) } diff --git a/crates/compiler/load_internal/src/module.rs b/crates/compiler/load_internal/src/module.rs index 089c22a4097..2d5869311c7 100644 --- a/crates/compiler/load_internal/src/module.rs +++ b/crates/compiler/load_internal/src/module.rs @@ -223,22 +223,6 @@ pub struct ExposedToHost { pub getters: Vec, } -impl<'a> MonomorphizedModule<'a> { - pub fn total_problems(&self) -> usize { - let mut total = 0; - - for problems in self.can_problems.values() { - total += problems.len(); - } - - for problems in self.type_problems.values() { - total += problems.len(); - } - - total - } -} - #[derive(Debug)] pub struct ModuleTiming { pub read_roc_file: Duration, diff --git a/crates/reporting/src/cli.rs b/crates/reporting/src/cli.rs index a31533f35fe..9447fdf0d6d 100644 --- a/crates/reporting/src/cli.rs +++ b/crates/reporting/src/cli.rs @@ -46,13 +46,12 @@ impl Problems { 1 => "warning", _ => "warnings", }, - total_time.as_millis(), + total_time.as_millis() ); } } pub fn report_problems( - total_problems: usize, sources: &MutMap)>, interns: &Interns, can_problems: &mut MutMap>, @@ -60,7 +59,17 @@ pub fn report_problems( ) -> Problems { use crate::report::{can_problem, type_problem, Report, RocDocAllocator, DEFAULT_PALETTE}; use roc_problem::Severity::*; + let palette = DEFAULT_PALETTE; + let mut total_problems = 0; + + for problems in can_problems.values() { + total_problems += problems.len(); + } + + for problems in type_problems.values() { + total_problems += problems.len(); + } // This will often over-allocate total memory, but it means we definitely // never need to re-allocate either the warnings or the errors vec! @@ -126,6 +135,9 @@ pub fn report_problems( } } + debug_assert!(can_problems.is_empty() && type_problems.is_empty(), "After reporting problems, there were {:?} can_problems and {:?} type_problems that could not be reported because they did not have corresponding entries in `sources`.", can_problems.len(), type_problems.len()); + debug_assert_eq!(errors.len() + warnings.len(), total_problems); + let problems_reported; // Only print warnings if there are no errors