diff --git a/.reuse/dep5 b/.reuse/dep5 new file mode 100644 index 0000000000..f1dd1329f6 --- /dev/null +++ b/.reuse/dep5 @@ -0,0 +1,8 @@ +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ +Upstream-Name: Roc +Upstream-Contact: Richard Feldman +Source: https://github.com/rtfeldman/roc + +Files: * +Copyright: © The Roc Contributors +License: UPL-1.0 diff --git a/AUTHORS b/AUTHORS index 41dcc89d13..00f072e430 100644 --- a/AUTHORS +++ b/AUTHORS @@ -63,3 +63,4 @@ Matthias Devlamynck Jan Van Bruggen Mats Sigge <> Drew Lazzeri +Tom Dohrmann diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bbe7a3822e..748a9edd08 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,8 @@ Earthly may temporarily use a lot of disk space, up to 90 GB. This disk space is ## Contribution Tips +- Create an issue if the purpose of a struct/field/type/function/... is not immediately clear from its name or nearby comments. +- You find good first issues [here](https://github.com/rtfeldman/roc/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22). - Before making your first pull request, definitely talk to an existing contributor on [Roc Zulip](https://roc.zulipchat.com) first about what you plan to do! This can not only avoid duplicated effort, it can also avoid making a whole PR only to discover it won't be accepted because the change doesn't fit with the goals of the language's design or implementation. - It's a good idea to open a work-in-progress pull request as you begin working on something. This way, others can see that you're working on it, which avoids duplicate effort, and others can give feedback sooner rather than later if they notice a problem in the direction things are going. Be sure to include "WIP" in the title of the PR as long as it's not ready for review! - Make sure to create a branch on the roc repository for your changes. We do not allow CI to be run on forks for security. @@ -31,8 +33,6 @@ Earthly may temporarily use a lot of disk space, up to 90 GB. This disk space is git config --global commit.gpgsign true ``` -- You find good first issues [here](https://github.com/rtfeldman/roc/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22). - ## Can we do better? Feel free to open an issue if you think this document can be improved or is unclear in any way. diff --git a/README.md b/README.md index c53721ef89..777f2bc0d9 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,12 @@ For NQueens, input 10 in the terminal and press enter. **Tip:** when programming in roc, we recommend to execute `./roc check myproject/Foo.roc` before `./roc myproject/Foo.roc` or `./roc build myproject/Foo.roc`. `./roc check` can produce clear error messages in cases where building/running may panic. +## Sponsor + +We are very grateful for our sponsor [NoRedInk](https://www.noredink.com/). + +NoRedInk logo + ## Applications and Platforms Applications are often built on a *framework.* Typically, both application and framework are written in the same language. diff --git a/TUTORIAL.md b/TUTORIAL.md index fb6a1a52ba..a5e4aa97a5 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -1546,14 +1546,14 @@ an open record or a closed record: ```coffee # Closed record fullName : { firstName : Str, lastName : Str } -> Str -fullName = \user - > +fullName = \user -> "\(user.firstName) \(user.lastName)" ``` ```coffee # Open record (because of the `*`) fullName : { firstName : Str, lastName : Str }* -> Str -fullName = \user - > +fullName = \user -> "\(user.firstName) \(user.lastName)" ``` diff --git a/ast/src/module.rs b/ast/src/module.rs index f13028e8d1..f8186d5faf 100644 --- a/ast/src/module.rs +++ b/ast/src/module.rs @@ -21,7 +21,6 @@ pub fn load_module(src_file: &Path) -> LoadedModule { }), subs_by_module, TargetInfo::default_x86_64(), - roc_can::builtins::builtin_defs_map, ); match loaded { diff --git a/cli/src/build.rs b/cli/src/build.rs index 4b0daee242..4786df2e65 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -4,7 +4,6 @@ use roc_build::{ program, }; use roc_builtins::bitcode; -use roc_can::builtins::builtin_defs_map; use roc_collections::all::MutMap; use roc_load::file::LoadingProblem; use roc_mono::ir::OptLevel; @@ -74,7 +73,6 @@ pub fn build_file<'a>( src_dir.as_path(), subs_by_module, target_info, - builtin_defs_map, )?; use target_lexicon::Architecture; @@ -309,7 +307,9 @@ fn spawn_rebuild_thread( ) -> std::thread::JoinHandle { let thread_local_target = target.clone(); std::thread::spawn(move || { - print!("🔨 Rebuilding host... "); + if !precompiled { + print!("🔨 Rebuilding host... "); + } let rebuild_host_start = SystemTime::now(); if !precompiled { @@ -340,7 +340,9 @@ fn spawn_rebuild_thread( } let rebuild_host_end = rebuild_host_start.elapsed().unwrap(); - println!("Done!"); + if !precompiled { + println!("Done!"); + } rebuild_host_end.as_millis() }) @@ -372,7 +374,6 @@ pub fn check_file( src_dir.as_path(), subs_by_module, target_info, - builtin_defs_map, )?; let buf = &mut String::with_capacity(1024); diff --git a/cli/src/format.rs b/cli/src/format.rs index 8ced266bce..f5a32c9caa 100644 --- a/cli/src/format.rs +++ b/cli/src/format.rs @@ -1,5 +1,6 @@ use std::path::PathBuf; +use crate::FormatMode; use bumpalo::collections::Vec; use bumpalo::Bump; use roc_error_macros::{internal_error, user_error}; @@ -24,7 +25,7 @@ use roc_parse::{ }; use roc_region::all::{Loc, Region}; -pub fn format(files: std::vec::Vec) { +pub fn format(files: std::vec::Vec, mode: FormatMode) -> Result<(), String> { for file in files { let arena = Bump::new(); @@ -99,9 +100,22 @@ pub fn format(files: std::vec::Vec) { unstable_2_file.display()); } - // If all the checks above passed, actually write out the new file. - std::fs::write(&file, buf.as_str()).unwrap(); + match mode { + FormatMode::CheckOnly => { + // If we notice that this file needs to be formatted, return early + if buf.as_str() != src { + return Err("One or more files need to be reformatted.".to_string()); + } + } + + FormatMode::Format => { + // If all the checks above passed, actually write out the new file. + std::fs::write(&file, buf.as_str()).unwrap(); + } + } } + + Ok(()) } #[derive(Debug, PartialEq)] diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 136f99eab7..e3027927ea 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -37,6 +37,7 @@ pub const FLAG_TIME: &str = "time"; pub const FLAG_LINK: &str = "roc-linker"; pub const FLAG_PRECOMPILED: &str = "precompiled-host"; pub const FLAG_VALGRIND: &str = "valgrind"; +pub const FLAG_CHECK: &str = "check"; pub const ROC_FILE: &str = "ROC_FILE"; pub const ROC_DIR: &str = "ROC_DIR"; pub const BACKEND: &str = "BACKEND"; @@ -122,6 +123,12 @@ pub fn build_app<'a>() -> App<'a> { .index(1) .multiple_values(true) .required(false)) + .arg( + Arg::new(FLAG_CHECK) + .long(FLAG_CHECK) + .about("Checks that specified files are formatted. If formatting is needed, it will return a non-zero exit code.") + .required(false), + ) ) .subcommand(App::new(CMD_VERSION) .about("Print version information") @@ -242,6 +249,11 @@ pub enum BuildConfig { BuildAndRun { roc_file_arg_index: usize }, } +pub enum FormatMode { + Format, + CheckOnly, +} + pub fn build(matches: &ArgMatches, config: BuildConfig) -> io::Result { use build::build_file; use std::str::FromStr; diff --git a/cli/src/main.rs b/cli/src/main.rs index 7fba8781c5..a90ca61554 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -1,7 +1,7 @@ use roc_cli::build::check_file; use roc_cli::{ - build_app, docs, format, BuildConfig, CMD_BUILD, CMD_CHECK, CMD_DOCS, CMD_EDIT, CMD_FORMAT, - CMD_REPL, CMD_VERSION, DIRECTORY_OR_FILES, FLAG_TIME, ROC_FILE, + build_app, docs, format, BuildConfig, FormatMode, CMD_BUILD, CMD_CHECK, CMD_DOCS, CMD_EDIT, + CMD_FORMAT, CMD_REPL, CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_TIME, ROC_FILE, }; use roc_load::file::LoadingProblem; use std::fs::{self, FileType}; @@ -150,9 +150,20 @@ fn main() -> io::Result<()> { roc_files_recursive(os_str.as_os_str(), metadata.file_type(), &mut roc_files)?; } - format(roc_files); + let format_mode = match matches.is_present(FLAG_CHECK) { + true => FormatMode::CheckOnly, + false => FormatMode::Format, + }; - Ok(0) + let format_exit_code = match format(roc_files, format_mode) { + Ok(_) => 0, + Err(message) => { + eprintln!("{}", message); + 1 + } + }; + + Ok(format_exit_code) } Some((CMD_VERSION, _)) => { println!("roc {}", concatcp!(include_str!("../../version.txt"), "\n")); diff --git a/cli_utils/Cargo.lock b/cli_utils/Cargo.lock index f6cf4cf5a9..c25e9072c9 100644 --- a/cli_utils/Cargo.lock +++ b/cli_utils/Cargo.lock @@ -964,12 +964,6 @@ dependencies = [ "toml", ] -[[package]] -name = "fixedbitset" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d" - [[package]] name = "flate2" version = "1.0.22" @@ -2047,14 +2041,11 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d76e8e1493bcac0d2766c42737f34458f1c8c50c0d23bcb24ea953affb273216" dependencies = [ - "backtrace", "cfg-if 1.0.0", "instant", "libc", - "petgraph", "redox_syscall", "smallvec", - "thread-id", "winapi", ] @@ -2107,16 +2098,6 @@ dependencies = [ "sha-1", ] -[[package]] -name = "petgraph" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "467d164a6de56270bd7c4d070df81d07beace25012d5103ced4e9ff08d6afdb7" -dependencies = [ - "fixedbitset", - "indexmap", -] - [[package]] name = "phf" version = "0.9.0" @@ -2790,6 +2771,7 @@ dependencies = [ "roc_types", "roc_unify", "ven_pretty", + "wasm-bindgen", ] [[package]] @@ -2799,6 +2781,7 @@ dependencies = [ "bumpalo", "lazy_static", "roc_collections", + "roc_error_macros", "roc_ident", "roc_region", "snafu", @@ -2815,6 +2798,7 @@ dependencies = [ "roc_builtins", "roc_can", "roc_collections", + "roc_error_macros", "roc_module", "roc_problem", "roc_region", @@ -2865,6 +2849,7 @@ dependencies = [ "inkwell 0.1.0", "libloading 0.7.1", "roc_build", + "roc_builtins", "roc_collections", "roc_gen_llvm", "roc_load", @@ -2895,6 +2880,7 @@ dependencies = [ "roc_reporting", "roc_target", "roc_types", + "wasm-bindgen", ] [[package]] @@ -2927,6 +2913,7 @@ dependencies = [ "roc_region", "roc_types", "roc_unify", + "wasm-bindgen", ] [[package]] @@ -2946,6 +2933,7 @@ version = "0.1.0" dependencies = [ "bumpalo", "roc_collections", + "roc_error_macros", "roc_module", "roc_region", "static_assertions", @@ -2956,6 +2944,7 @@ dependencies = [ name = "roc_unify" version = "0.1.0" dependencies = [ + "bitflags", "roc_collections", "roc_module", "roc_types", @@ -3402,17 +3391,6 @@ dependencies = [ "syn", ] -[[package]] -name = "thread-id" -version = "4.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fdfe0627923f7411a43ec9ec9c39c3a9b4151be313e0922042581fb6c9b717f" -dependencies = [ - "libc", - "redox_syscall", - "winapi", -] - [[package]] name = "threadpool" version = "1.8.1" @@ -3583,9 +3561,9 @@ checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" [[package]] name = "wasm-bindgen" -version = "0.2.78" +version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "632f73e236b219150ea279196e54e610f5dbafa5d61786303d4da54f84e47fce" +checksum = "25f1af7423d8588a3d840681122e72e6a24ddbcb3f0ec385cac0d12d24256c06" dependencies = [ "cfg-if 1.0.0", "wasm-bindgen-macro", @@ -3593,9 +3571,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-backend" -version = "0.2.78" +version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a317bf8f9fba2476b4b2c85ef4c4af8ff39c3c7f0cdfeed4f82c34a880aa837b" +checksum = "8b21c0df030f5a177f3cba22e9bc4322695ec43e7257d865302900290bcdedca" dependencies = [ "bumpalo", "lazy_static", @@ -3620,9 +3598,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.78" +version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d56146e7c495528bf6587663bea13a8eb588d39b36b679d83972e1a2dbbdacf9" +checksum = "2f4203d69e40a52ee523b2529a773d5ffc1dc0071801c87b3d270b471b80ed01" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -3630,9 +3608,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.78" +version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7803e0eea25835f8abdc585cd3021b3deb11543c6fe226dcd30b228857c5c5ab" +checksum = "bfa8a30d46208db204854cadbb5d4baf5fcf8071ba5bf48190c3e59937962ebc" dependencies = [ "proc-macro2", "quote", @@ -3643,9 +3621,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.78" +version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0237232789cf037d5480773fe568aac745bfe2afbc11a863e97901780a6b47cc" +checksum = "3d958d035c4438e28c70e4321a2911302f10135ce78a9c7834c0cab4123d06a2" [[package]] name = "wayland-client" diff --git a/compiler/builtins/bitcode/src/list.zig b/compiler/builtins/bitcode/src/list.zig index e94941f4ed..a5e237d4cd 100644 --- a/compiler/builtins/bitcode/src/list.zig +++ b/compiler/builtins/bitcode/src/list.zig @@ -210,6 +210,7 @@ pub fn listMap( } } +// List.mapWithIndex : List before, (before, Nat -> after) -> List after pub fn listMapWithIndex( list: RocList, caller: Caller2, @@ -231,7 +232,8 @@ pub fn listMapWithIndex( } while (i < size) : (i += 1) { - caller(data, @ptrCast(?[*]u8, &i), source_ptr + (i * old_element_width), target_ptr + (i * new_element_width)); + // before, Nat -> after + caller(data, source_ptr + (i * old_element_width), @ptrCast(?[*]u8, &i), target_ptr + (i * new_element_width)); } return output; diff --git a/compiler/builtins/docs/List.roc b/compiler/builtins/docs/List.roc index 9d6a58bdbe..25ddbc6344 100644 --- a/compiler/builtins/docs/List.roc +++ b/compiler/builtins/docs/List.roc @@ -207,7 +207,7 @@ empty : List * ## Returns a list with the given length, where every element is the given value. ## ## -repeat : Nat, elem -> List elem +repeat : elem, Nat -> List elem ## Returns a list of all the integers between one and another, ## including both of the given numbers. @@ -279,7 +279,7 @@ map4 : List a, List b, List c, List d, (a, b, c, d -> e) -> List e ## This works like [List.map], except it also passes the index ## of the element to the conversion function. -mapWithIndex : List before, (Nat, before -> after) -> List after +mapWithIndex : List before, (before, Nat -> after) -> List after ## This works like [List.map], except at any time you can return `Err` to ## cancel the entire operation immediately, and return that #Err. diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index 15276e9e8b..8a4ee50b2a 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -1078,14 +1078,14 @@ pub fn types() -> MutMap { Box::new(list_type(flex(TVAR2))), ); - // mapWithIndex : List before, (Nat, before -> after) -> List after + // mapWithIndex : List before, (before, Nat -> after) -> List after { let_tvars! { cvar, before, after}; add_top_level_function_type!( Symbol::LIST_MAP_WITH_INDEX, vec![ list_type(flex(before)), - closure(vec![nat_type(), flex(before)], cvar, Box::new(flex(after))), + closure(vec![flex(before), nat_type()], cvar, Box::new(flex(after))), ], Box::new(list_type(flex(after))), ) @@ -1264,10 +1264,10 @@ pub fn types() -> MutMap { Box::new(list_type(flex(TVAR1))) ); - // repeat : Nat, elem -> List elem + // repeat : elem, Nat -> List elem add_top_level_function_type!( Symbol::LIST_REPEAT, - vec![nat_type(), flex(TVAR1)], + vec![flex(TVAR1), nat_type()], Box::new(list_type(flex(TVAR1))), ); diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index 3a8d1ba9e4..3c21b53b9a 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -3300,7 +3300,7 @@ fn list_map(symbol: Symbol, var_store: &mut VarStore) -> Def { lowlevel_2(symbol, LowLevel::ListMap, var_store) } -/// List.mapWithIndex : List before, (Nat, before -> after) -> List after +/// List.mapWithIndex : List before, (before, Nat -> after) -> List after fn list_map_with_index(symbol: Symbol, var_store: &mut VarStore) -> Def { lowlevel_2(symbol, LowLevel::ListMapWithIndex, var_store) } diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 2e046dbb07..7e03f8e3b9 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -9,6 +9,7 @@ use crate::expr::{ }; use crate::pattern::{bindings_from_patterns, canonicalize_pattern, Pattern}; use crate::procedure::References; +use crate::scope::create_alias; use crate::scope::Scope; use roc_collections::all::{default_hasher, ImMap, ImSet, MutMap, MutSet, SendMap}; use roc_module::ident::Lowercase; @@ -22,7 +23,7 @@ use roc_types::subs::{VarStore, Variable}; use roc_types::types::{Alias, Type}; use std::collections::HashMap; use std::fmt::Debug; -use ven_graph::{strongly_connected_components, topological_sort, topological_sort_into_groups}; +use ven_graph::{strongly_connected_components, topological_sort}; #[derive(Clone, Debug, PartialEq)] pub struct Def { @@ -265,8 +266,7 @@ pub fn canonicalize_defs<'a>( let (name, vars, ann) = alias_defs.remove(&alias_name).unwrap(); let symbol = name.value; - let mut can_ann = - canonicalize_annotation(env, &mut scope, &ann.value, ann.region, var_store); + let can_ann = canonicalize_annotation(env, &mut scope, &ann.value, ann.region, var_store); // Record all the annotation's references in output.references.lookups for symbol in can_ann.references { @@ -303,43 +303,21 @@ pub fn canonicalize_defs<'a>( continue; } - let mut is_nested_datatype = false; - if can_ann.typ.contains_symbol(symbol) { - let alias_args = can_vars - .iter() - .map(|l| (l.value.0.clone(), Type::Variable(l.value.1))) - .collect::>(); - let alias_region = - Region::across_all([name.region].iter().chain(vars.iter().map(|l| &l.region))); - - let made_recursive = make_tag_union_recursive( - env, - Loc::at(alias_region, (symbol, &alias_args)), - name.region, - vec![], - &mut can_ann.typ, - var_store, - // Don't report any errors yet. We'll take care of self and mutual - // recursion errors after the sorted introductions are complete. - &mut false, - ); - - is_nested_datatype = made_recursive.is_err(); - } - - if is_nested_datatype { - // Bail out - continue; - } - - scope.add_alias(symbol, name.region, can_vars.clone(), can_ann.typ.clone()); - let alias = scope.lookup_alias(symbol).expect("alias is added to scope"); + let alias = create_alias(symbol, name.region, can_vars.clone(), can_ann.typ.clone()); aliases.insert(symbol, alias.clone()); } // Now that we know the alias dependency graph, we can try to insert recursion variables // where aliases are recursive tag unions, or detect illegal recursions. - correct_mutual_recursive_type_alias(env, &mut aliases, var_store); + let mut aliases = correct_mutual_recursive_type_alias(env, &aliases, var_store); + for (symbol, alias) in aliases.iter() { + scope.add_alias( + *symbol, + alias.region, + alias.type_variables.clone(), + alias.typ.clone(), + ); + } // Now that we have the scope completely assembled, and shadowing resolved, // we're ready to canonicalize any body exprs. @@ -1564,17 +1542,17 @@ fn pending_typed_body<'a>( /// Make aliases recursive fn correct_mutual_recursive_type_alias<'a>( env: &mut Env<'a>, - aliases: &mut SendMap, + original_aliases: &SendMap, var_store: &mut VarStore, -) { +) -> SendMap { let mut symbols_introduced = ImSet::default(); - for (key, _) in aliases.iter() { + for (key, _) in original_aliases.iter() { symbols_introduced.insert(*key); } let all_successors_with_self = |symbol: &Symbol| -> ImSet { - match aliases.get(symbol) { + match original_aliases.get(symbol) { Some(alias) => { let mut loc_succ = alias.typ.symbols(); // remove anything that is not defined in the current block @@ -1586,80 +1564,120 @@ fn correct_mutual_recursive_type_alias<'a>( } }; - let all_successors_without_self = |symbol: &Symbol| -> ImSet { - match aliases.get(symbol) { - Some(alias) => { - let mut loc_succ = alias.typ.symbols(); - // remove anything that is not defined in the current block - loc_succ.retain(|key| symbols_introduced.contains(key)); - loc_succ.remove(symbol); - - loc_succ - } - None => ImSet::default(), - } - }; - - let originals = aliases.clone(); - // TODO investigate should this be in a loop? - let defined_symbols: Vec = aliases.keys().copied().collect(); + let defined_symbols: Vec = original_aliases.keys().copied().collect(); - // split into self-recursive and mutually recursive - match topological_sort_into_groups(&defined_symbols, all_successors_with_self) { - Ok(_) => { - // no mutual recursion in any alias - } - Err((_, mutually_recursive_symbols)) => { - for cycle in strongly_connected_components( - &mutually_recursive_symbols, - all_successors_without_self, - ) { - // make sure we report only one error for the cycle, not an error for every - // alias in the cycle. - let mut can_still_report_error = true; + let cycles = strongly_connected_components(&defined_symbols, all_successors_with_self); + let mut solved_aliases = SendMap::default(); - // TODO use itertools to be more efficient here - for rec in &cycle { - let mut to_instantiate = ImMap::default(); - let mut others = Vec::with_capacity(cycle.len() - 1); - for other in &cycle { - if rec != other { - others.push(*other); - if let Some(alias) = originals.get(other) { - to_instantiate.insert(*other, alias.clone()); - } - } - } + for cycle in cycles { + debug_assert!(!cycle.is_empty()); - if let Some(alias) = aliases.get_mut(rec) { - alias.typ.instantiate_aliases( - alias.region, - &to_instantiate, - var_store, - &mut ImSet::default(), - ); + let mut pending_aliases: SendMap<_, _> = cycle + .iter() + .map(|&sym| (sym, original_aliases.get(&sym).unwrap().clone())) + .collect(); - let alias_args = &alias - .type_variables - .iter() - .map(|l| (l.value.0.clone(), Type::Variable(l.value.1))) - .collect::>(); + // Make sure we report only one error for the cycle, not an error for every + // alias in the cycle. + let mut can_still_report_error = true; - let _made_recursive = make_tag_union_recursive( - env, - Loc::at(alias.header_region(), (*rec, alias_args)), - alias.region, - others, - &mut alias.typ, - var_store, - &mut can_still_report_error, - ); + for &rec in cycle.iter() { + // First, we need to instantiate the alias with any symbols in the currrent module it + // depends on. + // We only need to worry about symbols in this SCC or any prior one, since the SCCs + // were sorted topologically, and we've already instantiated aliases coming from other + // modules. + let mut to_instantiate: ImMap<_, _> = solved_aliases.clone().into_iter().collect(); + let mut others_in_scc = Vec::with_capacity(cycle.len() - 1); + for &other in cycle.iter() { + if rec != other { + others_in_scc.push(other); + if let Some(alias) = original_aliases.get(&other) { + to_instantiate.insert(other, alias.clone()); } } } + + let alias = pending_aliases.get_mut(&rec).unwrap(); + alias.typ.instantiate_aliases( + alias.region, + &to_instantiate, + var_store, + &mut ImSet::default(), + ); + + // Now mark the alias recursive, if it needs to be. + let is_self_recursive = alias.typ.contains_symbol(rec); + let is_mutually_recursive = cycle.len() > 1; + + if is_self_recursive || is_mutually_recursive { + let _made_recursive = make_tag_union_of_alias_recursive( + env, + rec, + alias, + vec![], + var_store, + &mut can_still_report_error, + ); + } } + + // The cycle we just instantiated and marked recursive may still be an illegal cycle, if + // all the types in the cycle are narrow newtypes. We can't figure this out until now, + // because we need all the types to be deeply instantiated. + let all_are_narrow = cycle.iter().all(|sym| { + let typ = &pending_aliases.get(sym).unwrap().typ; + matches!(typ, Type::RecursiveTagUnion(..)) && typ.is_narrow() + }); + + if all_are_narrow { + // This cycle is illegal! + let mut rest = cycle; + let alias_name = rest.pop().unwrap(); + + let alias = pending_aliases.get_mut(&alias_name).unwrap(); + + mark_cyclic_alias( + env, + &mut alias.typ, + alias_name, + alias.region, + rest, + can_still_report_error, + ) + } + + // Now, promote all resolved aliases in this cycle as solved. + solved_aliases.extend(pending_aliases); } + + solved_aliases +} + +fn make_tag_union_of_alias_recursive<'a>( + env: &mut Env<'a>, + alias_name: Symbol, + alias: &mut Alias, + others: Vec, + var_store: &mut VarStore, + can_report_error: &mut bool, +) -> Result<(), ()> { + let alias_args = alias + .type_variables + .iter() + .map(|l| (l.value.0.clone(), Type::Variable(l.value.1))) + .collect::>(); + + make_tag_union_recursive_help( + env, + Loc::at(alias.header_region(), (alias_name, &alias_args)), + alias.region, + others, + &mut alias.typ, + var_store, + can_report_error, + ) } /// Attempt to make a tag union recursive at the position of `recursive_alias`; for example, @@ -1683,7 +1701,7 @@ fn correct_mutual_recursive_type_alias<'a>( /// ``` /// /// When `Err` is returned, a problem will be added to `env`. -fn make_tag_union_recursive<'a>( +fn make_tag_union_recursive_help<'a>( env: &mut Env<'a>, recursive_alias: Loc<(Symbol, &[(Lowercase, Type)])>, region: Region, @@ -1724,7 +1742,7 @@ fn make_tag_union_recursive<'a>( actual, type_arguments, .. - } => make_tag_union_recursive( + } => make_tag_union_recursive_help( env, Loc::at_zero((symbol, type_arguments)), region, @@ -1734,17 +1752,27 @@ fn make_tag_union_recursive<'a>( can_report_error, ), _ => { - let problem = roc_types::types::Problem::CyclicAlias(symbol, region, others.clone()); - *typ = Type::Erroneous(problem); + mark_cyclic_alias(env, typ, symbol, region, others, *can_report_error); + *can_report_error = false; - // ensure cyclic error is only reported for one element of the cycle - if *can_report_error { - *can_report_error = false; - - let problem = Problem::CyclicAlias(symbol, region, others); - env.problems.push(problem); - } Ok(()) } } } + +fn mark_cyclic_alias<'a>( + env: &mut Env<'a>, + typ: &mut Type, + symbol: Symbol, + region: Region, + others: Vec, + report: bool, +) { + let problem = roc_types::types::Problem::CyclicAlias(symbol, region, others.clone()); + *typ = Type::Erroneous(problem); + + if report { + let problem = Problem::CyclicAlias(symbol, region, others); + env.problems.push(problem); + } +} diff --git a/compiler/can/src/module.rs b/compiler/can/src/module.rs index 8fe6ff5cef..8deb5a9671 100644 --- a/compiler/can/src/module.rs +++ b/compiler/can/src/module.rs @@ -67,7 +67,7 @@ fn validate_generate_with<'a>( // TODO trim these down #[allow(clippy::too_many_arguments)] -pub fn canonicalize_module_defs<'a, F>( +pub fn canonicalize_module_defs<'a>( arena: &Bump, loc_defs: &'a [Loc>], header_for: &roc_parse::header::HeaderFor, @@ -79,11 +79,7 @@ pub fn canonicalize_module_defs<'a, F>( exposed_imports: MutMap, exposed_symbols: &MutSet, var_store: &mut VarStore, - look_up_builtin: F, -) -> Result -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result { let mut can_exposed_imports = MutMap::default(); let mut scope = Scope::new(home, var_store); let mut env = Env::new(home, dep_idents, module_ids, exposed_ident_ids); @@ -482,7 +478,7 @@ where for symbol in references.iter() { if symbol.is_builtin() { // this can fail when the symbol is for builtin types, or has no implementation yet - if let Some(def) = look_up_builtin(*symbol, var_store) { + if let Some(def) = crate::builtins::builtin_defs_map(*symbol, var_store) { declarations.push(Declaration::Builtin(def)); } } diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index de7b233b08..56dc837441 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -181,42 +181,7 @@ impl Scope { vars: Vec>, typ: Type, ) { - let roc_types::types::VariableDetail { - type_variables, - lambda_set_variables, - recursion_variables, - } = typ.variables_detail(); - - debug_assert!({ - let mut hidden = type_variables; - - for loc_var in vars.iter() { - hidden.remove(&loc_var.value.1); - } - - if !hidden.is_empty() { - panic!( - "Found unbound type variables {:?} \n in type alias {:?} {:?} : {:?}", - hidden, name, &vars, &typ - ) - } - - true - }); - - let lambda_set_variables: Vec<_> = lambda_set_variables - .into_iter() - .map(|v| roc_types::types::LambdaSet(Type::Variable(v))) - .collect(); - - let alias = Alias { - region, - type_variables: vars, - lambda_set_variables, - recursion_variables, - typ, - }; - + let alias = create_alias(name, region, vars, typ); self.aliases.insert(name, alias); } @@ -224,3 +189,46 @@ impl Scope { self.aliases.contains_key(&name) } } + +pub fn create_alias( + name: Symbol, + region: Region, + vars: Vec>, + typ: Type, +) -> Alias { + let roc_types::types::VariableDetail { + type_variables, + lambda_set_variables, + recursion_variables, + } = typ.variables_detail(); + + debug_assert!({ + let mut hidden = type_variables; + + for loc_var in vars.iter() { + hidden.remove(&loc_var.value.1); + } + + if !hidden.is_empty() { + panic!( + "Found unbound type variables {:?} \n in type alias {:?} {:?} : {:?}", + hidden, name, &vars, &typ + ) + } + + true + }); + + let lambda_set_variables: Vec<_> = lambda_set_variables + .into_iter() + .map(|v| roc_types::types::LambdaSet(Type::Variable(v))) + .collect(); + + Alias { + region, + type_variables: vars, + lambda_set_variables, + recursion_variables, + typ, + } +} diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 9ca24e1739..e1a1a981e4 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -5014,7 +5014,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( } } ListMapWithIndex { xs } => { - // List.mapWithIndex : List before, (Nat, before -> after) -> List after + // List.mapWithIndex : List before, (before, Nat -> after) -> List after let (list, list_layout) = load_symbol_and_layout(scope, xs); let (function, closure, closure_layout) = function_details!(); @@ -5024,7 +5024,7 @@ fn run_higher_order_low_level<'a, 'ctx, 'env>( Layout::Builtin(Builtin::List(element_layout)), Layout::Builtin(Builtin::List(result_layout)), ) => { - let argument_layouts = &[Layout::usize(env.target_info), **element_layout]; + let argument_layouts = &[**element_layout, Layout::usize(env.target_info)]; let roc_function_call = roc_function_call( env, @@ -5484,13 +5484,13 @@ fn run_low_level<'a, 'ctx, 'env>( list_single(env, arg, arg_layout) } ListRepeat => { - // List.repeat : Int, elem -> List elem + // List.repeat : elem, Nat -> List elem debug_assert_eq!(args.len(), 2); - let list_len = load_symbol(scope, &args[0]).into_int_value(); - let (elem, elem_layout) = load_symbol_and_layout(scope, &args[1]); + let (elem, elem_layout) = load_symbol_and_layout(scope, &args[0]); + let list_len = load_symbol(scope, &args[1]).into_int_value(); - list_repeat(env, layout_ids, list_len, elem, elem_layout) + list_repeat(env, layout_ids, elem, elem_layout, list_len) } ListReverse => { // List.reverse : List elem -> List elem @@ -5692,7 +5692,7 @@ fn run_low_level<'a, 'ctx, 'env>( match arg_builtin { Int(int_width) => { let int_type = convert::int_type_from_int_width(env, *int_width); - build_int_unary_op(env, arg.into_int_value(), int_type, op) + build_int_unary_op(env, arg.into_int_value(), int_type, op, layout) } Float(float_width) => build_float_unary_op( env, @@ -6924,6 +6924,7 @@ fn build_int_unary_op<'a, 'ctx, 'env>( arg: IntValue<'ctx>, int_type: IntType<'ctx>, op: LowLevel, + return_layout: &Layout<'a>, ) -> BasicValueEnum<'ctx> { use roc_module::low_level::LowLevel::*; @@ -6939,12 +6940,19 @@ fn build_int_unary_op<'a, 'ctx, 'env>( int_abs_raise_on_overflow(env, arg, int_type) } NumToFloat => { - // TODO: Handle different sized numbers // This is an Int, so we need to convert it. + + let target_float_type = match return_layout { + Layout::Builtin(Builtin::Float(float_width)) => { + convert::float_type_from_float_width(env, *float_width) + } + _ => internal_error!("There can only be floats here!"), + }; + bd.build_cast( InstructionOpcode::SIToFP, arg, - env.context.f64_type(), + target_float_type, "i64_to_f64", ) } diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index 16ef7bdde8..828520f5d2 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -119,13 +119,13 @@ pub fn list_single<'a, 'ctx, 'env>( ) } -/// List.repeat : Int, elem -> List elem +/// List.repeat : elem, Nat -> List elem pub fn list_repeat<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout_ids: &mut LayoutIds<'a>, - list_len: IntValue<'ctx>, element: BasicValueEnum<'ctx>, element_layout: &Layout<'a>, + list_len: IntValue<'ctx>, ) -> BasicValueEnum<'ctx> { let inc_element_fn = build_inc_n_wrapper(env, layout_ids, element_layout); @@ -687,7 +687,7 @@ pub fn list_sort_with<'a, 'ctx, 'env>( ) } -/// List.mapWithIndex : List before, (Nat, before -> after) -> List after +/// List.mapWithIndex : List before, (before, Nat -> after) -> List after pub fn list_map_with_index<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, roc_function_call: RocFunctionCall<'ctx>, diff --git a/compiler/ident/src/lib.rs b/compiler/ident/src/lib.rs index 7da0e914ca..e152271bc4 100644 --- a/compiler/ident/src/lib.rs +++ b/compiler/ident/src/lib.rs @@ -311,7 +311,7 @@ impl Clone for IdentStr { impl Drop for IdentStr { fn drop(&mut self) { - if !self.is_small_str() { + if !self.is_empty() && !self.is_small_str() { unsafe { let align = mem::align_of::(); let layout = Layout::from_size_align_unchecked(self.length, align); diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 13802dbcdf..0a119ecb23 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -6,7 +6,7 @@ use crossbeam::thread; use parking_lot::Mutex; use roc_builtins::std::StdLib; use roc_can::constraint::Constraint; -use roc_can::def::{Declaration, Def}; +use roc_can::def::Declaration; use roc_can::module::{canonicalize_module_defs, Module}; use roc_collections::all::{default_hasher, BumpMap, MutMap, MutSet}; use roc_constrain::module::{ @@ -40,15 +40,18 @@ use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::collections::{HashMap, HashSet}; use std::io; use std::iter; +use std::ops::ControlFlow; use std::path::{Path, PathBuf}; use std::str::from_utf8_unchecked; use std::sync::Arc; use std::{env, fs}; +use crate::work::{Dependencies, Phase}; + +#[cfg(target_family = "wasm")] +use crate::wasm_system_time::{Duration, SystemTime}; #[cfg(not(target_family = "wasm"))] use std::time::{Duration, SystemTime}; -#[cfg(target_family = "wasm")] -use wasm_system_time::{Duration, SystemTime}; /// Default name for the binary generated for an app, if an invalid one was specified. const DEFAULT_APP_OUTPUT_PATH: &str = "app"; @@ -71,253 +74,6 @@ macro_rules! log { ($($arg:tt)*) => (if SHOW_MESSAGE_LOG { println!($($arg)*); } else {}) } -/// NOTE the order of definition of the phases is used by the ord instance -/// make sure they are ordered from first to last! -#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug)] -pub enum Phase { - LoadHeader, - Parse, - CanonicalizeAndConstrain, - SolveTypes, - FindSpecializations, - MakeSpecializations, -} - -/// NOTE keep up to date manually, from ParseAndGenerateConstraints to the highest phase we support -const PHASES: [Phase; 6] = [ - Phase::LoadHeader, - Phase::Parse, - Phase::CanonicalizeAndConstrain, - Phase::SolveTypes, - Phase::FindSpecializations, - Phase::MakeSpecializations, -]; - -#[derive(Debug)] -enum Status { - NotStarted, - Pending, - Done, -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -enum Job<'a> { - Step(ModuleId, Phase), - ResolveShorthand(&'a str), -} - -#[derive(Default, Debug)] -struct Dependencies<'a> { - waiting_for: MutMap, MutSet>>, - notifies: MutMap, MutSet>>, - status: MutMap, Status>, -} - -impl<'a> Dependencies<'a> { - /// Add all the dependencies for a module, return (module, phase) pairs that can make progress - pub fn add_module( - &mut self, - module_id: ModuleId, - dependencies: &MutSet>, - goal_phase: Phase, - ) -> MutSet<(ModuleId, Phase)> { - use Phase::*; - - let mut output = MutSet::default(); - - for dep in dependencies.iter() { - let has_package_dependency = self.add_package_dependency(dep, Phase::LoadHeader); - - let dep = *dep.as_inner(); - - if !has_package_dependency { - // loading can start immediately on this dependency - output.insert((dep, Phase::LoadHeader)); - } - - // to parse and generate constraints, the headers of all dependencies must be loaded! - // otherwise, we don't know whether an imported symbol is actually exposed - self.add_dependency_help(module_id, dep, Phase::Parse, Phase::LoadHeader); - - // to canonicalize a module, all its dependencies must be canonicalized - self.add_dependency(module_id, dep, Phase::CanonicalizeAndConstrain); - - // to typecheck a module, all its dependencies must be type checked already - self.add_dependency(module_id, dep, Phase::SolveTypes); - - if goal_phase >= FindSpecializations { - self.add_dependency(module_id, dep, Phase::FindSpecializations); - } - - if goal_phase >= MakeSpecializations { - self.add_dependency(dep, module_id, Phase::MakeSpecializations); - } - } - - // add dependencies for self - // phase i + 1 of a file always depends on phase i being completed - { - let mut i = 0; - while PHASES[i] < goal_phase { - self.add_dependency_help(module_id, module_id, PHASES[i + 1], PHASES[i]); - i += 1; - } - } - - self.add_to_status(module_id, goal_phase); - - output - } - - fn add_to_status(&mut self, module_id: ModuleId, goal_phase: Phase) { - for phase in PHASES.iter() { - if *phase > goal_phase { - break; - } - - if let Vacant(entry) = self.status.entry(Job::Step(module_id, *phase)) { - entry.insert(Status::NotStarted); - } - } - } - - /// Propagate a notification, return (module, phase) pairs that can make progress - pub fn notify(&mut self, module_id: ModuleId, phase: Phase) -> MutSet<(ModuleId, Phase)> { - self.notify_help(Job::Step(module_id, phase)) - } - - /// Propagate a notification, return (module, phase) pairs that can make progress - pub fn notify_package(&mut self, shorthand: &'a str) -> MutSet<(ModuleId, Phase)> { - self.notify_help(Job::ResolveShorthand(shorthand)) - } - - fn notify_help(&mut self, key: Job<'a>) -> MutSet<(ModuleId, Phase)> { - self.status.insert(key.clone(), Status::Done); - - let mut output = MutSet::default(); - - if let Some(to_notify) = self.notifies.get(&key) { - for notify_key in to_notify { - let mut is_empty = false; - if let Some(waiting_for_pairs) = self.waiting_for.get_mut(notify_key) { - waiting_for_pairs.remove(&key); - is_empty = waiting_for_pairs.is_empty(); - } - - if is_empty { - self.waiting_for.remove(notify_key); - - if let Job::Step(module, phase) = *notify_key { - output.insert((module, phase)); - } - } - } - } - - self.notifies.remove(&key); - - output - } - - fn add_package_dependency( - &mut self, - module: &PackageQualified<'a, ModuleId>, - next_phase: Phase, - ) -> bool { - match module { - PackageQualified::Unqualified(_) => { - // no dependency, we can just start loading the file - false - } - PackageQualified::Qualified(shorthand, module_id) => { - let job = Job::ResolveShorthand(shorthand); - let next_step = Job::Step(*module_id, next_phase); - match self.status.get(&job) { - None | Some(Status::NotStarted) | Some(Status::Pending) => { - // this shorthand is not resolved, add a dependency - { - let entry = self - .waiting_for - .entry(next_step.clone()) - .or_insert_with(Default::default); - - entry.insert(job.clone()); - } - - { - let entry = self.notifies.entry(job).or_insert_with(Default::default); - - entry.insert(next_step); - } - - true - } - Some(Status::Done) => { - // shorthand is resolved; no dependency - false - } - } - } - } - } - - /// A waits for B, and B will notify A when it completes the phase - fn add_dependency(&mut self, a: ModuleId, b: ModuleId, phase: Phase) { - self.add_dependency_help(a, b, phase, phase); - } - - /// phase_a of module a is waiting for phase_b of module_b - fn add_dependency_help(&mut self, a: ModuleId, b: ModuleId, phase_a: Phase, phase_b: Phase) { - // no need to wait if the dependency is already done! - if let Some(Status::Done) = self.status.get(&Job::Step(b, phase_b)) { - return; - } - - let key = Job::Step(a, phase_a); - let value = Job::Step(b, phase_b); - match self.waiting_for.get_mut(&key) { - Some(existing) => { - existing.insert(value); - } - None => { - let mut set = MutSet::default(); - set.insert(value); - self.waiting_for.insert(key, set); - } - } - - let key = Job::Step(b, phase_b); - let value = Job::Step(a, phase_a); - match self.notifies.get_mut(&key) { - Some(existing) => { - existing.insert(value); - } - None => { - let mut set = MutSet::default(); - set.insert(value); - self.notifies.insert(key, set); - } - } - } - - fn solved_all(&self) -> bool { - debug_assert_eq!(self.notifies.is_empty(), self.waiting_for.is_empty()); - - for status in self.status.values() { - match status { - Status::Done => { - continue; - } - _ => { - return false; - } - } - } - - true - } -} - /// Struct storing various intermediate stages by their ModuleId #[derive(Debug, Default)] struct ModuleCache<'a> { @@ -351,42 +107,22 @@ fn start_phase<'a>( ) -> Vec> { // we blindly assume all dependencies are met - match state - .dependencies - .status - .get_mut(&Job::Step(module_id, phase)) - { - Some(current @ Status::NotStarted) => { - // start this phase! - *current = Status::Pending; + use crate::work::PrepareStartPhase::*; + match state.dependencies.prepare_start_phase(module_id, phase) { + Continue => { + // fall through } - Some(Status::Pending) => { - // don't start this task again! + Done => { + // no more work to do return vec![]; } - Some(Status::Done) => { - // don't start this task again, but tell those waiting for it they can continue - return state - .dependencies - .notify(module_id, phase) + Recurse(new) => { + return new .into_iter() .map(|(module_id, phase)| start_phase(module_id, phase, arena, state)) .flatten() - .collect(); + .collect() } - None => match phase { - Phase::LoadHeader => { - // this is fine, mark header loading as pending - state - .dependencies - .status - .insert(Job::Step(module_id, Phase::LoadHeader), Status::Pending); - } - _ => unreachable!( - "Pair {:?} is not in dependencies.status, that should never happen!", - (module_id, phase) - ), - }, } let task = { @@ -862,6 +598,43 @@ struct State<'a> { pub layout_caches: std::vec::Vec>, } +impl<'a> State<'a> { + fn new( + root_id: ModuleId, + target_info: TargetInfo, + goal_phase: Phase, + stdlib: &'a StdLib, + exposed_types: SubsByModule, + arc_modules: Arc>>, + ident_ids_by_module: Arc>>, + ) -> Self { + let arc_shorthands = Arc::new(Mutex::new(MutMap::default())); + + Self { + root_id, + target_info, + platform_data: None, + goal_phase, + stdlib, + output_path: None, + platform_path: PlatformPath::NotSpecified, + module_cache: ModuleCache::default(), + dependencies: Dependencies::default(), + procedures: MutMap::default(), + exposed_to_host: ExposedToHost::default(), + exposed_types, + arc_modules, + arc_shorthands, + constrained_ident_ids: IdentIds::exposed_builtins(0), + ident_ids_by_module, + declarations_by_id: MutMap::default(), + exposed_symbols_by_module: MutMap::default(), + timings: MutMap::default(), + layout_caches: std::vec::Vec::with_capacity(num_cpus::get()), + } + } +} + #[derive(Debug)] pub struct ModuleTiming { pub read_roc_file: Duration, @@ -914,7 +687,7 @@ impl ModuleTiming { end_time, } = self; - let calculate = |t: Result| -> Option { + let calculate = |t: Result| -> Option { t.ok()? .checked_sub(*make_specializations)? .checked_sub(*find_specializations)? @@ -1030,18 +803,14 @@ fn enqueue_task<'a>( Ok(()) } -pub fn load_and_typecheck<'a, F>( +pub fn load_and_typecheck<'a>( arena: &'a Bump, filename: PathBuf, stdlib: &'a StdLib, src_dir: &Path, exposed_types: SubsByModule, target_info: TargetInfo, - look_up_builtin: F, -) -> Result> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result> { use LoadResult::*; let load_start = LoadStart::from_path(arena, filename)?; @@ -1054,7 +823,6 @@ where exposed_types, Phase::SolveTypes, target_info, - look_up_builtin, )? { Monomorphized(_) => unreachable!(""), TypeChecked(module) => Ok(module), @@ -1062,18 +830,14 @@ where } /// Main entry point to the compiler from the CLI and tests -pub fn load_and_monomorphize<'a, F>( +pub fn load_and_monomorphize<'a>( arena: &'a Bump, filename: PathBuf, stdlib: &'a StdLib, src_dir: &Path, exposed_types: SubsByModule, target_info: TargetInfo, - look_up_builtin: F, -) -> Result, LoadingProblem<'a>> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result, LoadingProblem<'a>> { use LoadResult::*; let load_start = LoadStart::from_path(arena, filename)?; @@ -1086,7 +850,6 @@ where exposed_types, Phase::MakeSpecializations, target_info, - look_up_builtin, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -1094,7 +857,7 @@ where } #[allow(clippy::too_many_arguments)] -pub fn load_and_monomorphize_from_str<'a, F>( +pub fn load_and_monomorphize_from_str<'a>( arena: &'a Bump, filename: PathBuf, src: &'a str, @@ -1102,11 +865,7 @@ pub fn load_and_monomorphize_from_str<'a, F>( src_dir: &Path, exposed_types: SubsByModule, target_info: TargetInfo, - look_up_builtin: F, -) -> Result, LoadingProblem<'a>> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result, LoadingProblem<'a>> { use LoadResult::*; let load_start = LoadStart::from_str(arena, filename, src)?; @@ -1119,7 +878,6 @@ where exposed_types, Phase::MakeSpecializations, target_info, - look_up_builtin, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -1266,20 +1024,51 @@ enum LoadResult<'a> { /// specializations, so if none of their specializations changed, we don't even need /// to rebuild the module and can link in the cached one directly.) #[allow(clippy::too_many_arguments)] -fn load<'a, F>( +fn load<'a>( arena: &'a Bump, - //filename: PathBuf, load_start: LoadStart<'a>, stdlib: &'a StdLib, src_dir: &Path, exposed_types: SubsByModule, goal_phase: Phase, target_info: TargetInfo, - look_up_builtins: F, -) -> Result, LoadingProblem<'a>> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result, LoadingProblem<'a>> { + // When compiling to wasm, we cannot spawn extra threads + // so we have a single-threaded implementation + if cfg!(target_family = "wasm") { + load_single_threaded( + arena, + load_start, + stdlib, + src_dir, + exposed_types, + goal_phase, + target_info, + ) + } else { + load_multi_threaded( + arena, + load_start, + stdlib, + src_dir, + exposed_types, + goal_phase, + target_info, + ) + } +} + +/// Load using only a single thread; used when compiling to webassembly +#[allow(clippy::too_many_arguments)] +fn load_single_threaded<'a>( + arena: &'a Bump, + load_start: LoadStart<'a>, + stdlib: &'a StdLib, + src_dir: &Path, + exposed_types: SubsByModule, + goal_phase: Phase, + target_info: TargetInfo, +) -> Result, LoadingProblem<'a>> { let LoadStart { arc_modules, ident_ids_by_module, @@ -1287,7 +1076,191 @@ where root_msg, } = load_start; - let arc_shorthands = Arc::new(Mutex::new(MutMap::default())); + let (msg_tx, msg_rx) = bounded(1024); + + msg_tx + .send(root_msg) + .map_err(|_| LoadingProblem::MsgChannelDied)?; + + let mut state = State::new( + root_id, + target_info, + goal_phase, + stdlib, + exposed_types, + arc_modules, + ident_ids_by_module, + ); + + // We'll add tasks to this, and then worker threads will take tasks from it. + let injector = Injector::new(); + + let (worker_msg_tx, worker_msg_rx) = bounded(1024); + let worker_listener = worker_msg_tx; + let worker_listeners = arena.alloc([worker_listener]); + + let worker = Worker::new_fifo(); + let stealer = worker.stealer(); + let stealers = &[stealer]; + + // now we just manually interleave stepping the state "thread" and the worker "thread" + loop { + match state_thread_step(arena, state, worker_listeners, &injector, &msg_tx, &msg_rx) { + Ok(ControlFlow::Break(done)) => return Ok(done), + Ok(ControlFlow::Continue(new_state)) => { + state = new_state; + } + Err(e) => return Err(e), + } + + // then check if the worker can step + let control_flow = worker_task_step( + arena, + &worker, + &injector, + stealers, + &worker_msg_rx, + &msg_tx, + src_dir, + target_info, + ); + + match control_flow { + Ok(ControlFlow::Break(())) => panic!("the worker should not break!"), + Ok(ControlFlow::Continue(())) => { + // progress was made + } + Err(e) => return Err(e), + } + } +} + +fn state_thread_step<'a>( + arena: &'a Bump, + state: State<'a>, + worker_listeners: &'a [Sender], + injector: &Injector>, + msg_tx: &crossbeam::channel::Sender>, + msg_rx: &crossbeam::channel::Receiver>, +) -> Result, State<'a>>, LoadingProblem<'a>> { + match msg_rx.try_recv() { + Ok(msg) => { + match msg { + Msg::FinishedAllTypeChecking { + solved_subs, + exposed_vars_by_symbol, + exposed_aliases_by_symbol, + exposed_values, + dep_idents, + documentation, + } => { + // We're done! There should be no more messages pending. + debug_assert!(msg_rx.is_empty()); + + let typechecked = finish( + state, + solved_subs, + exposed_values, + exposed_aliases_by_symbol, + exposed_vars_by_symbol, + dep_idents, + documentation, + ); + + Ok(ControlFlow::Break(LoadResult::TypeChecked(typechecked))) + } + Msg::FinishedAllSpecialization { + subs, + exposed_to_host, + } => { + // We're done! There should be no more messages pending. + debug_assert!(msg_rx.is_empty()); + + let monomorphized = finish_specialization(state, subs, exposed_to_host)?; + + Ok(ControlFlow::Break(LoadResult::Monomorphized(monomorphized))) + } + Msg::FailedToReadFile { filename, error } => { + let buf = to_file_problem_report(&filename, error); + Err(LoadingProblem::FormattedReport(buf)) + } + + Msg::FailedToParse(problem) => { + let module_ids = (*state.arc_modules).lock().clone().into_module_ids(); + let buf = + to_parse_problem_report(problem, module_ids, state.constrained_ident_ids); + Err(LoadingProblem::FormattedReport(buf)) + } + msg => { + // This is where most of the main thread's work gets done. + // Everything up to this point has been setting up the threading + // system which lets this logic work efficiently. + let constrained_ident_ids = state.constrained_ident_ids.clone(); + let arc_modules = state.arc_modules.clone(); + + let res_state = update( + state, + msg, + msg_tx.clone(), + injector, + worker_listeners, + arena, + ); + + match res_state { + Ok(new_state) => Ok(ControlFlow::Continue(new_state)), + Err(LoadingProblem::ParsingFailed(problem)) => { + let module_ids = Arc::try_unwrap(arc_modules) + .unwrap_or_else(|_| { + panic!( + r"There were still outstanding Arc references to module_ids" + ) + }) + .into_inner() + .into_module_ids(); + + let buf = + to_parse_problem_report(problem, module_ids, constrained_ident_ids); + Err(LoadingProblem::FormattedReport(buf)) + } + Err(e) => Err(e), + } + } + } + } + Err(err) => match err { + crossbeam::channel::TryRecvError::Empty => Ok(ControlFlow::Continue(state)), + crossbeam::channel::TryRecvError::Disconnected => Err(LoadingProblem::MsgChannelDied), + }, + } +} + +#[allow(clippy::too_many_arguments)] +fn load_multi_threaded<'a>( + arena: &'a Bump, + load_start: LoadStart<'a>, + stdlib: &'a StdLib, + src_dir: &Path, + exposed_types: SubsByModule, + goal_phase: Phase, + target_info: TargetInfo, +) -> Result, LoadingProblem<'a>> { + let LoadStart { + arc_modules, + ident_ids_by_module, + root_id, + root_msg, + } = load_start; + + let mut state = State::new( + root_id, + target_info, + goal_phase, + stdlib, + exposed_types, + arc_modules, + ident_ids_by_module, + ); let (msg_tx, msg_rx) = bounded(1024); msg_tx @@ -1309,14 +1282,9 @@ where Err(_) => default_num_workers, }; - let worker_arenas = arena.alloc(bumpalo::collections::Vec::with_capacity_in( - num_workers, - arena, - )); - - for _ in 0..num_workers { - worker_arenas.push(Bump::new()); - } + // 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)); // We'll add tasks to this, and then worker threads will take tasks from it. let injector = Injector::new(); @@ -1328,29 +1296,28 @@ where let mut worker_queues = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); let mut stealers = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); - let it = worker_arenas.iter_mut(); + for _ in 0..num_workers { + let worker = Worker::new_fifo(); + stealers.push(worker.stealer()); + worker_queues.push(worker); + } + + // 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(); { thread::scope(|thread_scope| { - for _ in 0..num_workers { - let worker = Worker::new_lifo(); - - stealers.push(worker.stealer()); - worker_queues.push(worker); - } - - // 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 mut worker_listeners = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); for worker_arena in it { let msg_tx = msg_tx.clone(); let worker = worker_queues.pop().unwrap(); - let (worker_msg_tx, worker_msg_rx) = bounded(1024); + let (worker_msg_tx, worker_msg_rx) = bounded(1024); worker_listeners.push(worker_msg_tx); // We only want to move a *reference* to the main task queue's @@ -1363,95 +1330,22 @@ where .builder() .stack_size(EXPANDED_STACK_SIZE) .spawn(move |_| { - // Keep listening until we receive a Shutdown msg - - for msg in worker_msg_rx.iter() { - match msg { - WorkerMsg::Shutdown => { - // We've finished all our work. It's time to - // shut down the thread, so when the main thread - // blocks on joining with all the worker threads, - // it can finally exit too! - return Ok(()); - } - WorkerMsg::TaskAdded => { - // Find a task - either from this thread's queue, - // or from the main queue, or from another worker's - // queue - and run it. - // - // There might be no tasks to work on! That could - // happen if another thread is working on a task - // which will later result in more tasks being - // added. In that case, do nothing, and keep waiting - // until we receive a Shutdown message. - if let Some(task) = find_task(&worker, injector, stealers) { - let result = run_task( - task, - worker_arena, - src_dir, - msg_tx.clone(), - target_info, - look_up_builtins, - ); - - match result { - Ok(()) => {} - Err(LoadingProblem::MsgChannelDied) => { - panic!("Msg channel closed unexpectedly.") - } - Err(LoadingProblem::ParsingFailed(problem)) => { - msg_tx.send(Msg::FailedToParse(problem)).unwrap(); - } - Err(LoadingProblem::FileProblem { - filename, - error, - }) => { - msg_tx - .send(Msg::FailedToReadFile { filename, error }) - .unwrap(); - } - Err(other) => { - return Err(other); - } - } - } - } - } - } - - // Needed to prevent a borrow checker error about this closure - // outliving its enclosing function. - drop(worker_msg_rx); - - Ok(()) + // will process messages until we run out + worker_task( + worker_arena, + worker, + injector, + stealers, + worker_msg_rx, + msg_tx, + src_dir, + target_info, + ) }); res_join_handle.unwrap(); } - let mut state = State { - root_id, - target_info, - platform_data: None, - goal_phase, - stdlib, - output_path: None, - platform_path: PlatformPath::NotSpecified, - module_cache: ModuleCache::default(), - dependencies: Dependencies::default(), - procedures: MutMap::default(), - exposed_to_host: ExposedToHost::default(), - exposed_types, - arc_modules, - arc_shorthands, - constrained_ident_ids: IdentIds::exposed_builtins(0), - ident_ids_by_module, - declarations_by_id: MutMap::default(), - exposed_symbols_by_module: MutMap::default(), - timings: MutMap::default(), - layout_caches: std::vec::Vec::with_capacity(num_cpus::get()), - }; - // We've now distributed one worker queue to each thread. // There should be no queues left to distribute! debug_assert!(worker_queues.is_empty()); @@ -1474,117 +1368,152 @@ where // The root module will have already queued up messages to process, // and processing those messages will in turn queue up more messages. - for msg in msg_rx.iter() { - match msg { - Msg::FinishedAllTypeChecking { - solved_subs, - exposed_vars_by_symbol, - exposed_aliases_by_symbol, - exposed_values, - dep_idents, - documentation, - } => { - // We're done! There should be no more messages pending. - debug_assert!(msg_rx.is_empty()); - - // Shut down all the worker threads. - for listener in worker_listeners { - listener - .send(WorkerMsg::Shutdown) - .map_err(|_| LoadingProblem::MsgChannelDied)?; - } - - return Ok(LoadResult::TypeChecked(finish( - state, - solved_subs, - exposed_values, - exposed_aliases_by_symbol, - exposed_vars_by_symbol, - dep_idents, - documentation, - ))); - } - Msg::FinishedAllSpecialization { - subs, - exposed_to_host, - } => { - // We're done! There should be no more messages pending. - debug_assert!(msg_rx.is_empty()); - + loop { + match state_thread_step(arena, state, worker_listeners, &injector, &msg_tx, &msg_rx) + { + Ok(ControlFlow::Break(load_result)) => { shut_down_worker_threads!(); - return Ok(LoadResult::Monomorphized(finish_specialization( - state, - subs, - exposed_to_host, - )?)); + return Ok(load_result); } - Msg::FailedToReadFile { filename, error } => { + Ok(ControlFlow::Continue(new_state)) => { + state = new_state; + continue; + } + Err(e) => { shut_down_worker_threads!(); - let buf = to_file_problem_report(&filename, error); - return Err(LoadingProblem::FormattedReport(buf)); + return Err(e); } + } + } + }) + } + .unwrap() +} - Msg::FailedToParse(problem) => { - shut_down_worker_threads!(); +#[allow(clippy::too_many_arguments)] +fn worker_task_step<'a>( + worker_arena: &'a Bump, + worker: &Worker>, + injector: &Injector>, + stealers: &[Stealer>], + worker_msg_rx: &crossbeam::channel::Receiver, + msg_tx: &MsgSender<'a>, + src_dir: &Path, + target_info: TargetInfo, +) -> Result, LoadingProblem<'a>> { + match worker_msg_rx.try_recv() { + Ok(msg) => { + match msg { + WorkerMsg::Shutdown => { + // We've finished all our work. It's time to + // shut down the thread, so when the main thread + // blocks on joining with all the worker threads, + // it can finally exit too! + Ok(ControlFlow::Break(())) + } + WorkerMsg::TaskAdded => { + // Find a task - either from this thread's queue, + // or from the main queue, or from another worker's + // queue - and run it. + // + // There might be no tasks to work on! That could + // happen if another thread is working on a task + // which will later result in more tasks being + // added. In that case, do nothing, and keep waiting + // until we receive a Shutdown message. + if let Some(task) = find_task(worker, injector, stealers) { + let result = + run_task(task, worker_arena, src_dir, msg_tx.clone(), target_info); - let module_ids = (*state.arc_modules).lock().clone().into_module_ids(); - let buf = to_parse_problem_report( - problem, - module_ids, - state.constrained_ident_ids, - ); - return Err(LoadingProblem::FormattedReport(buf)); - } - msg => { - // This is where most of the main thread's work gets done. - // Everything up to this point has been setting up the threading - // system which lets this logic work efficiently. - let constrained_ident_ids = state.constrained_ident_ids.clone(); - let arc_modules = state.arc_modules.clone(); - - let res_state = update( - state, - msg, - msg_tx.clone(), - &injector, - worker_listeners, - arena, - ); - - match res_state { - Ok(new_state) => { - state = new_state; + match result { + Ok(()) => {} + Err(LoadingProblem::MsgChannelDied) => { + panic!("Msg channel closed unexpectedly.") } Err(LoadingProblem::ParsingFailed(problem)) => { - shut_down_worker_threads!(); - - let module_ids = Arc::try_unwrap(arc_modules) - .unwrap_or_else(|_| { - panic!(r"There were still outstanding Arc references to module_ids") - }) - .into_inner() - .into_module_ids(); - - let buf = to_parse_problem_report( - problem, - module_ids, - constrained_ident_ids, - ); - return Err(LoadingProblem::FormattedReport(buf)); + msg_tx.send(Msg::FailedToParse(problem)).unwrap(); } - Err(e) => return Err(e), + Err(LoadingProblem::FileProblem { filename, error }) => { + msg_tx + .send(Msg::FailedToReadFile { filename, error }) + .unwrap(); + } + Err(other) => { + return Err(other); + } + } + } + + Ok(ControlFlow::Continue(())) + } + } + } + Err(err) => match err { + crossbeam::channel::TryRecvError::Empty => Ok(ControlFlow::Continue(())), + crossbeam::channel::TryRecvError::Disconnected => Ok(ControlFlow::Break(())), + }, + } +} + +#[allow(clippy::too_many_arguments)] +fn worker_task<'a>( + worker_arena: &'a Bump, + worker: Worker>, + injector: &Injector>, + stealers: &[Stealer>], + worker_msg_rx: crossbeam::channel::Receiver, + msg_tx: MsgSender<'a>, + src_dir: &Path, + target_info: TargetInfo, +) -> Result<(), LoadingProblem<'a>> { + // Keep listening until we receive a Shutdown msg + for msg in worker_msg_rx.iter() { + match msg { + WorkerMsg::Shutdown => { + // We've finished all our work. It's time to + // shut down the thread, so when the main thread + // blocks on joining with all the worker threads, + // it can finally exit too! + return Ok(()); + } + WorkerMsg::TaskAdded => { + // Find a task - either from this thread's queue, + // or from the main queue, or from another worker's + // queue - and run it. + // + // There might be no tasks to work on! That could + // happen if another thread is working on a task + // which will later result in more tasks being + // added. In that case, do nothing, and keep waiting + // until we receive a Shutdown message. + if let Some(task) = find_task(&worker, injector, stealers) { + let result = run_task(task, worker_arena, src_dir, msg_tx.clone(), target_info); + + match result { + Ok(()) => {} + Err(LoadingProblem::MsgChannelDied) => { + panic!("Msg channel closed unexpectedly.") + } + Err(LoadingProblem::ParsingFailed(problem)) => { + msg_tx.send(Msg::FailedToParse(problem)).unwrap(); + } + Err(LoadingProblem::FileProblem { filename, error }) => { + msg_tx + .send(Msg::FailedToReadFile { filename, error }) + .unwrap(); + } + Err(other) => { + return Err(other); } } } } - - // The msg_rx receiver closed unexpectedly before we finished solving everything - Err(LoadingProblem::MsgChannelDied) - }) + } } - .unwrap() + + Ok(()) } fn start_tasks<'a>( @@ -3257,18 +3186,14 @@ fn fabricate_pkg_config_module<'a>( #[allow(clippy::too_many_arguments)] #[allow(clippy::unnecessary_wraps)] -fn canonicalize_and_constrain<'a, F>( +fn canonicalize_and_constrain<'a>( arena: &'a Bump, module_ids: &ModuleIds, dep_idents: MutMap, exposed_symbols: MutSet, aliases: MutMap, parsed: ParsedModule<'a>, - look_up_builtins: F, -) -> Result, LoadingProblem<'a>> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result, LoadingProblem<'a>> { let canonicalize_start = SystemTime::now(); let ParsedModule { @@ -3296,7 +3221,6 @@ where exposed_imports, &exposed_symbols, &mut var_store, - look_up_builtins, ); let canonicalize_end = SystemTime::now(); @@ -3777,17 +3701,13 @@ fn add_def_to_module<'a>( } } -fn run_task<'a, F>( +fn run_task<'a>( task: BuildTask<'a>, arena: &'a Bump, src_dir: &Path, msg_tx: MsgSender<'a>, target_info: TargetInfo, - look_up_builtins: F, -) -> Result<(), LoadingProblem<'a>> -where - F: Fn(Symbol, &mut VarStore) -> Option + 'static + Send + Copy, -{ +) -> Result<(), LoadingProblem<'a>> { use BuildTask::*; let msg = match task { @@ -3819,7 +3739,6 @@ where exposed_symbols, aliases, parsed, - look_up_builtins, ), Solve { module, diff --git a/compiler/load/src/lib.rs b/compiler/load/src/lib.rs index 90f9140930..474a9bc679 100644 --- a/compiler/load/src/lib.rs +++ b/compiler/load/src/lib.rs @@ -3,6 +3,7 @@ #![allow(clippy::large_enum_variant)] pub mod docs; pub mod file; +mod work; #[cfg(target_family = "wasm")] mod wasm_system_time; diff --git a/compiler/load/src/wasm_system_time.rs b/compiler/load/src/wasm_system_time.rs index 9709c1e22d..4fa9728871 100644 --- a/compiler/load/src/wasm_system_time.rs +++ b/compiler/load/src/wasm_system_time.rs @@ -11,13 +11,13 @@ Instead we use these dummy implementations, which should just disappear at compi pub struct SystemTime; impl SystemTime { - fn now() -> Self { + pub fn now() -> Self { SystemTime } - fn duration_since(&self, _: SystemTime) -> Result { + pub fn duration_since(&self, _: SystemTime) -> Result { Ok(Duration) } - fn elapsed(&self) -> Result { + pub fn elapsed(&self) -> Result { Ok(Duration) } } @@ -26,7 +26,7 @@ impl SystemTime { pub struct Duration; impl Duration { - fn checked_sub(&self, _: Duration) -> Option { + pub fn checked_sub(&self, _: Duration) -> Option { Some(Duration) } } diff --git a/compiler/load/src/work.rs b/compiler/load/src/work.rs new file mode 100644 index 0000000000..f26209163d --- /dev/null +++ b/compiler/load/src/work.rs @@ -0,0 +1,290 @@ +use roc_collections::all::{MutMap, MutSet}; +use roc_module::symbol::{ModuleId, PackageQualified}; + +use std::collections::hash_map::Entry; + +/// NOTE the order of definition of the phases is used by the ord instance +/// make sure they are ordered from first to last! +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug)] +pub enum Phase { + LoadHeader, + Parse, + CanonicalizeAndConstrain, + SolveTypes, + FindSpecializations, + MakeSpecializations, +} + +/// NOTE keep up to date manually, from ParseAndGenerateConstraints to the highest phase we support +const PHASES: [Phase; 6] = [ + Phase::LoadHeader, + Phase::Parse, + Phase::CanonicalizeAndConstrain, + Phase::SolveTypes, + Phase::FindSpecializations, + Phase::MakeSpecializations, +]; + +#[derive(Debug)] +enum Status { + NotStarted, + Pending, + Done, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +enum Job<'a> { + Step(ModuleId, Phase), + ResolveShorthand(&'a str), +} + +#[derive(Default, Debug)] +pub struct Dependencies<'a> { + waiting_for: MutMap, MutSet>>, + notifies: MutMap, MutSet>>, + status: MutMap, Status>, +} + +impl<'a> Dependencies<'a> { + /// Add all the dependencies for a module, return (module, phase) pairs that can make progress + pub fn add_module( + &mut self, + module_id: ModuleId, + dependencies: &MutSet>, + goal_phase: Phase, + ) -> MutSet<(ModuleId, Phase)> { + use Phase::*; + + let mut output = MutSet::default(); + + for dep in dependencies.iter() { + let has_package_dependency = self.add_package_dependency(dep, Phase::LoadHeader); + + let dep = *dep.as_inner(); + + if !has_package_dependency { + // loading can start immediately on this dependency + output.insert((dep, Phase::LoadHeader)); + } + + // to parse and generate constraints, the headers of all dependencies must be loaded! + // otherwise, we don't know whether an imported symbol is actually exposed + self.add_dependency_help(module_id, dep, Phase::Parse, Phase::LoadHeader); + + // to canonicalize a module, all its dependencies must be canonicalized + self.add_dependency(module_id, dep, Phase::CanonicalizeAndConstrain); + + // to typecheck a module, all its dependencies must be type checked already + self.add_dependency(module_id, dep, Phase::SolveTypes); + + if goal_phase >= FindSpecializations { + self.add_dependency(module_id, dep, Phase::FindSpecializations); + } + + if goal_phase >= MakeSpecializations { + self.add_dependency(dep, module_id, Phase::MakeSpecializations); + } + } + + // add dependencies for self + // phase i + 1 of a file always depends on phase i being completed + { + let mut i = 0; + while PHASES[i] < goal_phase { + self.add_dependency_help(module_id, module_id, PHASES[i + 1], PHASES[i]); + i += 1; + } + } + + self.add_to_status(module_id, goal_phase); + + output + } + + fn add_to_status(&mut self, module_id: ModuleId, goal_phase: Phase) { + for phase in PHASES.iter() { + if *phase > goal_phase { + break; + } + + if let Entry::Vacant(entry) = self.status.entry(Job::Step(module_id, *phase)) { + entry.insert(Status::NotStarted); + } + } + } + + /// Propagate a notification, return (module, phase) pairs that can make progress + pub fn notify(&mut self, module_id: ModuleId, phase: Phase) -> MutSet<(ModuleId, Phase)> { + self.notify_help(Job::Step(module_id, phase)) + } + + /// Propagate a notification, return (module, phase) pairs that can make progress + pub fn notify_package(&mut self, shorthand: &'a str) -> MutSet<(ModuleId, Phase)> { + self.notify_help(Job::ResolveShorthand(shorthand)) + } + + fn notify_help(&mut self, key: Job<'a>) -> MutSet<(ModuleId, Phase)> { + self.status.insert(key.clone(), Status::Done); + + let mut output = MutSet::default(); + + if let Some(to_notify) = self.notifies.get(&key) { + for notify_key in to_notify { + let mut is_empty = false; + if let Some(waiting_for_pairs) = self.waiting_for.get_mut(notify_key) { + waiting_for_pairs.remove(&key); + is_empty = waiting_for_pairs.is_empty(); + } + + if is_empty { + self.waiting_for.remove(notify_key); + + if let Job::Step(module, phase) = *notify_key { + output.insert((module, phase)); + } + } + } + } + + self.notifies.remove(&key); + + output + } + + fn add_package_dependency( + &mut self, + module: &PackageQualified<'a, ModuleId>, + next_phase: Phase, + ) -> bool { + match module { + PackageQualified::Unqualified(_) => { + // no dependency, we can just start loading the file + false + } + PackageQualified::Qualified(shorthand, module_id) => { + let job = Job::ResolveShorthand(shorthand); + let next_step = Job::Step(*module_id, next_phase); + match self.status.get(&job) { + None | Some(Status::NotStarted) | Some(Status::Pending) => { + // this shorthand is not resolved, add a dependency + { + let entry = self + .waiting_for + .entry(next_step.clone()) + .or_insert_with(Default::default); + + entry.insert(job.clone()); + } + + { + let entry = self.notifies.entry(job).or_insert_with(Default::default); + + entry.insert(next_step); + } + + true + } + Some(Status::Done) => { + // shorthand is resolved; no dependency + false + } + } + } + } + } + + /// A waits for B, and B will notify A when it completes the phase + fn add_dependency(&mut self, a: ModuleId, b: ModuleId, phase: Phase) { + self.add_dependency_help(a, b, phase, phase); + } + + /// phase_a of module a is waiting for phase_b of module_b + fn add_dependency_help(&mut self, a: ModuleId, b: ModuleId, phase_a: Phase, phase_b: Phase) { + // no need to wait if the dependency is already done! + if let Some(Status::Done) = self.status.get(&Job::Step(b, phase_b)) { + return; + } + + let key = Job::Step(a, phase_a); + let value = Job::Step(b, phase_b); + match self.waiting_for.get_mut(&key) { + Some(existing) => { + existing.insert(value); + } + None => { + let mut set = MutSet::default(); + set.insert(value); + self.waiting_for.insert(key, set); + } + } + + let key = Job::Step(b, phase_b); + let value = Job::Step(a, phase_a); + match self.notifies.get_mut(&key) { + Some(existing) => { + existing.insert(value); + } + None => { + let mut set = MutSet::default(); + set.insert(value); + self.notifies.insert(key, set); + } + } + } + + pub fn solved_all(&self) -> bool { + debug_assert_eq!(self.notifies.is_empty(), self.waiting_for.is_empty()); + + for status in self.status.values() { + match status { + Status::Done => { + continue; + } + _ => { + return false; + } + } + } + + true + } + + pub fn prepare_start_phase(&mut self, module_id: ModuleId, phase: Phase) -> PrepareStartPhase { + match self.status.get_mut(&Job::Step(module_id, phase)) { + Some(current @ Status::NotStarted) => { + // start this phase! + *current = Status::Pending; + PrepareStartPhase::Continue + } + Some(Status::Pending) => { + // don't start this task again! + PrepareStartPhase::Done + } + Some(Status::Done) => { + // don't start this task again, but tell those waiting for it they can continue + let new = self.notify(module_id, phase); + + PrepareStartPhase::Recurse(new) + } + None => match phase { + Phase::LoadHeader => { + // this is fine, mark header loading as pending + self.status + .insert(Job::Step(module_id, Phase::LoadHeader), Status::Pending); + + PrepareStartPhase::Continue + } + _ => unreachable!( + "Pair {:?} is not in dependencies.status, that should never happen!", + (module_id, phase) + ), + }, + } + } +} + +pub enum PrepareStartPhase { + Continue, + Done, + Recurse(MutSet<(ModuleId, Phase)>), +} diff --git a/compiler/load/tests/test_load.rs b/compiler/load/tests/test_load.rs index 8b4abdd7ca..dc1afddcc0 100644 --- a/compiler/load/tests/test_load.rs +++ b/compiler/load/tests/test_load.rs @@ -16,7 +16,6 @@ mod helpers; mod test_load { use crate::helpers::fixtures_dir; use bumpalo::Bump; - use roc_can::builtins::builtin_defs_map; use roc_can::def::Declaration::*; use roc_can::def::Def; use roc_collections::all::MutMap; @@ -111,7 +110,6 @@ mod test_load { dir.path(), exposed_types, TARGET_INFO, - builtin_defs_map, ) }; @@ -135,7 +133,6 @@ mod test_load { src_dir.as_path(), subs_by_module, TARGET_INFO, - builtin_defs_map, ); let mut loaded_module = match loaded { Ok(x) => x, @@ -301,7 +298,6 @@ mod test_load { src_dir.as_path(), subs_by_module, TARGET_INFO, - builtin_defs_map, ); let mut loaded_module = loaded.expect("Test module failed to load"); diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index e52409c240..f42b554628 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -809,6 +809,7 @@ fn call_spec( add_loop(builder, block, state_type, init_state, loop_body) } + // List.mapWithIndex : List before, (before, Nat -> after) -> List after ListMapWithIndex { xs } => { let list = env.symbols[xs]; @@ -818,7 +819,8 @@ fn call_spec( let element = builder.add_bag_get(block, input_bag)?; let index = builder.add_make_tuple(block, &[])?; - let new_element = call_function!(builder, block, [index, element]); + // before, Nat -> after + let new_element = call_function!(builder, block, [element, index]); list_append(builder, block, update_mode_var, state, new_element) }; diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index 2109204d7a..1036d55279 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -624,8 +624,9 @@ impl<'a> BorrowInfState<'a> { } } ListMapWithIndex { xs } => { - // own the list if the function wants to own the element - if !function_ps[1].borrow { + // List.mapWithIndex : List before, (before, Nat -> after) -> List after + // own the list if the function wants to own the element (before, index 0) + if !function_ps[0].borrow { self.own_var(*xs); } } @@ -943,7 +944,8 @@ pub fn lowlevel_borrow_signature(arena: &Bump, op: LowLevel) -> &[bool] { StrSplit => arena.alloc_slice_copy(&[borrowed, borrowed]), StrToNum => arena.alloc_slice_copy(&[borrowed]), ListSingle => arena.alloc_slice_copy(&[irrelevant]), - ListRepeat => arena.alloc_slice_copy(&[irrelevant, borrowed]), + // List.repeat : elem, Nat -> List.elem + ListRepeat => arena.alloc_slice_copy(&[borrowed, irrelevant]), ListReverse => arena.alloc_slice_copy(&[owned]), ListPrepend => arena.alloc_slice_copy(&[owned, owned]), StrJoinWith => arena.alloc_slice_copy(&[borrowed, borrowed]), diff --git a/compiler/parse/fuzz/.gitignore b/compiler/parse/fuzz/.gitignore index 572e03bdf3..a0925114d6 100644 --- a/compiler/parse/fuzz/.gitignore +++ b/compiler/parse/fuzz/.gitignore @@ -1,4 +1,3 @@ - target corpus artifacts diff --git a/compiler/parse/fuzz/Cargo.toml b/compiler/parse/fuzz/Cargo.toml index 792dae3329..2d1bd145cf 100644 --- a/compiler/parse/fuzz/Cargo.toml +++ b/compiler/parse/fuzz/Cargo.toml @@ -1,4 +1,3 @@ - [package] name = "roc_parse-fuzz" version = "0.0.0" diff --git a/compiler/parse/src/blankspace.rs b/compiler/parse/src/blankspace.rs index 2f91aa648b..1b85af03a0 100644 --- a/compiler/parse/src/blankspace.rs +++ b/compiler/parse/src/blankspace.rs @@ -1,5 +1,6 @@ use crate::ast::CommentOrNewline; use crate::ast::Spaceable; +use crate::parser::SpaceProblem; use crate::parser::{self, and, backtrackable, BadInputError, Parser, Progress::*}; use crate::state::State; use bumpalo::collections::vec::Vec; @@ -10,7 +11,6 @@ use roc_region::all::Position; pub fn space0_around_ee<'a, P, S, E>( parser: P, min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_before_problem: fn(Position) -> E, indent_after_problem: fn(Position) -> E, ) -> impl Parser<'a, Loc, E> @@ -19,15 +19,12 @@ where S: 'a, P: Parser<'a, Loc, E>, P: 'a, - E: 'a, + E: 'a + SpaceProblem, { parser::map_with_arena( and( - space0_e(min_indent, space_problem, indent_before_problem), - and( - parser, - space0_e(min_indent, space_problem, indent_after_problem), - ), + space0_e(min_indent, indent_before_problem), + and(parser, space0_e(min_indent, indent_after_problem)), ), spaces_around_help, ) @@ -36,7 +33,6 @@ where pub fn space0_before_optional_after<'a, P, S, E>( parser: P, min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_before_problem: fn(Position) -> E, indent_after_problem: fn(Position) -> E, ) -> impl Parser<'a, Loc, E> @@ -45,15 +41,15 @@ where S: 'a, P: Parser<'a, Loc, E>, P: 'a, - E: 'a, + E: 'a + SpaceProblem, { parser::map_with_arena( and( - space0_e(min_indent, space_problem, indent_before_problem), + space0_e(min_indent, indent_before_problem), and( parser, one_of![ - backtrackable(space0_e(min_indent, space_problem, indent_after_problem)), + backtrackable(space0_e(min_indent, indent_after_problem)), succeed!(&[] as &[_]), ], ), @@ -101,7 +97,6 @@ where pub fn space0_before_e<'a, P, S, E>( parser: P, min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_problem: fn(Position) -> E, ) -> impl Parser<'a, Loc, E> where @@ -109,10 +104,10 @@ where S: 'a, P: Parser<'a, Loc, E>, P: 'a, - E: 'a, + E: 'a + SpaceProblem, { parser::map_with_arena( - and!(space0_e(min_indent, space_problem, indent_problem), parser), + and!(space0_e(min_indent, indent_problem), parser), |arena: &'a Bump, (space_list, loc_expr): (&'a [CommentOrNewline<'a>], Loc)| { if space_list.is_empty() { loc_expr @@ -128,7 +123,6 @@ where pub fn space0_after_e<'a, P, S, E>( parser: P, min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_problem: fn(Position) -> E, ) -> impl Parser<'a, Loc, E> where @@ -136,10 +130,10 @@ where S: 'a, P: Parser<'a, Loc, E>, P: 'a, - E: 'a, + E: 'a + SpaceProblem, { parser::map_with_arena( - and!(parser, space0_e(min_indent, space_problem, indent_problem)), + and!(parser, space0_e(min_indent, indent_problem)), |arena: &'a Bump, (loc_expr, space_list): (Loc, &'a [CommentOrNewline<'a>])| { if space_list.is_empty() { loc_expr @@ -170,23 +164,21 @@ where pub fn space0_e<'a, E>( min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_problem: fn(Position) -> E, ) -> impl Parser<'a, &'a [CommentOrNewline<'a>], E> where - E: 'a, + E: 'a + SpaceProblem, { - spaces_help_help(min_indent, space_problem, indent_problem) + spaces_help_help(min_indent, indent_problem) } #[inline(always)] fn spaces_help_help<'a, E>( min_indent: u32, - space_problem: fn(BadInputError, Position) -> E, indent_problem: fn(Position) -> E, ) -> impl Parser<'a, &'a [CommentOrNewline<'a>], E> where - E: 'a, + E: 'a + SpaceProblem, { use SpaceState::*; @@ -195,7 +187,7 @@ where match eat_spaces(state.clone(), false, comments_and_newlines) { HasTab(state) => Err(( MadeProgress, - space_problem(BadInputError::HasTab, state.pos()), + E::space_problem(BadInputError::HasTab, state.pos()), state, )), Good { diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index f45cbe5c9a..21c8cc1277 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -39,7 +39,6 @@ pub fn test_parse_expr<'a>( space0_before_e( move |a, s| parse_loc_expr(min_indent, a, s), min_indent, - EExpr::Space, EExpr::IndentStart, ), expr_end() @@ -106,7 +105,6 @@ fn loc_expr_in_parens_help_help<'a>( min_indent, arena, state )), min_indent, - EInParens::Space, EInParens::IndentOpen, EInParens::IndentEnd, ), @@ -340,7 +338,7 @@ fn parse_expr_operator_chain<'a>( let initial = state.clone(); let end = state.pos(); - match space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state) { + match space0_e(min_indent, EExpr::IndentEnd).parse(arena, state) { Err((_, _, state)) => Ok((MadeProgress, expr.value, state)), Ok((_, spaces_before_op, state)) => { let expr_state = ExprState { @@ -781,7 +779,7 @@ fn parse_defs_end<'a>( let min_indent = start_column; let initial = state.clone(); - let state = match space0_e(min_indent, EExpr::Space, EExpr::IndentStart).parse(arena, state) { + let state = match space0_e(min_indent, EExpr::IndentStart).parse(arena, state) { Err((MadeProgress, _, s)) => { return Err((MadeProgress, EExpr::DefMissingFinalExpr(s.pos()), s)); } @@ -798,7 +796,6 @@ fn parse_defs_end<'a>( match space0_after_e( crate::pattern::loc_pattern_help(min_indent), min_indent, - EPattern::Space, EPattern::IndentEnd, ) .parse(arena, state.clone()) @@ -815,7 +812,6 @@ fn parse_defs_end<'a>( let parse_def_expr = space0_before_e( move |a, s| parse_loc_expr(min_indent + 1, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -842,7 +838,6 @@ fn parse_defs_end<'a>( let parse_def_expr = space0_before_e( move |a, s| parse_loc_expr(min_indent + 1, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -864,7 +859,6 @@ fn parse_defs_end<'a>( space0_before_e( type_annotation::located_help(min_indent + 1, false), min_indent + 1, - EType::TSpace, EType::TIndentStart, ), ) @@ -902,7 +896,6 @@ fn parse_defs_expr<'a>( let parse_final_expr = space0_before_e( move |a, s| parse_loc_expr(min_indent, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -936,7 +929,7 @@ fn parse_expr_operator<'a>( state: State<'a>, ) -> ParseResult<'a, Expr<'a>, EExpr<'a>> { let (_, spaces_after_operator, state) = - space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state)?; + space0_e(min_indent, EExpr::IndentEnd).parse(arena, state)?; // a `-` is unary if it is preceded by a space and not followed by a space @@ -961,11 +954,10 @@ fn parse_expr_operator<'a>( expr_state.initial = state.clone(); - let (spaces, state) = - match space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state) { - Err((_, _, state)) => (&[] as &[_], state), - Ok((_, spaces, state)) => (spaces, state), - }; + let (spaces, state) = match space0_e(min_indent, EExpr::IndentEnd).parse(arena, state) { + Err((_, _, state)) => (&[] as &[_], state), + Ok((_, spaces, state)) => (spaces, state), + }; expr_state.arguments.push(arena.alloc(arg)); expr_state.spaces_after = spaces; @@ -1054,7 +1046,6 @@ fn parse_expr_operator<'a>( let parse_cont = space0_before_e( move |a, s| parse_loc_expr(min_indent, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -1094,7 +1085,6 @@ fn parse_expr_operator<'a>( space0_before_e( type_annotation::located_help(indented_more, true), min_indent, - EType::TSpace, EType::TIndentStart, ), ) @@ -1123,7 +1113,6 @@ fn parse_expr_operator<'a>( space0_before_e( type_annotation::located_help(indented_more, false), min_indent, - EType::TSpace, EType::TIndentStart, ), ); @@ -1180,7 +1169,7 @@ fn parse_expr_operator<'a>( .with_spaces_before(spaces_after_operator, new_expr.region); } - match space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state) { + match space0_e(min_indent, EExpr::IndentEnd).parse(arena, state) { Err((_, _, state)) => { let args = std::mem::replace(&mut expr_state.arguments, Vec::new_in(arena)); @@ -1243,7 +1232,7 @@ fn parse_expr_end<'a>( } expr_state.initial = state.clone(); - match space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state) { + match space0_e(min_indent, EExpr::IndentEnd).parse(arena, state) { Err((_, _, state)) => { expr_state.arguments.push(arena.alloc(arg)); expr_state.end = new_end; @@ -1290,7 +1279,6 @@ fn parse_expr_end<'a>( space0_around_ee( crate::pattern::loc_pattern_help(min_indent), min_indent, - EPattern::Space, EPattern::Start, EPattern::IndentEnd, ), @@ -1316,7 +1304,6 @@ fn parse_expr_end<'a>( let parse_body = space0_before_e( move |a, s| parse_loc_expr(min_indent + 1, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -1325,7 +1312,6 @@ fn parse_expr_end<'a>( let parse_cont = space0_before_e( move |a, s| parse_loc_expr(min_indent, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ); @@ -1538,7 +1524,7 @@ pub fn defs<'a>(min_indent: u32) -> impl Parser<'a, Vec<'a, Loc>>, EExpr }; let (_, initial_space, state) = - space0_e(min_indent, EExpr::Space, EExpr::IndentEnd).parse(arena, state)?; + space0_e(min_indent, EExpr::IndentEnd).parse(arena, state)?; let start_column = state.column(); @@ -1550,7 +1536,7 @@ pub fn defs<'a>(min_indent: u32) -> impl Parser<'a, Vec<'a, Loc>>, EExpr let (_, def_state, state) = parse_defs_end(options, start_column, def_state, arena, state)?; let (_, final_space, state) = - space0_e(start_column, EExpr::Space, EExpr::IndentEnd).parse(arena, state)?; + space0_e(start_column, EExpr::IndentEnd).parse(arena, state)?; let mut output = Vec::with_capacity_in(def_state.defs.len(), arena); @@ -1601,7 +1587,6 @@ fn closure_help<'a>( space0_around_ee( specialize(ELambda::Pattern, loc_closure_param(min_indent)), min_indent, - ELambda::Space, ELambda::IndentArg, ELambda::IndentArrow ), @@ -1616,7 +1601,6 @@ fn closure_help<'a>( parse_loc_expr_with_options(min_indent, options, arena, state) }), min_indent, - ELambda::Space, ELambda::IndentBody ) ) @@ -1649,7 +1633,6 @@ mod when { parse_loc_expr_with_options(min_indent, options, arena, state) }), min_indent, - EWhen::Space, EWhen::IndentCondition, EWhen::IndentIs, ), @@ -1797,7 +1780,6 @@ mod when { parse_loc_expr_with_options(min_indent + 1, options, arena, state) }), min_indent, - EWhen::Space, EWhen::IndentIfGuard, EWhen::IndentArrow, ) @@ -1814,13 +1796,11 @@ mod when { ) -> impl Parser<'a, Loc>, EWhen<'a>> { move |arena, state| { let (_, spaces, state) = - backtrackable(space0_e(min_indent, EWhen::Space, EWhen::IndentPattern)) - .parse(arena, state)?; + backtrackable(space0_e(min_indent, EWhen::IndentPattern)).parse(arena, state)?; let (_, loc_pattern, state) = space0_after_e( specialize(EWhen::Pattern, crate::pattern::loc_pattern_help(min_indent)), min_indent, - EWhen::Space, EWhen::IndentPattern, ) .parse(arena, state)?; @@ -1847,7 +1827,7 @@ mod when { let initial = state.clone(); // put no restrictions on the indent after the spaces; we'll check it manually - match space0_e(0, EWhen::Space, EWhen::IndentPattern).parse(arena, state) { + match space0_e(0, EWhen::IndentPattern).parse(arena, state) { Err((MadeProgress, fail, _)) => Err((NoProgress, fail, initial)), Err((NoProgress, fail, _)) => Err((NoProgress, fail, initial)), Ok((_progress, spaces, state)) => { @@ -1914,7 +1894,6 @@ mod when { indent, arena, state )), indent, - EWhen::Space, EWhen::IndentBranch, ) ) @@ -1929,7 +1908,6 @@ fn if_branch<'a>(min_indent: u32) -> impl Parser<'a, (Loc>, Loc(min_indent: u32) -> impl Parser<'a, (Loc>, Loc( parse_loc_expr_with_options(start_column + 1, options, arena, state) }), start_column + 1, - EExpect::Space, EExpect::IndentCondition, ) .parse(arena, state) @@ -1986,7 +1962,6 @@ fn expect_help<'a>( space0_before_e( move |a, s| parse_loc_expr(min_indent, a, s), min_indent, - EExpr::Space, EExpr::IndentEnd, ), ); @@ -2018,7 +1993,7 @@ fn if_expr_help<'a>( // try to parse another `if` // NOTE this drops spaces between the `else` and the `if` let optional_if = and!( - backtrackable(space0_e(min_indent, EIf::Space, EIf::IndentIf)), + backtrackable(space0_e(min_indent, EIf::IndentIf)), parser::keyword_e(keyword::IF, EIf::If) ); @@ -2036,7 +2011,6 @@ fn if_expr_help<'a>( parse_loc_expr_with_options(min_indent, options, arena, state) }), min_indent, - EIf::Space, EIf::IndentElseBranch, ) .parse(arena, state_final_else) @@ -2133,7 +2107,6 @@ fn list_literal_help<'a>(min_indent: u32) -> impl Parser<'a, Expr<'a>, EList<'a> word1(b']', EList::End), min_indent, EList::Open, - EList::Space, EList::IndentEnd, Expr::SpaceBefore ) @@ -2158,8 +2131,7 @@ fn record_field_help<'a>( .parse(arena, state)?; debug_assert_eq!(progress, MadeProgress); - let (_, spaces, state) = - space0_e(min_indent, ERecord::Space, ERecord::IndentColon).parse(arena, state)?; + let (_, spaces, state) = space0_e(min_indent, ERecord::IndentColon).parse(arena, state)?; // Having a value is optional; both `{ email }` and `{ email: blah }` work. // (This is true in both literals and types.) @@ -2173,7 +2145,6 @@ fn record_field_help<'a>( parse_loc_expr_no_multi_backpassing(min_indent, a, s) }), min_indent, - ERecord::Space, ERecord::IndentEnd, ) )) @@ -2236,7 +2207,6 @@ fn record_help<'a>( // (and not e.g. an `Expr::Access`) and extract its string. loc!(record_updateable_identifier()), min_indent, - ERecord::Space, ERecord::IndentEnd, ERecord::IndentAmpersand, ), @@ -2254,12 +2224,11 @@ fn record_help<'a>( space0_around_ee( loc!(record_field_help(min_indent)), min_indent, - ERecord::Space, ERecord::IndentEnd, ERecord::IndentEnd ), ), - space0_e(min_indent, ERecord::Space, ERecord::IndentEnd) + space0_e(min_indent, ERecord::IndentEnd) ), word1(b'}', ERecord::End) ) diff --git a/compiler/parse/src/header.rs b/compiler/parse/src/header.rs index 6688b845fe..1a1b301f9f 100644 --- a/compiler/parse/src/header.rs +++ b/compiler/parse/src/header.rs @@ -255,11 +255,7 @@ pub fn package_entry<'a>() -> impl Parser<'a, Spaced<'a, PackageEntry<'a>>, EPac specialize(|_, pos| EPackageEntry::Shorthand(pos), lowercase_ident()), word1(b':', EPackageEntry::Colon) ), - space0_e( - min_indent, - EPackageEntry::Space, - EPackageEntry::IndentPackage - ) + space0_e(min_indent, EPackageEntry::IndentPackage) )) .parse(arena, state)?; diff --git a/compiler/parse/src/module.rs b/compiler/parse/src/module.rs index e9db8397ce..b1e43773f5 100644 --- a/compiler/parse/src/module.rs +++ b/compiler/parse/src/module.rs @@ -9,7 +9,7 @@ use crate::parser::Progress::{self, *}; use crate::parser::{ backtrackable, optional, specialize, specialize_region, word1, EExposes, EGenerates, EGeneratesWith, EHeader, EImports, EPackages, EProvides, ERequires, ETypedIdent, Parser, - SourceError, SyntaxError, + SourceError, SpaceProblem, SyntaxError, }; use crate::state::State; use crate::string_literal; @@ -57,7 +57,7 @@ fn header<'a>() -> impl Parser<'a, Module<'a>, EHeader<'a>> { map!( and!( - space0_e(0, EHeader::Space, EHeader::IndentStart), + space0_e(0, EHeader::IndentStart), one_of![ map!( skip_first!(keyword_e("interface", EHeader::Start), interface_header()), @@ -107,7 +107,7 @@ fn interface_header<'a>() -> impl Parser<'a, InterfaceHeader<'a>, EHeader<'a>> { let min_indent = 1; let (_, after_interface_keyword, state) = - space0_e(min_indent, EHeader::Space, EHeader::IndentStart).parse(arena, state)?; + space0_e(min_indent, EHeader::IndentStart).parse(arena, state)?; let (_, name, state) = loc!(module_name_help(EHeader::ModuleName)).parse(arena, state)?; let (_, ((before_exposes, after_exposes), exposes), state) = @@ -137,7 +137,7 @@ fn hosted_header<'a>() -> impl Parser<'a, HostedHeader<'a>, EHeader<'a>> { let min_indent = 1; let (_, after_hosted_keyword, state) = - space0_e(min_indent, EHeader::Space, EHeader::IndentStart).parse(arena, state)?; + space0_e(min_indent, EHeader::IndentStart).parse(arena, state)?; let (_, name, state) = loc!(module_name_help(EHeader::ModuleName)).parse(arena, state)?; let (_, ((before_exposes, after_exposes), exposes), state) = @@ -236,7 +236,7 @@ fn app_header<'a>() -> impl Parser<'a, AppHeader<'a>, EHeader<'a>> { let min_indent = 1; let (_, after_app_keyword, state) = - space0_e(min_indent, EHeader::Space, EHeader::IndentStart).parse(arena, state)?; + space0_e(min_indent, EHeader::IndentStart).parse(arena, state)?; let (_, name, state) = loc!(crate::parser::specialize( EHeader::AppName, string_literal::parse() @@ -303,7 +303,7 @@ fn platform_header<'a>() -> impl Parser<'a, PlatformHeader<'a>, EHeader<'a>> { let min_indent = 1; let (_, after_platform_keyword, state) = - space0_e(min_indent, EHeader::Space, EHeader::IndentStart).parse(arena, state)?; + space0_e(min_indent, EHeader::IndentStart).parse(arena, state)?; let (_, name, state) = loc!(specialize(EHeader::PlatformName, package_name())).parse(arena, state)?; @@ -380,7 +380,6 @@ fn provides_to<'a>() -> impl Parser<'a, ProvidesTo<'a>, EProvides<'a>> { min_indent, "to", EProvides::To, - EProvides::Space, EProvides::IndentTo, EProvides::IndentListStart ), @@ -422,7 +421,6 @@ fn provides_without_to<'a>() -> impl Parser< min_indent, "provides", EProvides::Provides, - EProvides::Space, EProvides::IndentProvides, EProvides::IndentListStart ), @@ -434,7 +432,6 @@ fn provides_without_to<'a>() -> impl Parser< word1(b']', EProvides::ListEnd), min_indent, EProvides::Open, - EProvides::Space, EProvides::IndentListEnd, Spaced::SpaceBefore ), @@ -468,7 +465,6 @@ fn provides_types<'a>( word1(b'}', EProvides::ListEnd), min_indent, EProvides::Open, - EProvides::Space, EProvides::IndentListEnd, Spaced::SpaceBefore ) @@ -518,7 +514,6 @@ fn requires<'a>() -> impl Parser< min_indent, "requires", ERequires::Requires, - ERequires::Space, ERequires::IndentRequires, ERequires::IndentListStart ), @@ -530,10 +525,7 @@ fn requires<'a>() -> impl Parser< fn platform_requires<'a>() -> impl Parser<'a, PlatformRequires<'a>, ERequires<'a>> { map!( and!( - skip_second!( - requires_rigids(0), - space0_e(0, ERequires::Space, ERequires::ListStart) - ), + skip_second!(requires_rigids(0), space0_e(0, ERequires::ListStart)), requires_typed_ident() ), |(rigids, signature)| { PlatformRequires { rigids, signature } } @@ -554,7 +546,6 @@ fn requires_rigids<'a>( word1(b'}', ERequires::ListEnd), min_indent, ERequires::Open, - ERequires::Space, ERequires::IndentListEnd, Spaced::SpaceBefore ) @@ -568,7 +559,6 @@ fn requires_typed_ident<'a>() -> impl Parser<'a, Loc>> space0_around_ee( specialize(ERequires::TypedIdent, loc!(typed_ident()),), 0, - ERequires::Space, ERequires::ListStart, ERequires::ListEnd ), @@ -593,7 +583,6 @@ fn exposes_values<'a>() -> impl Parser< min_indent, "exposes", EExposes::Exposes, - EExposes::Space, EExposes::IndentExposes, EExposes::IndentListStart ), @@ -604,7 +593,6 @@ fn exposes_values<'a>() -> impl Parser< word1(b']', EExposes::ListEnd), min_indent, EExposes::Open, - EExposes::Space, EExposes::IndentListEnd, Spaced::SpaceBefore ) @@ -615,19 +603,18 @@ fn spaces_around_keyword<'a, E>( min_indent: u32, keyword: &'static str, expectation: fn(Position) -> E, - space_problem: fn(crate::parser::BadInputError, Position) -> E, indent_problem1: fn(Position) -> E, indent_problem2: fn(Position) -> E, ) -> impl Parser<'a, (&'a [CommentOrNewline<'a>], &'a [CommentOrNewline<'a>]), E> where - E: 'a, + E: 'a + SpaceProblem, { and!( skip_second!( - backtrackable(space0_e(min_indent, space_problem, indent_problem1)), + backtrackable(space0_e(min_indent, indent_problem1)), crate::parser::keyword_e(keyword, expectation) ), - space0_e(min_indent, space_problem, indent_problem2) + space0_e(min_indent, indent_problem2) ) } @@ -647,7 +634,6 @@ fn exposes_modules<'a>() -> impl Parser< min_indent, "exposes", EExposes::Exposes, - EExposes::Space, EExposes::IndentExposes, EExposes::IndentListStart ), @@ -658,7 +644,6 @@ fn exposes_modules<'a>() -> impl Parser< word1(b']', EExposes::ListEnd), min_indent, EExposes::Open, - EExposes::Space, EExposes::IndentListEnd, Spaced::SpaceBefore ) @@ -696,7 +681,6 @@ fn packages<'a>() -> impl Parser<'a, Packages<'a>, EPackages<'a>> { min_indent, "packages", EPackages::Packages, - EPackages::Space, EPackages::IndentPackages, EPackages::IndentListStart ), @@ -707,7 +691,6 @@ fn packages<'a>() -> impl Parser<'a, Packages<'a>, EPackages<'a>> { word1(b'}', EPackages::ListEnd), min_indent, EPackages::Open, - EPackages::Space, EPackages::IndentListEnd, Spaced::SpaceBefore ) @@ -741,7 +724,6 @@ fn generates<'a>() -> impl Parser< min_indent, "generates", EGenerates::Generates, - EGenerates::Space, EGenerates::IndentGenerates, EGenerates::IndentTypeStart ), @@ -765,7 +747,6 @@ fn generates_with<'a>() -> impl Parser< min_indent, "with", EGeneratesWith::With, - EGeneratesWith::Space, EGeneratesWith::IndentWith, EGeneratesWith::IndentListStart ), @@ -776,7 +757,6 @@ fn generates_with<'a>() -> impl Parser< word1(b']', EGeneratesWith::ListEnd), min_indent, EGeneratesWith::Open, - EGeneratesWith::Space, EGeneratesWith::IndentListEnd, Spaced::SpaceBefore ) @@ -799,7 +779,6 @@ fn imports<'a>() -> impl Parser< min_indent, "imports", EImports::Imports, - EImports::Space, EImports::IndentImports, EImports::IndentListStart ), @@ -810,7 +789,6 @@ fn imports<'a>() -> impl Parser< word1(b']', EImports::ListEnd), min_indent, EImports::Open, - EImports::Space, EImports::IndentListEnd, Spaced::SpaceBefore ) @@ -831,7 +809,7 @@ fn typed_ident<'a>() -> impl Parser<'a, Spaced<'a, TypedIdent<'a>>, ETypedIdent< |_, pos| ETypedIdent::Identifier(pos), lowercase_ident() )), - space0_e(min_indent, ETypedIdent::Space, ETypedIdent::IndentHasType) + space0_e(min_indent, ETypedIdent::IndentHasType) ), skip_first!( word1(b':', ETypedIdent::HasType), @@ -841,7 +819,6 @@ fn typed_ident<'a>() -> impl Parser<'a, Spaced<'a, TypedIdent<'a>>, ETypedIdent< type_annotation::located_help(min_indent, true) ), min_indent, - ETypedIdent::Space, ETypedIdent::IndentType, ) ) @@ -899,7 +876,6 @@ fn imports_entry<'a>() -> impl Parser<'a, Spaced<'a, ImportsEntry<'a>>, EImports word1(b'}', EImports::SetEnd), min_indent, EImports::Open, - EImports::Space, EImports::IndentSetEnd, Spaced::SpaceBefore ) diff --git a/compiler/parse/src/parser.rs b/compiler/parse/src/parser.rs index 25471207a8..d29b62f85b 100644 --- a/compiler/parse/src/parser.rs +++ b/compiler/parse/src/parser.rs @@ -63,6 +63,50 @@ pub enum SyntaxError<'a> { Space(BadInputError), NotEndOfFile(Position), } +pub trait SpaceProblem { + fn space_problem(e: BadInputError, pos: Position) -> Self; +} + +macro_rules! impl_space_problem { + ($($name:ident $(< $lt:tt >)?),*) => { + $( + impl $(< $lt >)? SpaceProblem for $name $(< $lt >)? { + fn space_problem(e: BadInputError, pos: Position) -> Self { + Self::Space(e, pos) + } + } + )* + }; +} + +impl_space_problem! { + EExpect<'a>, + EExposes, + EExpr<'a>, + EGenerates, + EGeneratesWith, + EHeader<'a>, + EIf<'a>, + EImports, + EInParens<'a>, + ELambda<'a>, + EList<'a>, + EPackageEntry<'a>, + EPackages<'a>, + EPattern<'a>, + EProvides<'a>, + ERecord<'a>, + ERequires<'a>, + EString<'a>, + EType<'a>, + ETypeInParens<'a>, + ETypeRecord<'a>, + ETypeTagUnion<'a>, + ETypedIdent<'a>, + EWhen<'a>, + PInParens<'a>, + PRecord<'a> +} #[derive(Debug, Clone, PartialEq, Eq)] pub enum EHeader<'a> { @@ -505,6 +549,8 @@ pub enum PInParens<'a> { #[derive(Debug, Clone, PartialEq, Eq)] pub enum EType<'a> { + Space(BadInputError, Position), + TRecord(ETypeRecord<'a>, Position), TTagUnion(ETypeTagUnion<'a>, Position), TInParens(ETypeInParens<'a>, Position), @@ -516,7 +562,6 @@ pub enum EType<'a> { /// TStart(Position), TEnd(Position), - TSpace(BadInputError, Position), TFunctionArgument(Position), /// TIndentStart(Position), @@ -1153,11 +1198,11 @@ macro_rules! collection { #[macro_export] macro_rules! collection_trailing_sep_e { - ($opening_brace:expr, $elem:expr, $delimiter:expr, $closing_brace:expr, $min_indent:expr, $open_problem:expr, $space_problem:expr, $indent_problem:expr, $space_before:expr) => { + ($opening_brace:expr, $elem:expr, $delimiter:expr, $closing_brace:expr, $min_indent:expr, $open_problem:expr, $indent_problem:expr, $space_before:expr) => { skip_first!( $opening_brace, |arena, state| { - let (_, spaces, state) = space0_e($min_indent, $space_problem, $indent_problem) + let (_, spaces, state) = space0_e($min_indent, $indent_problem) .parse(arena, state)?; let (_, (mut parsed_elems, mut final_comments), state) = @@ -1167,12 +1212,11 @@ macro_rules! collection_trailing_sep_e { $crate::blankspace::space0_before_optional_after( $elem, $min_indent, - $space_problem, $indent_problem, $indent_problem ) ), - $crate::blankspace::space0_e($min_indent, $space_problem, $indent_problem) + $crate::blankspace::space0_e($min_indent, $indent_problem) ).parse(arena, state)?; let (_,_, state) = diff --git a/compiler/parse/src/pattern.rs b/compiler/parse/src/pattern.rs index c76d115fc9..aa54dde475 100644 --- a/compiler/parse/src/pattern.rs +++ b/compiler/parse/src/pattern.rs @@ -75,8 +75,7 @@ fn loc_tag_pattern_arg<'a>(min_indent: u32) -> impl Parser<'a, Loc>, // If we encounter one, we're done parsing function args! move |arena, state| { let (_, spaces, state) = - backtrackable(space0_e(min_indent, EPattern::Space, EPattern::IndentStart)) - .parse(arena, state)?; + backtrackable(space0_e(min_indent, EPattern::IndentStart)).parse(arena, state)?; let (_, loc_pat, state) = loc_parse_tag_pattern_arg(min_indent, arena, state)?; @@ -123,7 +122,6 @@ fn loc_pattern_in_parens_help<'a>( move |arena, state| specialize_ref(PInParens::Pattern, loc_pattern_help(min_indent)) .parse(arena, state), min_indent, - PInParens::Space, PInParens::IndentOpen, PInParens::IndentEnd, ), @@ -321,7 +319,6 @@ fn record_pattern_help<'a>(min_indent: u32) -> impl Parser<'a, Pattern<'a>, PRec word1(b'}', PRecord::End), min_indent, PRecord::Open, - PRecord::Space, PRecord::IndentEnd, Pattern::SpaceBefore ) @@ -347,8 +344,7 @@ fn record_pattern_field<'a>(min_indent: u32) -> impl Parser<'a, Loc> .parse(arena, state)?; debug_assert_eq!(progress, MadeProgress); - let (_, spaces, state) = - space0_e(min_indent, PRecord::Space, PRecord::IndentEnd).parse(arena, state)?; + let (_, spaces, state) = space0_e(min_indent, PRecord::IndentEnd).parse(arena, state)?; // Having a value is optional; both `{ email }` and `{ email: blah }` work. // (This is true in both literals and types.) @@ -362,7 +358,7 @@ fn record_pattern_field<'a>(min_indent: u32) -> impl Parser<'a, Loc> Some(First(_)) => { let val_parser = specialize_ref(PRecord::Pattern, loc_pattern_help(min_indent)); let (_, loc_val, state) = - space0_before_e(val_parser, min_indent, PRecord::Space, PRecord::IndentColon) + space0_before_e(val_parser, min_indent, PRecord::IndentColon) .parse(arena, state)?; let Loc { @@ -392,7 +388,7 @@ fn record_pattern_field<'a>(min_indent: u32) -> impl Parser<'a, Loc> }); let (_, loc_val, state) = - space0_before_e(val_parser, min_indent, PRecord::Space, PRecord::IndentColon) + space0_before_e(val_parser, min_indent, PRecord::IndentColon) .parse(arena, state)?; let Loc { diff --git a/compiler/parse/src/type_annotation.rs b/compiler/parse/src/type_annotation.rs index c73aebab6c..9a08ea1694 100644 --- a/compiler/parse/src/type_annotation.rs +++ b/compiler/parse/src/type_annotation.rs @@ -28,7 +28,6 @@ fn tag_union_type<'a>(min_indent: u32) -> impl Parser<'a, TypeAnnotation<'a>, ET word1(b']', ETypeTagUnion::End), min_indent, ETypeTagUnion::Open, - ETypeTagUnion::Space, ETypeTagUnion::IndentEnd, Tag::SpaceBefore ) @@ -87,16 +86,11 @@ fn check_type_alias( fn parse_type_alias_after_as<'a>(min_indent: u32) -> impl Parser<'a, AliasHeader<'a>, EType<'a>> { move |arena, state| { - space0_before_e( - term(min_indent), - min_indent, - EType::TSpace, - EType::TAsIndentStart, - ) - .parse(arena, state) - .and_then(|(p, annot, state)| { - specialize(EType::TInlineAlias, check_type_alias(p, annot)).parse(arena, state) - }) + space0_before_e(term(min_indent), min_indent, EType::TAsIndentStart) + .parse(arena, state) + .and_then(|(p, annot, state)| { + specialize(EType::TInlineAlias, check_type_alias(p, annot)).parse(arena, state) + }) } } @@ -122,7 +116,7 @@ fn term<'a>(min_indent: u32) -> impl Parser<'a, Loc>, EType<' map!( and!( skip_second!( - backtrackable(space0_e(min_indent, EType::TSpace, EType::TIndentEnd)), + backtrackable(space0_e(min_indent, EType::TIndentEnd)), crate::parser::keyword_e(keyword::AS, EType::TEnd) ), parse_type_alias_after_as(min_indent) @@ -170,7 +164,7 @@ fn loc_applied_arg<'a>(min_indent: u32) -> impl Parser<'a, Loc( move |arena, state| specialize_ref(ETypeInParens::Type, expression(min_indent, true)) .parse(arena, state), min_indent, - ETypeInParens::Space, ETypeInParens::IndentOpen, ETypeInParens::IndentEnd, ), @@ -263,7 +256,7 @@ fn record_type_field<'a>( debug_assert_eq!(progress, MadeProgress); let (_, spaces, state) = - space0_e(min_indent, ETypeRecord::Space, ETypeRecord::IndentEnd).parse(arena, state)?; + space0_e(min_indent, ETypeRecord::IndentEnd).parse(arena, state)?; // Having a value is optional; both `{ email }` and `{ email: blah }` work. // (This is true in both literals and types.) @@ -277,13 +270,9 @@ fn record_type_field<'a>( match opt_loc_val { Some(First(_)) => { - let (_, loc_val, state) = space0_before_e( - val_parser, - min_indent, - ETypeRecord::Space, - ETypeRecord::IndentColon, - ) - .parse(arena, state)?; + let (_, loc_val, state) = + space0_before_e(val_parser, min_indent, ETypeRecord::IndentColon) + .parse(arena, state)?; Ok(( MadeProgress, @@ -292,13 +281,9 @@ fn record_type_field<'a>( )) } Some(Second(_)) => { - let (_, loc_val, state) = space0_before_e( - val_parser, - min_indent, - ETypeRecord::Space, - ETypeRecord::IndentOptional, - ) - .parse(arena, state)?; + let (_, loc_val, state) = + space0_before_e(val_parser, min_indent, ETypeRecord::IndentOptional) + .parse(arena, state)?; Ok(( MadeProgress, @@ -336,7 +321,6 @@ fn record_type<'a>(min_indent: u32) -> impl Parser<'a, TypeAnnotation<'a>, EType word1(b'}', ETypeRecord::End), min_indent, ETypeRecord::Open, - ETypeRecord::Space, ETypeRecord::IndentEnd, AssignedField::SpaceBefore ) @@ -389,13 +373,8 @@ fn expression<'a>( is_trailing_comma_valid: bool, ) -> impl Parser<'a, Loc>, EType<'a>> { (move |arena, state: State<'a>| { - let (p1, first, state) = space0_before_e( - term(min_indent), - min_indent, - EType::TSpace, - EType::TIndentStart, - ) - .parse(arena, state)?; + let (p1, first, state) = space0_before_e(term(min_indent), min_indent, EType::TIndentStart) + .parse(arena, state)?; let result = and![ zero_or_more!(skip_first!( @@ -404,7 +383,6 @@ fn expression<'a>( space0_around_ee( term(min_indent), min_indent, - EType::TSpace, EType::TIndentStart, EType::TIndentEnd ), @@ -419,7 +397,7 @@ fn expression<'a>( // TODO this space0 is dropped, so newlines just before the function arrow when there // is only one argument are not seen by the formatter. Can we do better? skip_second!( - space0_e(min_indent, EType::TSpace, EType::TIndentStart), + space0_e(min_indent, EType::TIndentStart), word2(b'-', b'>', EType::TStart) ) .trace("type_annotation:expression:arrow") @@ -428,13 +406,9 @@ fn expression<'a>( match result { Ok((p2, (rest, _dropped_spaces), state)) => { - let (p3, return_type, state) = space0_before_e( - term(min_indent), - min_indent, - EType::TSpace, - EType::TIndentStart, - ) - .parse(arena, state)?; + let (p3, return_type, state) = + space0_before_e(term(min_indent), min_indent, EType::TIndentStart) + .parse(arena, state)?; // prepare arguments let mut arguments = Vec::with_capacity_in(rest.len() + 1, arena); @@ -452,7 +426,7 @@ fn expression<'a>( Err(err) => { if !is_trailing_comma_valid { let (_, comma, _) = optional(skip_first!( - space0_e(min_indent, EType::TSpace, EType::TIndentStart), + space0_e(min_indent, EType::TIndentStart), word1(b',', EType::TStart) )) .trace("check trailing comma") diff --git a/compiler/parse/tests/snapshots/pass/interface_with_newline.header.roc b/compiler/parse/tests/snapshots/pass/interface_with_newline.header.roc index 696cdada15..18d25e0fb5 100644 --- a/compiler/parse/tests/snapshots/pass/interface_with_newline.header.roc +++ b/compiler/parse/tests/snapshots/pass/interface_with_newline.header.roc @@ -1,2 +1 @@ - interface T exposes [] imports [] diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 8c2baee3e5..e2ace30be3 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -895,8 +895,11 @@ fn type_to_variable<'a>( let tag_union_var = register(subs, rank, pools, content); - subs.set_content( + register_with_known_var( + subs, *rec_var, + rank, + pools, Content::RecursionVar { opt_name: None, structure: tag_union_var, @@ -2030,3 +2033,22 @@ fn register(subs: &mut Subs, rank: Rank, pools: &mut Pools, content: Content) -> var } + +fn register_with_known_var( + subs: &mut Subs, + var: Variable, + rank: Rank, + pools: &mut Pools, + content: Content, +) { + let descriptor = Descriptor { + content, + rank, + mark: Mark::NONE, + copy: OptVariable::NONE, + }; + + subs.set(var, descriptor); + + pools.get_mut(rank).push(var); +} diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index d8c5a38d6e..f524adca8a 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -10,7 +10,6 @@ mod helpers; #[cfg(test)] mod solve_expr { use crate::helpers::with_larger_debug_stack; - use roc_can::builtins::builtin_defs_map; use roc_collections::all::MutMap; use roc_types::pretty_print::{content_to_string, name_all_type_vars}; @@ -64,7 +63,6 @@ mod solve_expr { dir.path(), exposed_types, roc_target::TargetInfo::default_x86_64(), - builtin_defs_map, ); dir.close()?; @@ -3036,7 +3034,6 @@ mod solve_expr { } #[test] - #[ignore] fn typecheck_mutually_recursive_tag_union_2() { infer_eq_without_problem( indoc!( @@ -3064,7 +3061,6 @@ mod solve_expr { } #[test] - #[ignore] fn typecheck_mutually_recursive_tag_union_listabc() { infer_eq_without_problem( indoc!( @@ -5196,4 +5192,63 @@ mod solve_expr { r#"{ bi128 : I128 -> I128, bi16 : I16 -> I16, bi32 : I32 -> I32, bi64 : I64 -> I64, bi8 : I8 -> I8, bnat : Nat -> Nat, bu128 : U128 -> U128, bu16 : U16 -> U16, bu32 : U32 -> U32, bu64 : U64 -> U64, bu8 : U8 -> U8, dec : Dec -> Dec, f32 : F32 -> F32, f64 : F64 -> F64, fdec : Dec -> Dec, ff32 : F32 -> F32, ff64 : F64 -> F64, i128 : I128 -> I128, i16 : I16 -> I16, i32 : I32 -> I32, i64 : I64 -> I64, i8 : I8 -> I8, nat : Nat -> Nat, u128 : U128 -> U128, u16 : U16 -> U16, u32 : U32 -> U32, u64 : U64 -> U64, u8 : U8 -> U8 }"#, ) } + + #[test] + fn issue_2458() { + infer_eq_without_problem( + indoc!( + r#" + Foo a : [ Blah (Result (Bar a) { val: a }) ] + Bar a : Foo a + + v : Bar U8 + v = Blah (Ok (Blah (Err { val: 1 }))) + + v + "# + ), + "Bar U8", + ) + } + + // https://github.com/rtfeldman/roc/issues/2379 + #[test] + fn copy_vars_referencing_copied_vars() { + infer_eq_without_problem( + indoc!( + r#" + Job : [ Job [ Command ] (List Job) ] + + job : Job + + job + "# + ), + "Job", + ) + } + + #[test] + fn copy_vars_referencing_copied_vars_specialized() { + infer_eq_without_problem( + indoc!( + r#" + Job a : [ Job [ Command ] (Job a) (List (Job a)) a ] + + job : Job Str + + when job is + Job _ j lst _ -> + when j is + Job _ _ _ s -> + { j, lst, s } + "# + ), + // TODO: this means that we're doing our job correctly, as now both `Job a`s have been + // specialized to the same type, and the second destructuring proves the reified type + // is `Job Str`. But we should just print the structure of the recursive type directly. + // See https://github.com/rtfeldman/roc/issues/2513 + "{ j : a, lst : List a, s : Str }", + ) + } } diff --git a/compiler/test_gen/src/gen_list.rs b/compiler/test_gen/src/gen_list.rs index 585013cc54..4fbe60537d 100644 --- a/compiler/test_gen/src/gen_list.rs +++ b/compiler/test_gen/src/gen_list.rs @@ -99,7 +99,7 @@ fn bool_list_literal() { true : Bool true = True - List.repeat 23 true + List.repeat true 23 "# ), RocList::from_slice(&[true; 23]), @@ -112,7 +112,7 @@ fn bool_list_literal() { true : Bool true = True - List.repeat 23 { x: true, y: true } + List.repeat { x: true, y: true } 23 "# ), RocList::from_slice(&[[true, true]; 23]), @@ -125,7 +125,7 @@ fn bool_list_literal() { true : Bool true = True - List.repeat 23 { x: true, y: true, a: true, b: true, c: true, d : true, e: true, f: true } + List.repeat { x: true, y: true, a: true, b: true, c: true, d : true, e: true, f: true } 23 "# ), RocList::from_slice(&[[true, true, true, true, true, true, true, true]; 23]), @@ -1240,18 +1240,18 @@ fn list_single() { #[cfg(any(feature = "gen-llvm"))] fn list_repeat() { assert_evals_to!( - "List.repeat 5 1", + "List.repeat 1 5", RocList::from_slice(&[1, 1, 1, 1, 1]), RocList ); assert_evals_to!( - "List.repeat 4 2", + "List.repeat 2 4", RocList::from_slice(&[2, 2, 2, 2]), RocList ); assert_evals_to!( - "List.repeat 2 []", + "List.repeat [] 2", RocList::from_slice(&[RocList::default(), RocList::default()]), RocList> ); @@ -1263,7 +1263,7 @@ fn list_repeat() { noStrs = [] - List.repeat 2 noStrs + List.repeat noStrs 2 "# ), RocList::from_slice(&[RocList::default(), RocList::default()]), @@ -1271,7 +1271,7 @@ fn list_repeat() { ); assert_evals_to!( - "List.repeat 15 4", + "List.repeat 4 15", RocList::from_slice(&[4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4]), RocList ); @@ -2363,7 +2363,7 @@ fn list_keep_errs() { #[cfg(any(feature = "gen-llvm"))] fn list_map_with_index() { assert_evals_to!( - "List.mapWithIndex [0,0,0] (\\index, x -> Num.intCast index + x)", + "List.mapWithIndex [0,0,0] (\\x, index -> Num.intCast index + x)", RocList::from_slice(&[0, 1, 2]), RocList ); diff --git a/compiler/test_gen/src/gen_num.rs b/compiler/test_gen/src/gen_num.rs index fd7009328e..3a4951a36d 100644 --- a/compiler/test_gen/src/gen_num.rs +++ b/compiler/test_gen/src/gen_num.rs @@ -2490,3 +2490,41 @@ fn monomorphized_ints_aliased() { u8 ) } + +#[test] +#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))] +fn to_float_f32() { + assert_evals_to!( + indoc!( + r#" + n : U8 + n = 100 + + f : F32 + f = Num.toFloat n + f + "# + ), + 100., + f32 + ) +} + +#[test] +#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))] +fn to_float_f64() { + assert_evals_to!( + indoc!( + r#" + n : U8 + n = 100 + + f : F64 + f = Num.toFloat n + f + "# + ), + 100., + f64 + ) +} diff --git a/compiler/test_gen/src/gen_tags.rs b/compiler/test_gen/src/gen_tags.rs index 89251ac981..c1643ecc9a 100644 --- a/compiler/test_gen/src/gen_tags.rs +++ b/compiler/test_gen/src/gen_tags.rs @@ -1474,3 +1474,83 @@ fn issue_2445() { i64 ); } + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn issue_2458() { + assert_evals_to!( + indoc!( + r#" + Foo a : [ Blah (Bar a), Nothing {} ] + Bar a : Foo a + + v : Bar {} + v = Blah (Blah (Nothing {})) + + when v is + Blah (Blah (Nothing {})) -> 15 + _ -> 25 + "# + ), + 15, + u8 + ) +} + +#[test] +#[ignore = "See https://github.com/rtfeldman/roc/issues/2466"] +#[cfg(any(feature = "gen-llvm"))] +fn issue_2458_deep_recursion_var() { + assert_evals_to!( + indoc!( + r#" + Foo a : [ Blah (Result (Bar a) {}) ] + Bar a : Foo a + + v : Bar {} + + when v is + Blah (Ok (Blah (Err {}))) -> "1" + _ -> "2" + "# + ), + 15, + u8 + ) +} + +#[test] +#[cfg(any(feature = "gen-llvm"))] +fn issue_1162() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + RBTree k : [ Node k (RBTree k) (RBTree k), Empty ] + + balance : a, RBTree a -> RBTree a + balance = \key, left -> + when left is + Node _ _ lRight -> + Node key lRight Empty + + _ -> + Empty + + + tree : RBTree {} + tree = + balance {} Empty + + main : U8 + main = + when tree is + Empty -> 15 + _ -> 25 + "# + ), + 15, + u8 + ) +} diff --git a/compiler/test_gen/src/helpers/dev.rs b/compiler/test_gen/src/helpers/dev.rs index d17c326161..8942c7356f 100644 --- a/compiler/test_gen/src/helpers/dev.rs +++ b/compiler/test_gen/src/helpers/dev.rs @@ -1,7 +1,6 @@ use libloading::Library; use roc_build::link::{link, LinkType}; use roc_builtins::bitcode; -use roc_can::builtins::builtin_defs_map; use roc_collections::all::MutMap; use roc_region::all::LineInfo; use tempfile::tempdir; @@ -58,7 +57,6 @@ pub fn helper( src_dir, exposed_types, roc_target::TargetInfo::default_x86_64(), - builtin_defs_map, ); let mut loaded = loaded.expect("failed to load module"); diff --git a/compiler/test_gen/src/helpers/llvm.rs b/compiler/test_gen/src/helpers/llvm.rs index d9af1a821d..d7a24effa9 100644 --- a/compiler/test_gen/src/helpers/llvm.rs +++ b/compiler/test_gen/src/helpers/llvm.rs @@ -3,7 +3,6 @@ use inkwell::module::Module; use libloading::Library; use roc_build::link::module_to_dylib; use roc_build::program::FunctionIterator; -use roc_can::builtins::builtin_defs_map; use roc_can::def::Def; use roc_collections::all::{MutMap, MutSet}; use roc_gen_llvm::llvm::externs::add_default_roc_externs; @@ -25,9 +24,6 @@ fn promote_expr_to_module(src: &str) -> String { buffer } -pub fn test_builtin_defs(symbol: Symbol, var_store: &mut VarStore) -> Option { - builtin_defs_map(symbol, var_store) -} #[allow(clippy::too_many_arguments)] fn create_llvm_module<'a>( @@ -67,7 +63,6 @@ fn create_llvm_module<'a>( src_dir, exposed_types, target_info, - test_builtin_defs, ); let mut loaded = match loaded { diff --git a/compiler/test_gen/src/helpers/wasm.rs b/compiler/test_gen/src/helpers/wasm.rs index a0fb311283..3420cb2498 100644 --- a/compiler/test_gen/src/helpers/wasm.rs +++ b/compiler/test_gen/src/helpers/wasm.rs @@ -7,7 +7,6 @@ use std::path::{Path, PathBuf}; use wasmer::{Memory, WasmPtr}; use crate::helpers::from_wasmer_memory::FromWasmerMemory; -use roc_can::builtins::builtin_defs_map; use roc_collections::all::{MutMap, MutSet}; use roc_gen_wasm::wasm32_result::Wasm32Result; use roc_gen_wasm::{DEBUG_LOG_SETTINGS, MEMORY_NAME}; @@ -94,7 +93,6 @@ fn compile_roc_to_wasm_bytes<'a, T: Wasm32Result>( src_dir, exposed_types, roc_target::TargetInfo::default_wasm32(), - builtin_defs_map, ); let loaded = loaded.expect("failed to load module"); diff --git a/compiler/test_mono/src/tests.rs b/compiler/test_mono/src/tests.rs index 34001fe46a..6903714eaa 100644 --- a/compiler/test_mono/src/tests.rs +++ b/compiler/test_mono/src/tests.rs @@ -18,7 +18,6 @@ const EXPANDED_STACK_SIZE: usize = 8 * 1024 * 1024; use test_mono_macros::*; -use roc_can::builtins::builtin_defs_map; use roc_collections::all::MutMap; use roc_module::symbol::Symbol; use roc_mono::ir::Proc; @@ -107,7 +106,6 @@ fn compiles_to_ir(test_name: &str, src: &str) { src_dir, exposed_types, TARGET_INFO, - builtin_defs_map, ); let mut loaded = match loaded { diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 7dac3c50c7..3227c58796 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -1633,7 +1633,7 @@ roc_error_macros::assert_sizeof_all!(FlatType, 3 * 8); roc_error_macros::assert_sizeof_aarch64!((Variable, Option), 4 * 8); roc_error_macros::assert_sizeof_wasm!((Variable, Option), 4 * 4); -roc_error_macros::assert_sizeof_all!((Variable, Option), 4 * 8); +roc_error_macros::assert_sizeof_default!((Variable, Option), 4 * 8); #[derive(Clone, Debug)] pub enum Content { diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 15fa24a597..af99cb5303 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -3,6 +3,7 @@ use crate::subs::{ GetSubsSlice, RecordFields, Subs, UnionTags, VarStore, Variable, VariableSubsSlice, }; use roc_collections::all::{ImMap, ImSet, Index, MutSet, SendMap}; +use roc_error_macros::internal_error; use roc_module::called_via::CalledVia; use roc_module::ident::{ForeignSymbol, Ident, Lowercase, TagName}; use roc_module::low_level::LowLevel; @@ -595,9 +596,15 @@ impl Type { ext.substitute_alias(rep_symbol, rep_args, actual) } Alias { + type_arguments, actual: alias_actual, .. - } => alias_actual.substitute_alias(rep_symbol, rep_args, actual), + } => { + for (_, ta) in type_arguments { + ta.substitute_alias(rep_symbol, rep_args, actual)?; + } + alias_actual.substitute_alias(rep_symbol, rep_args, actual) + } HostExposedAlias { actual: actual_type, .. @@ -809,22 +816,24 @@ impl Type { substitution.insert(*placeholder, filler); } + // make sure hidden variables are freshly instantiated let mut lambda_set_variables = Vec::with_capacity(alias.lambda_set_variables.len()); - for lambda_set in alias.lambda_set_variables.iter() { - let fresh = var_store.fresh(); - introduced.insert(fresh); - - lambda_set_variables.push(LambdaSet(Type::Variable(fresh))); - - if let Type::Variable(lambda_set_var) = lambda_set.0 { - substitution.insert(lambda_set_var, Type::Variable(fresh)); + for typ in alias.lambda_set_variables.iter() { + if let Type::Variable(var) = typ.0 { + let fresh = var_store.fresh(); + introduced.insert(fresh); + substitution.insert(var, Type::Variable(fresh)); + lambda_set_variables.push(LambdaSet(Type::Variable(fresh))); + } else { + unreachable!("at this point there should be only vars in there"); } } - actual.substitute(&substitution); actual.instantiate_aliases(region, aliases, var_store, introduced); + actual.substitute(&substitution); + // instantiate recursion variable! if let Type::RecursiveTagUnion(rec_var, mut tags, mut ext) = actual { let new_rec_var = var_store.fresh(); @@ -836,20 +845,15 @@ impl Type { } ext.substitute(&substitution); - *self = Type::Alias { - symbol: *symbol, - type_arguments: named_args, - lambda_set_variables: alias.lambda_set_variables.clone(), - actual: Box::new(Type::RecursiveTagUnion(new_rec_var, tags, ext)), - }; - } else { - *self = Type::Alias { - symbol: *symbol, - type_arguments: named_args, - lambda_set_variables: alias.lambda_set_variables.clone(), - actual: Box::new(actual), - }; + actual = Type::RecursiveTagUnion(new_rec_var, tags, ext); } + + *self = Type::Alias { + symbol: *symbol, + type_arguments: named_args, + lambda_set_variables, + actual: Box::new(actual), + }; } else { // one of the special-cased Apply types. for x in args { @@ -863,6 +867,61 @@ impl Type { EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => {} } } + + pub fn is_tag_union_like(&self) -> bool { + matches!( + self, + Type::TagUnion(..) + | Type::RecursiveTagUnion(..) + | Type::FunctionOrTagUnion(..) + | Type::EmptyTagUnion + ) + } + + /// We say a type is "narrow" if no type composing it is a proper sum; that is, no type + /// composing it is a tag union with more than one variant. + /// + /// The types checked here must have all of their non-builtin `Apply`s instantiated, as a + /// non-instantiated `Apply` would be ambiguous. + /// + /// The following are narrow: + /// + /// ```roc + /// U8 + /// [ A I8 ] + /// [ A [ B [ C U8 ] ] ] + /// [ A (R a) ] as R a + /// ``` + /// + /// The following are not: + /// + /// ```roc + /// [ A I8, B U8 ] + /// [ A [ B [ Result U8 {} ] ] ] (Result U8 {} is actually [ Ok U8, Err {} ]) + /// [ A { lst: List (R a) } ] as R a (List a is morally [ Cons (List a), Nil ] as List a) + /// ``` + pub fn is_narrow(&self) -> bool { + match self.shallow_dealias() { + Type::TagUnion(tags, ext) | Type::RecursiveTagUnion(_, tags, ext) => { + ext.is_empty_tag_union() + && tags.len() == 1 + && tags[0].1.len() == 1 + && tags[0].1[0].is_narrow() + } + Type::Record(fields, ext) => { + fields.values().all(|field| field.as_inner().is_narrow()) && ext.is_narrow() + } + Type::Function(args, clos, ret) => { + args.iter().all(|a| a.is_narrow()) && clos.is_narrow() && ret.is_narrow() + } + // Lists and sets are morally two-tagged unions, as they can be empty + Type::Apply(Symbol::LIST_LIST | Symbol::SET_SET, _, _) => false, + Type::Apply(..) => internal_error!("cannot chase an Apply!"), + Type::Alias { .. } => internal_error!("should be dealiased"), + // Non-composite types are trivially narrow + _ => true, + } + } } fn symbols_help(tipe: &Type, accum: &mut ImSet) { diff --git a/docs/src/lib.rs b/docs/src/lib.rs index 8c791b72ee..5749caf14c 100644 --- a/docs/src/lib.rs +++ b/docs/src/lib.rs @@ -5,7 +5,6 @@ use def::defs_to_html; use docs_error::DocsResult; use expr::expr_to_html; use roc_builtins::std::StdLib; -use roc_can::builtins::builtin_defs_map; use roc_can::scope::Scope; use roc_collections::all::MutMap; use roc_load::docs::DocEntry::DocDef; @@ -429,7 +428,6 @@ pub fn load_modules_for_files(filenames: Vec, std_lib: StdLib) -> Vec modules.push(loaded), Err(LoadingProblem::FormattedReport(report)) => { diff --git a/editor/.gitignore b/editor/.gitignore index 8b13789179..e69de29bb2 100644 --- a/editor/.gitignore +++ b/editor/.gitignore @@ -1 +0,0 @@ - diff --git a/editor/src/graphics/shaders/shader.wgsl b/editor/src/graphics/shaders/shader.wgsl index 343f8b0d14..e25484bdb1 100644 --- a/editor/src/graphics/shaders/shader.wgsl +++ b/editor/src/graphics/shaders/shader.wgsl @@ -1,4 +1,3 @@ - struct VertexOutput { [[location(0)]] color: vec4; [[builtin(position)]] position: vec4; diff --git a/examples/benchmarks/Bytes/Encode.roc b/examples/benchmarks/Bytes/Encode.roc index a541f4806b..37321e8506 100644 --- a/examples/benchmarks/Bytes/Encode.roc +++ b/examples/benchmarks/Bytes/Encode.roc @@ -57,7 +57,7 @@ getWidths = \encoders, initial -> encode : Encoder -> List U8 encode = \encoder -> - output = List.repeat (getWidth encoder) 0 + output = List.repeat 0 (getWidth encoder) encodeHelp encoder 0 output |> .output diff --git a/examples/cli/echo.roc b/examples/cli/echo.roc index 9f5401ab3c..29cb05c17c 100644 --- a/examples/cli/echo.roc +++ b/examples/cli/echo.roc @@ -18,12 +18,12 @@ echo = \shout -> silence = \length -> spaceInUtf8 = 32 - List.repeat length spaceInUtf8 + List.repeat spaceInUtf8 length shout |> Str.toUtf8 |> List.mapWithIndex - (\i, _ -> + (\_, i -> length = (List.len (Str.toUtf8 shout) - i) phrase = (List.split (Str.toUtf8 shout) length).before diff --git a/examples/false-interpreter/Context.roc b/examples/false-interpreter/Context.roc index 11fea10961..57dbf1171c 100644 --- a/examples/false-interpreter/Context.roc +++ b/examples/false-interpreter/Context.roc @@ -81,7 +81,7 @@ with = \path, callback -> # I cant define scope here and put it in the list in callback. It breaks alias anaysis. # Instead I have to inline this. # root_scope = { data: Some handle, index: 0, buf: [], whileInfo: None } - callback { scopes: [ { data: Some handle, index: 0, buf: [], whileInfo: None } ], state: Executing, stack: [], vars: List.repeat Variable.totalCount (Number 0) } + callback { scopes: [ { data: Some handle, index: 0, buf: [], whileInfo: None } ], state: Executing, stack: [], vars: List.repeat (Number 0) Variable.totalCount } # I am pretty sure there is a syntax to destructure and keep a reference to the whole, but Im not sure what it is. getChar : Context -> Task [ T U8 Context ] [ EndOfData, NoScope ]* diff --git a/repl_eval/src/gen.rs b/repl_eval/src/gen.rs index 0874c42b49..a11ba966b8 100644 --- a/repl_eval/src/gen.rs +++ b/repl_eval/src/gen.rs @@ -1,7 +1,6 @@ use bumpalo::Bump; use std::path::{Path, PathBuf}; -use roc_can::builtins::builtin_defs_map; use roc_collections::all::MutMap; use roc_fmt::annotation::Formattable; use roc_fmt::annotation::{Newlines, Parens}; @@ -65,7 +64,6 @@ pub fn compile_to_mono<'a>( src_dir, exposed_types, target_info, - builtin_defs_map, ); let mut loaded = match loaded { diff --git a/repl_www/build.sh b/repl_www/build.sh index 2b9c6e4023..549dbfaf1c 100755 --- a/repl_www/build.sh +++ b/repl_www/build.sh @@ -17,8 +17,12 @@ WWW_DIR="repl_www/build" mkdir -p $WWW_DIR cp repl_www/public/* $WWW_DIR -# Pass all script arguments through to wasm-pack (such as --release) -wasm-pack build --target web "$@" repl_wasm +# Pass all script arguments through to wasm-pack +# For debugging, pass the --profiling option, which enables optimizations + debug info +# (We need optimizations to get rid of dead code that otherwise causes compile errors!) +cargo build --target wasm32-unknown-unknown -p roc_repl_wasm --release +wasm-bindgen --target web --keep-debug target/wasm32-unknown-unknown/release/roc_repl_wasm.wasm --out-dir repl_wasm/pkg/ +# wasm-pack build --target web "$@" repl_wasm cp repl_wasm/pkg/*.wasm $WWW_DIR diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 4ac42f5fec..a9c1251647 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -148,6 +148,9 @@ pub fn cyclic_alias<'b>( region: roc_region::all::Region, others: Vec, ) -> (RocDocBuilder<'b>, String) { + let when_is_recursion_legal = + alloc.reflow("Recursion in aliases is only allowed if recursion happens behind a tagged union, at least one variant of which is not recursive."); + let doc = if others.is_empty() { alloc.stack(vec![ alloc @@ -155,7 +158,7 @@ pub fn cyclic_alias<'b>( .append(alloc.symbol_unqualified(symbol)) .append(alloc.reflow(" alias is self-recursive in an invalid way:")), alloc.region(lines.convert_region(region)), - alloc.reflow("Recursion in aliases is only allowed if recursion happens behind a tag."), + when_is_recursion_legal, ]) } else { alloc.stack(vec![ @@ -179,7 +182,7 @@ pub fn cyclic_alias<'b>( .map(|other| alloc.symbol_unqualified(other)) .collect::>(), ), - alloc.reflow("Recursion in aliases is only allowed if recursion happens behind a tag."), + when_is_recursion_legal, ]) }; diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 7a7abd9be2..20011557c9 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -2852,7 +2852,7 @@ mod test_reporting { ^^^ Recursion in aliases is only allowed if recursion happens behind a - tag. + tagged union, at least one variant of which is not recursive. "# ), ) @@ -3348,6 +3348,8 @@ mod test_reporting { { x, y } "# ), + // TODO render tag unions across multiple lines + // TODO do not show recursion var if the recursion var does not render on the surface of a type indoc!( r#" ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── @@ -3360,12 +3362,13 @@ mod test_reporting { This `ACons` global tag application has the type: - [ ACons Num (Integer Signed64) [ BCons (Num a) [ ACons Str [ BNil - ]b ]c ]d, ANil ] + [ ACons (Num (Integer Signed64)) [ + BCons (Num (Integer Signed64)) [ ACons Str [ BCons I64 a, BNil ], + ANil ], BNil ], ANil ] But the type annotation on `x` says it should be: - [ ACons I64 BList I64 I64, ANil ] + [ ACons I64 (BList I64 I64), ANil ] as a "# ), ) @@ -7075,7 +7078,7 @@ I need all branches in an `if` to have the same type! ^ Recursion in aliases is only allowed if recursion happens behind a - tag. + tagged union, at least one variant of which is not recursive. "# ), ) @@ -7102,7 +7105,7 @@ I need all branches in an `if` to have the same type! ^ Recursion in aliases is only allowed if recursion happens behind a - tag. + tagged union, at least one variant of which is not recursive. "# ), ) @@ -7128,7 +7131,7 @@ I need all branches in an `if` to have the same type! ^ Recursion in aliases is only allowed if recursion happens behind a - tag. + tagged union, at least one variant of which is not recursive. "# ), ) @@ -7977,4 +7980,111 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn recursive_type_alias_is_newtype() { + report_problem_as( + indoc!( + r#" + R a : [ Only (R a) ] + + v : R U8 + v + "# + ), + indoc!( + r#" + ── CYCLIC ALIAS ──────────────────────────────────────────────────────────────── + + The `R` alias is self-recursive in an invalid way: + + 1│ R a : [ Only (R a) ] + ^ + + Recursion in aliases is only allowed if recursion happens behind a + tagged union, at least one variant of which is not recursive. + "# + ), + ) + } + + #[test] + fn recursive_type_alias_is_newtype_deep() { + report_problem_as( + indoc!( + r#" + R a : [ Only { very: [ Deep (R a) ] } ] + + v : R U8 + v + "# + ), + indoc!( + r#" + ── CYCLIC ALIAS ──────────────────────────────────────────────────────────────── + + The `R` alias is self-recursive in an invalid way: + + 1│ R a : [ Only { very: [ Deep (R a) ] } ] + ^ + + Recursion in aliases is only allowed if recursion happens behind a + tagged union, at least one variant of which is not recursive. + "# + ), + ) + } + + #[test] + fn recursive_type_alias_is_newtype_mutual() { + report_problem_as( + indoc!( + r#" + Foo a : [ Thing (Bar a) ] + Bar a : [ Stuff (Foo a) ] + + v : Bar U8 + v + "# + ), + indoc!( + r#" + ── CYCLIC ALIAS ──────────────────────────────────────────────────────────────── + + The `Bar` alias is recursive in an invalid way: + + 2│ Bar a : [ Stuff (Foo a) ] + ^^^^^ + + The `Bar` alias depends on itself through the following chain of + definitions: + + ┌─────┐ + │ Bar + │ ↓ + │ Foo + └─────┘ + + Recursion in aliases is only allowed if recursion happens behind a + tagged union, at least one variant of which is not recursive. + "# + ), + ) + } + + #[test] + fn issue_2458() { + report_problem_as( + indoc!( + r#" + Foo a : [ Blah (Result (Bar a) []) ] + Bar a : Foo a + + v : Bar U8 + v + "# + ), + "", + ) + } } diff --git a/roc_std/src/lib.rs b/roc_std/src/lib.rs index 972152cb6e..0880c83038 100644 --- a/roc_std/src/lib.rs +++ b/roc_std/src/lib.rs @@ -140,7 +140,6 @@ impl RocList { assert!(capacity > 0); assert!(slice.len() <= capacity); - let ptr = slice.as_ptr(); let element_bytes = capacity * core::mem::size_of::(); let padding = { @@ -165,25 +164,11 @@ impl RocList { let refcount_ptr = raw_ptr as *mut isize; *(refcount_ptr.offset(-1)) = isize::MIN; - { - // NOTE: using a memcpy here causes weird issues - let target_ptr = raw_ptr as *mut T; - let source_ptr = ptr as *const T; - for index in 0..slice.len() { - let source = &*source_ptr.add(index); - let target = &mut *target_ptr.add(index); - - // NOTE for a weird reason, it's important that we clone onto the stack - // and explicitly forget the swapped-in value - // cloning directly from source to target causes some garbage memory (cast to a - // RocStr) to end up in the drop implementation of RocStr and cause havoc by - // freeing NULL - let mut temporary = source.clone(); - - core::mem::swap(target, &mut temporary); - - core::mem::forget(temporary); - } + // Clone the elements into the new array. + let target_ptr = raw_ptr; + for (i, value) in slice.iter().cloned().enumerate() { + let target_ptr = target_ptr.add(i); + target_ptr.write(value); } raw_ptr