From c308276c22a57e85623d95c596b0dde1b6fb4e5c Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 13:20:53 +0200 Subject: [PATCH 01/11] remove a to_vec() call --- compiler/solve/src/solve.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index b35e08c205..d87953df9d 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -2426,7 +2426,7 @@ fn generalize( young_rank: Rank, pools: &mut Pools, ) { - let young_vars = pools.get(young_rank); + let young_vars = std::mem::take(pools.get_mut(young_rank)); let rank_table = pool_to_rank_table(subs, young_mark, young_rank, young_vars); // Get the ranks right for each entry. @@ -2464,32 +2464,38 @@ fn generalize( } } } + + // re-use the last_vector (which likely has a good capacity for future runs + *pools.get_mut(young_rank) = last_pool; } /// Sort the variables into buckets by rank. +#[inline] fn pool_to_rank_table( subs: &mut Subs, young_mark: Mark, young_rank: Rank, - young_vars: &[Variable], + mut young_vars: Vec, ) -> Pools { let mut pools = Pools::new(young_rank.into_usize() + 1); // the vast majority of young variables have young_rank - // using `retain` here prevents many `pools.get_mut(young_rank)` lookups - let mut young_vars = young_vars.to_vec(); - young_vars.retain(|var| { - let rank = subs.get_rank_set_mark(*var, young_mark); + let mut i = 0; + while i < young_vars.len() { + let var = young_vars[i]; + let rank = subs.get_rank_set_mark(var, young_mark); if rank != young_rank { debug_assert!(rank.into_usize() < young_rank.into_usize() + 1); - pools.get_mut(rank).push(*var); - false + pools.get_mut(rank).push(var); + + // swap an element in; don't increment i + young_vars.swap_remove(i); } else { - true + i += 1; } - }); + } std::mem::swap(pools.get_mut(young_rank), &mut young_vars); From fa3eff6515c8b0e9459d2772effc6155614c2775 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 13:21:23 +0200 Subject: [PATCH 02/11] re-use a vector that already has a good capacity --- compiler/solve/src/solve.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index d87953df9d..a8a370917c 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -476,10 +476,13 @@ impl Pools { self.0.iter() } - pub fn split_last(&self) -> (&Vec, &[Vec]) { - self.0 - .split_last() - .unwrap_or_else(|| panic!("Attempted to split_last() on non-empty Pools")) + pub fn split_last(mut self) -> (Vec, Vec>) { + let last = self + .0 + .pop() + .unwrap_or_else(|| panic!("Attempted to split_last() on non-empty Pools")); + + (last, self.0) } pub fn extend_to(&mut self, n: usize) { @@ -737,8 +740,7 @@ fn solve( // pop pool generalize(subs, young_mark, visit_mark, next_rank, pools); - - pools.get_mut(next_rank).clear(); + debug_assert!(pools.get(next_rank).is_empty()); // check that things went well dbg_do!(ROC_VERIFY_RIGID_LET_GENERALIZED, { @@ -2437,12 +2439,12 @@ fn generalize( } } - let (last_pool, all_but_last_pool) = rank_table.split_last(); + let (mut last_pool, all_but_last_pool) = rank_table.split_last(); // For variables that have rank lowerer than young_rank, register them in // the appropriate old pool if they are not redundant. for vars in all_but_last_pool { - for &var in vars { + for var in vars { if !subs.redundant(var) { let rank = subs.get_rank(var); @@ -2453,7 +2455,7 @@ fn generalize( // For variables with rank young_rank, if rank < young_rank: register in old pool, // otherwise generalize - for &var in last_pool { + for var in last_pool.drain(..) { if !subs.redundant(var) { let desc_rank = subs.get_rank(var); From 56e3438c614c4cdcd24025087bf2a751d16f5320 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 13:38:42 +0200 Subject: [PATCH 03/11] add max-threads flag --- cli/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 18d4e4de53..e6603cdbdb 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -35,6 +35,7 @@ pub const CMD_FORMAT: &str = "format"; pub const FLAG_DEBUG: &str = "debug"; pub const FLAG_DEV: &str = "dev"; pub const FLAG_OPTIMIZE: &str = "optimize"; +pub const FLAG_MAX_THREADS: &str = "max-threads"; pub const FLAG_OPT_SIZE: &str = "opt-size"; pub const FLAG_LIB: &str = "lib"; pub const FLAG_NO_LINK: &str = "no-link"; @@ -58,6 +59,14 @@ pub fn build_app<'a>() -> Command<'a> { .requires(ROC_FILE) .required(false); + let flag_max_threads = Arg::new(FLAG_MAX_THREADS) + .long(FLAG_MAX_THREADS) + .help("Limit the number of threads (and hence cores) used during compilation.") + .takes_value(true) + .requires(ROC_FILE) + .validator(|s| s.parse::()) + .required(false); + let flag_opt_size = Arg::new(FLAG_OPT_SIZE) .long(FLAG_OPT_SIZE) .help("Optimize the compiled program to have a small binary size. (Optimization takes time to complete.)") @@ -102,6 +111,7 @@ pub fn build_app<'a>() -> Command<'a> { .subcommand(Command::new(CMD_BUILD) .about("Build a binary from the given .roc file, but don't run it") .arg(flag_optimize.clone()) + .arg(flag_max_threads.clone()) .arg(flag_opt_size.clone()) .arg(flag_dev.clone()) .arg(flag_debug.clone()) @@ -141,6 +151,7 @@ pub fn build_app<'a>() -> Command<'a> { .subcommand(Command::new(CMD_RUN) .about("Run a .roc file even if it has build errors") .arg(flag_optimize.clone()) + .arg(flag_max_threads.clone()) .arg(flag_opt_size.clone()) .arg(flag_dev.clone()) .arg(flag_debug.clone()) @@ -193,6 +204,7 @@ pub fn build_app<'a>() -> Command<'a> { ) .trailing_var_arg(true) .arg(flag_optimize) + .arg(flag_max_threads.clone()) .arg(flag_opt_size) .arg(flag_dev) .arg(flag_debug) From 206699990873c04561ad53812dcc285375e3e4af Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 14:01:34 +0200 Subject: [PATCH 04/11] get the threading info into file.rs --- cli/src/build.rs | 2 +- cli/src/lib.rs | 12 +++++++++++- compiler/load/build.rs | 2 +- compiler/load_internal/src/file.rs | 5 ++++- compiler/test_gen/src/helpers/llvm.rs | 2 +- docs/src/lib.rs | 2 +- editor/src/editor/main.rs | 2 +- editor/src/editor/mvc/ed_model.rs | 2 +- 8 files changed, 21 insertions(+), 8 deletions(-) diff --git a/cli/src/build.rs b/cli/src/build.rs index 12e11bc409..0f5241e4c3 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -368,7 +368,7 @@ pub fn check_file( target_info, // TODO: expose this from CLI? RenderTarget::ColorTerminal, - Threading::Multi, + Threading::AllAvailable, )?; let buf = &mut String::with_capacity(1024); diff --git a/cli/src/lib.rs b/cli/src/lib.rs index e6603cdbdb..d260866e17 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -284,6 +284,16 @@ pub fn build( let emit_debug_info = matches.is_present(FLAG_DEBUG); let emit_timings = matches.is_present(FLAG_TIME); + let threading = match matches + .value_of(FLAG_MAX_THREADS) + .and_then(|s| s.parse::().ok()) + { + None => Threading::AllAvailable, + Some(0) => user_error!("cannot build with at most 0 threads"), + Some(1) => Threading::Single, + Some(n) => Threading::AtMost(n), + }; + // Use surgical linking when supported, or when explicitly requested with --linker surgical let surgically_link = if matches.is_present(FLAG_LINKER) { matches.value_of(FLAG_LINKER) == Some("surgical") @@ -333,7 +343,7 @@ pub fn build( surgically_link, precompiled, target_valgrind, - Threading::Multi, + threading, ); match res_binary_path { diff --git a/compiler/load/build.rs b/compiler/load/build.rs index e0e5d643f8..5f2754723e 100644 --- a/compiler/load/build.rs +++ b/compiler/load/build.rs @@ -38,7 +38,7 @@ fn write_subs_for_module(module_id: ModuleId, filename: &str) { Default::default(), target_info, roc_reporting::report::RenderTarget::ColorTerminal, - Threading::Multi, + Threading::AllAvailable, ); let module = res_module.unwrap(); diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index f97bf7d012..1edf634c17 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -1099,7 +1099,8 @@ pub enum LoadResult<'a> { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum Threading { Single, - Multi, + AllAvailable, + AtMost(usize), } /// The loading process works like this, starting from the given filename (e.g. "main.roc"): @@ -1180,6 +1181,7 @@ pub fn load<'a>( target_info, cached_subs, render, + threading, ) } } @@ -1390,6 +1392,7 @@ fn load_multi_threaded<'a>( target_info: TargetInfo, cached_subs: MutMap)>, render: RenderTarget, + threading: Threading, ) -> Result, LoadingProblem<'a>> { let LoadStart { arc_modules, diff --git a/compiler/test_gen/src/helpers/llvm.rs b/compiler/test_gen/src/helpers/llvm.rs index d4c89f25e6..068a38bc3b 100644 --- a/compiler/test_gen/src/helpers/llvm.rs +++ b/compiler/test_gen/src/helpers/llvm.rs @@ -60,7 +60,7 @@ fn create_llvm_module<'a>( Default::default(), target_info, RenderTarget::ColorTerminal, - Threading::Multi, + Threading::AllAvailable, ); let mut loaded = match loaded { diff --git a/docs/src/lib.rs b/docs/src/lib.rs index bff952e22a..aa748352c2 100644 --- a/docs/src/lib.rs +++ b/docs/src/lib.rs @@ -435,7 +435,7 @@ pub fn load_modules_for_files(filenames: Vec) -> Vec { Default::default(), roc_target::TargetInfo::default_x86_64(), // This is just type-checking for docs, so "target" doesn't matter roc_reporting::report::RenderTarget::ColorTerminal, - Threading::Multi, + Threading::AllAvailable, ) { Ok(loaded) => modules.push(loaded), Err(LoadingProblem::FormattedReport(report)) => { diff --git a/editor/src/editor/main.rs b/editor/src/editor/main.rs index 8c6219d984..c3c5c63559 100644 --- a/editor/src/editor/main.rs +++ b/editor/src/editor/main.rs @@ -129,7 +129,7 @@ fn run_event_loop(project_dir_path_opt: Option<&Path>) -> Result<(), Box Date: Sat, 7 May 2022 14:18:28 +0200 Subject: [PATCH 05/11] hook up the max_threads flag --- compiler/load_internal/src/file.rs | 56 +++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index 1edf634c17..9b9538b310 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -1158,10 +1158,32 @@ pub fn load<'a>( render: RenderTarget, threading: Threading, ) -> Result, LoadingProblem<'a>> { - // When compiling to wasm, we cannot spawn extra threads - // so we have a single-threaded implementation - if threading == Threading::Single || cfg!(target_family = "wasm") { - load_single_threaded( + enum Threads { + Single, + Many(usize), + } + + let threads = { + if cfg!(target_family = "wasm") { + // When compiling to wasm, we cannot spawn extra threads + // so we have a single-threaded implementation + Threads::Single + } else { + match std::thread::available_parallelism().map(|v| v.get()) { + Err(_) => Threads::Single, + Ok(0) => unreachable!("NonZeroUsize"), + Ok(1) => Threads::Single, + Ok(reported) => match threading { + Threading::Single => Threads::Single, + Threading::AllAvailable => Threads::Many(reported), + Threading::AtMost(at_most) => Threads::Many(Ord::min(reported, at_most)), + }, + } + } + }; + + match threads { + Threads::Single => load_single_threaded( arena, load_start, src_dir, @@ -1170,9 +1192,8 @@ pub fn load<'a>( target_info, cached_subs, render, - ) - } else { - load_multi_threaded( + ), + Threads::Many(threads) => load_multi_threaded( arena, load_start, src_dir, @@ -1181,8 +1202,8 @@ pub fn load<'a>( target_info, cached_subs, render, - threading, - ) + threads, + ), } } @@ -1392,7 +1413,7 @@ fn load_multi_threaded<'a>( target_info: TargetInfo, cached_subs: MutMap)>, render: RenderTarget, - threading: Threading, + available_threads: usize, ) -> Result, LoadingProblem<'a>> { let LoadStart { arc_modules, @@ -1426,13 +1447,22 @@ fn load_multi_threaded<'a>( // doing .max(1) on the entire expression guards against // num_cpus returning 0, while also avoiding wrapping // unsigned subtraction overflow. - let default_num_workers = num_cpus::get().max(2) - 1; + // let default_num_workers = num_cpus::get().max(2) - 1; + let available_workers = available_threads - 1; let num_workers = match env::var("ROC_NUM_WORKERS") { - Ok(env_str) => env_str.parse::().unwrap_or(default_num_workers), - Err(_) => default_num_workers, + Ok(env_str) => env_str + .parse::() + .unwrap_or(available_workers) + .min(available_workers), + Err(_) => available_workers, }; + assert!( + num_workers >= 1, + "`load_multi_threaded` needs at least one worker" + ); + // an arena for every worker, stored in an arena-allocated bumpalo vec to make the lifetimes work let arenas = std::iter::repeat_with(Bump::new).take(num_workers); let worker_arenas = arena.alloc(bumpalo::collections::Vec::from_iter_in(arenas, arena)); From abbe8ecd247fa9894eded683b6e742a0df53be90 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 14:32:38 +0200 Subject: [PATCH 06/11] hook max-threads up for `roc check` --- cli/src/build.rs | 3 ++- cli/src/lib.rs | 3 ++- cli/src/main.rs | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/cli/src/build.rs b/cli/src/build.rs index 0f5241e4c3..72f9a53c05 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -350,6 +350,7 @@ pub fn check_file( src_dir: PathBuf, roc_file_path: PathBuf, emit_timings: bool, + threading: Threading, ) -> Result<(program::Problems, Duration), LoadingProblem> { let compilation_start = SystemTime::now(); @@ -368,7 +369,7 @@ pub fn check_file( target_info, // TODO: expose this from CLI? RenderTarget::ColorTerminal, - Threading::AllAvailable, + threading, )?; let buf = &mut String::with_capacity(1024); diff --git a/cli/src/lib.rs b/cli/src/lib.rs index d260866e17..79a4828e5e 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -62,8 +62,8 @@ pub fn build_app<'a>() -> Command<'a> { let flag_max_threads = Arg::new(FLAG_MAX_THREADS) .long(FLAG_MAX_THREADS) .help("Limit the number of threads (and hence cores) used during compilation.") - .takes_value(true) .requires(ROC_FILE) + .takes_value(true) .validator(|s| s.parse::()) .required(false); @@ -185,6 +185,7 @@ pub fn build_app<'a>() -> Command<'a> { .subcommand(Command::new(CMD_CHECK) .about("Check the code for problems, but doesn’t build or run it") .arg(flag_time.clone()) + .arg(flag_max_threads.clone()) .arg( Arg::new(ROC_FILE) .help("The .roc file of an app to check") diff --git a/cli/src/main.rs b/cli/src/main.rs index 0a21c66ed5..41f7868c9f 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -6,7 +6,7 @@ use roc_cli::{ FLAG_NO_LINK, FLAG_TARGET, FLAG_TIME, ROC_FILE, }; use roc_error_macros::user_error; -use roc_load::LoadingProblem; +use roc_load::{LoadingProblem, Threading}; use std::fs::{self, FileType}; use std::io; use std::path::{Path, PathBuf}; @@ -94,7 +94,17 @@ fn main() -> io::Result<()> { let roc_file_path = PathBuf::from(filename); let src_dir = roc_file_path.parent().unwrap().to_owned(); - match check_file(&arena, src_dir, roc_file_path, emit_timings) { + let threading = match matches + .value_of(roc_cli::FLAG_MAX_THREADS) + .and_then(|s| s.parse::().ok()) + { + None => Threading::AllAvailable, + Some(0) => user_error!("cannot build with at most 0 threads"), + Some(1) => Threading::Single, + Some(n) => Threading::AtMost(n), + }; + + match check_file(&arena, src_dir, roc_file_path, emit_timings, threading) { Ok((problems, total_time)) => { println!( "\x1B[{}m{}\x1B[39m {} and \x1B[{}m{}\x1B[39m {} found in {} ms.", From ec8b2cd8b1b092e4bf0f1a9cdf51b0c64056080b Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 7 May 2022 15:11:18 +0200 Subject: [PATCH 07/11] get rid of num_cpus in favor of `std::thread::available_parallelism` --- Cargo.lock | 1 - compiler/load_internal/Cargo.toml | 1 - compiler/load_internal/src/file.rs | 37 ++++++++++++++---------------- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b4691a883..e581b97656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3799,7 +3799,6 @@ dependencies = [ "indoc", "maplit", "morphic_lib", - "num_cpus", "parking_lot 0.12.0", "pretty_assertions", "roc_builtins", diff --git a/compiler/load_internal/Cargo.toml b/compiler/load_internal/Cargo.toml index df3ffe4ffb..bee842e420 100644 --- a/compiler/load_internal/Cargo.toml +++ b/compiler/load_internal/Cargo.toml @@ -27,7 +27,6 @@ ven_pretty = { path = "../../vendor/pretty" } bumpalo = { version = "3.8.0", features = ["collections"] } parking_lot = "0.12" crossbeam = "0.8.1" -num_cpus = "1.13.0" [dev-dependencies] tempfile = "3.2.0" diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index 9b9538b310..2c3b62cd42 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -747,6 +747,7 @@ impl<'a> State<'a> { ident_ids_by_module: SharedIdentIdsByModule, cached_subs: MutMap)>, render: RenderTarget, + number_of_workers: usize, ) -> Self { let arc_shorthands = Arc::new(Mutex::new(MutMap::default())); @@ -770,7 +771,7 @@ impl<'a> State<'a> { declarations_by_id: MutMap::default(), exposed_symbols_by_module: MutMap::default(), timings: MutMap::default(), - layout_caches: std::vec::Vec::with_capacity(num_cpus::get()), + layout_caches: std::vec::Vec::with_capacity(number_of_workers), cached_subs: Arc::new(Mutex::new(cached_subs)), render, } @@ -1233,6 +1234,7 @@ pub fn load_single_threaded<'a>( .send(root_msg) .map_err(|_| LoadingProblem::MsgChannelDied)?; + let number_of_workers = 1; let mut state = State::new( root_id, target_info, @@ -1242,6 +1244,7 @@ pub fn load_single_threaded<'a>( ident_ids_by_module, cached_subs, render, + number_of_workers, ); // We'll add tasks to this, and then worker threads will take tasks from it. @@ -1423,31 +1426,13 @@ fn load_multi_threaded<'a>( .. } = load_start; - let mut state = State::new( - root_id, - target_info, - goal_phase, - exposed_types, - arc_modules, - ident_ids_by_module, - cached_subs, - render, - ); - let (msg_tx, msg_rx) = bounded(1024); msg_tx .send(root_msg) .map_err(|_| LoadingProblem::MsgChannelDied)?; // Reserve one CPU for the main thread, and let all the others be eligible - // to spawn workers. We use .max(2) to enforce that we always - // end up with at least 1 worker - since (.max(2) - 1) will - // always return a number that's at least 1. Using - // .max(2) on the initial number of CPUs instead of - // doing .max(1) on the entire expression guards against - // num_cpus returning 0, while also avoiding wrapping - // unsigned subtraction overflow. - // let default_num_workers = num_cpus::get().max(2) - 1; + // to spawn workers. let available_workers = available_threads - 1; let num_workers = match env::var("ROC_NUM_WORKERS") { @@ -1463,6 +1448,18 @@ fn load_multi_threaded<'a>( "`load_multi_threaded` needs at least one worker" ); + let mut state = State::new( + root_id, + target_info, + goal_phase, + exposed_types, + arc_modules, + ident_ids_by_module, + cached_subs, + render, + num_workers, + ); + // an arena for every worker, stored in an arena-allocated bumpalo vec to make the lifetimes work let arenas = std::iter::repeat_with(Bump::new).take(num_workers); let worker_arenas = arena.alloc(bumpalo::collections::Vec::from_iter_in(arenas, arena)); From 02bff35203a33c120c04fa5c6e4bbc09bd5a75d6 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 7 May 2022 15:01:04 -0400 Subject: [PATCH 08/11] Remove dead let-identifier code --- compiler/mono/src/ir.rs | 86 ++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index e04ad60fac..5915ee0515 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -15,6 +15,7 @@ use roc_debug_flags::{ dbg_do, ROC_PRINT_IR_AFTER_REFCOUNT, ROC_PRINT_IR_AFTER_RESET_REUSE, ROC_PRINT_IR_AFTER_SPECIALIZATION, }; +use roc_error_macros::internal_error; use roc_exhaustive::{Ctor, CtorName, Guard, RenderAs, TagId}; use roc_module::ident::{ForeignSymbol, Lowercase, TagName}; use roc_module::low_level::LowLevel; @@ -5867,68 +5868,49 @@ pub fn from_can<'a>( }; if let Pattern::Identifier(symbol) = mono_pattern { - let mut hole = - env.arena - .alloc(from_can(env, variable, cont.value, procs, layout_cache)); + internal_error!("Identifier patterns should be handled in a higher code pass!") + } - for (symbol, variable, expr) in assignments { - let stmt = with_hole(env, expr, variable, procs, layout_cache, symbol, hole); + // convert the continuation + let mut stmt = from_can(env, variable, cont.value, procs, layout_cache); - hole = env.arena.alloc(stmt); - } + // layer on any default record fields + for (symbol, variable, expr) in assignments { + let specialization_symbol = procs + .symbol_specializations + .remove_single(symbol) + // Can happen when the symbol was never used under this body, and hence has no + // requested specialization. + .unwrap_or(symbol); + let hole = env.arena.alloc(stmt); + stmt = with_hole( + env, + expr, + variable, + procs, + layout_cache, + specialization_symbol, + hole, + ); + } + + if let roc_can::expr::Expr::Var(outer_symbol) = def.loc_expr.value { + store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt) + } else { + let outer_symbol = env.unique_symbol(); + stmt = store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); + + // convert the def body, store in outer_symbol with_hole( env, def.loc_expr.value, def.expr_var, procs, layout_cache, - symbol, - hole, + outer_symbol, + env.arena.alloc(stmt), ) - } else { - // convert the continuation - let mut stmt = from_can(env, variable, cont.value, procs, layout_cache); - - // layer on any default record fields - for (symbol, variable, expr) in assignments { - let specialization_symbol = procs - .symbol_specializations - .remove_single(symbol) - // Can happen when the symbol was never used under this body, and hence has no - // requested specialization. - .unwrap_or(symbol); - - let hole = env.arena.alloc(stmt); - stmt = with_hole( - env, - expr, - variable, - procs, - layout_cache, - specialization_symbol, - hole, - ); - } - - if let roc_can::expr::Expr::Var(outer_symbol) = def.loc_expr.value { - store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt) - } else { - let outer_symbol = env.unique_symbol(); - stmt = - store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); - - // convert the def body, store in outer_symbol - with_hole( - env, - def.loc_expr.value, - def.expr_var, - procs, - layout_cache, - outer_symbol, - env.arena.alloc(stmt), - ) - } } } From 7e58c4ddeab980d344baa8f215a5bf651568dcce Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 7 May 2022 15:01:25 -0400 Subject: [PATCH 09/11] Unused var --- compiler/mono/src/ir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 5915ee0515..fcd5d2ab0f 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5867,7 +5867,7 @@ pub fn from_can<'a>( Err(_) => todo!(), }; - if let Pattern::Identifier(symbol) = mono_pattern { + if let Pattern::Identifier(_symbol) = mono_pattern { internal_error!("Identifier patterns should be handled in a higher code pass!") } From 19175a85d8d45b442018c3bbf4e20cfad4ee3a79 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 7 May 2022 15:04:35 -0400 Subject: [PATCH 10/11] Drop another unused branch --- compiler/mono/src/ir.rs | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index fcd5d2ab0f..660ae0b0cb 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5867,7 +5867,9 @@ pub fn from_can<'a>( Err(_) => todo!(), }; - if let Pattern::Identifier(_symbol) = mono_pattern { + if matches!(mono_pattern, Pattern::Identifier(_symbol)) + || matches!(def.loc_expr.value, roc_can::expr::Expr::Var(..)) + { internal_error!("Identifier patterns should be handled in a higher code pass!") } @@ -5895,23 +5897,19 @@ pub fn from_can<'a>( ); } - if let roc_can::expr::Expr::Var(outer_symbol) = def.loc_expr.value { - store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt) - } else { - let outer_symbol = env.unique_symbol(); - stmt = store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); + let outer_symbol = env.unique_symbol(); + stmt = store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); - // convert the def body, store in outer_symbol - with_hole( - env, - def.loc_expr.value, - def.expr_var, - procs, - layout_cache, - outer_symbol, - env.arena.alloc(stmt), - ) - } + // convert the def body, store in outer_symbol + with_hole( + env, + def.loc_expr.value, + def.expr_var, + procs, + layout_cache, + outer_symbol, + env.arena.alloc(stmt), + ) } _ => { From 50bbf1349d41ea6394fa60c2f0a81bb996eeda52 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 7 May 2022 15:36:11 -0400 Subject: [PATCH 11/11] Revert "Drop another unused branch" This reverts commit 19175a85d8d45b442018c3bbf4e20cfad4ee3a79. --- compiler/mono/src/ir.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 660ae0b0cb..fcd5d2ab0f 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5867,9 +5867,7 @@ pub fn from_can<'a>( Err(_) => todo!(), }; - if matches!(mono_pattern, Pattern::Identifier(_symbol)) - || matches!(def.loc_expr.value, roc_can::expr::Expr::Var(..)) - { + if let Pattern::Identifier(_symbol) = mono_pattern { internal_error!("Identifier patterns should be handled in a higher code pass!") } @@ -5897,19 +5895,23 @@ pub fn from_can<'a>( ); } - let outer_symbol = env.unique_symbol(); - stmt = store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); + if let roc_can::expr::Expr::Var(outer_symbol) = def.loc_expr.value { + store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt) + } else { + let outer_symbol = env.unique_symbol(); + stmt = store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt); - // convert the def body, store in outer_symbol - with_hole( - env, - def.loc_expr.value, - def.expr_var, - procs, - layout_cache, - outer_symbol, - env.arena.alloc(stmt), - ) + // convert the def body, store in outer_symbol + with_hole( + env, + def.loc_expr.value, + def.expr_var, + procs, + layout_cache, + outer_symbol, + env.arena.alloc(stmt), + ) + } } _ => {