From 90b43d77d57cfa6273d5d3ede3de00f98701cda2 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Mon, 22 Nov 2021 19:45:48 +0100 Subject: [PATCH 01/45] encourage making issues when code is unclear. --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b4b0a3214..6035bb0897 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,8 @@ Earthly may temporarily use a lot of disk space, up to 90 GB. This disk space is - 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. +- Make sure to create a branch on the roc repository for your changes. Our CI can not be run on forks. +- Create an issue if the purpose of a struct/field/type/function/... is not immediately clear from its name or nearby comments. ## Can we do better? From 13552b11a68c5028bf45942a4f7694869df530e6 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Thu, 10 Feb 2022 08:15:48 -0500 Subject: [PATCH 02/45] Check self- and mutually-recursive aliases in the same pass --- compiler/can/src/def.rs | 205 +++++++++++++++++++----------------- compiler/types/src/types.rs | 8 +- 2 files changed, 113 insertions(+), 100 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 2e046dbb07..43c5d9104f 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -22,7 +22,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 +265,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,35 +302,6 @@ 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"); aliases.insert(symbol, alias.clone()); @@ -339,7 +309,7 @@ pub fn canonicalize_defs<'a>( // 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); + correct_mutual_recursive_type_alias(env, &mut scope, &mut aliases, var_store); // Now that we have the scope completely assembled, and shadowing resolved, // we're ready to canonicalize any body exprs. @@ -1564,6 +1534,7 @@ fn pending_typed_body<'a>( /// Make aliases recursive fn correct_mutual_recursive_type_alias<'a>( env: &mut Env<'a>, + scope: &mut Scope, aliases: &mut SendMap, var_store: &mut VarStore, ) { @@ -1586,82 +1557,113 @@ 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(); - // 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); - // 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 cycle.len() == 1 { + let symbol = cycle[0]; + let alias = aliases.get_mut(&symbol).unwrap(); + + if alias.typ.contains_symbol(symbol) { + let mut can_still_report_error = true; + let mut opt_rec_var = None; + + let _made_recursive = make_tag_union_of_alias_recursive( + env, + symbol, + alias, + vec![], + var_store, + &mut can_still_report_error, + &mut opt_rec_var, + ); + + scope.add_alias( + symbol, + alias.region, + alias.type_variables.clone(), + alias.typ.clone(), + ); + } + } else { + // This is a mutually recursive cycle + + // 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 mut opt_rec_var = None; + for symbol in cycle { + let alias = aliases.get_mut(&symbol).unwrap(); + + let _made_recursive = make_tag_union_of_alias_recursive( + env, + *symbol, + alias, + vec![], + var_store, + &mut can_still_report_error, + &mut opt_rec_var, + ); + } + + // 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) = aliases.get(other) { + to_instantiate.insert(*other, alias.clone()); } } + } - if let Some(alias) = aliases.get_mut(rec) { - alias.typ.instantiate_aliases( - alias.region, - &to_instantiate, - var_store, - &mut ImSet::default(), - ); - - let alias_args = &alias - .type_variables - .iter() - .map(|l| (l.value.0.clone(), Type::Variable(l.value.1))) - .collect::>(); - - 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, - ); - } + if let Some(alias) = aliases.get_mut(rec) { + alias.typ.instantiate_aliases( + alias.region, + &to_instantiate, + var_store, + &mut ImSet::default(), + ); } } } } } +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, + opt_rec_var: &mut Option, +) -> 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, + opt_rec_var, + ) +} + /// Attempt to make a tag union recursive at the position of `recursive_alias`; for example, /// /// ```roc @@ -1683,7 +1685,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, @@ -1691,6 +1693,7 @@ fn make_tag_union_recursive<'a>( typ: &mut Type, var_store: &mut VarStore, can_report_error: &mut bool, + opt_rec_var: &mut Option, ) -> Result<(), ()> { let Loc { value: (symbol, args), @@ -1699,7 +1702,10 @@ fn make_tag_union_recursive<'a>( let vars = args.iter().map(|(_, t)| t.clone()).collect::>(); match typ { Type::TagUnion(tags, ext) => { - let rec_var = var_store.fresh(); + if opt_rec_var.is_none() { + *opt_rec_var = Some(var_store.fresh()); + } + let rec_var = opt_rec_var.unwrap(); let mut pending_typ = Type::RecursiveTagUnion(rec_var, tags.to_vec(), ext.clone()); let substitution_result = pending_typ.substitute_alias(symbol, &vars, &Type::Variable(rec_var)); @@ -1724,7 +1730,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, @@ -1732,6 +1738,7 @@ fn make_tag_union_recursive<'a>( actual, var_store, can_report_error, + opt_rec_var, ), _ => { let problem = roc_types::types::Problem::CyclicAlias(symbol, region, others.clone()); diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 15fa24a597..68c11f2630 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -595,9 +595,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, .. From c064c500362798c4623952efcc0b2aaafa9cbefa Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Thu, 10 Feb 2022 22:12:33 -0500 Subject: [PATCH 03/45] Catch illegal alias cycles more strictly Part of #2458 --- compiler/can/src/def.rs | 134 ++++++++++++++++++++---------- compiler/types/src/types.rs | 49 +++++++++++ reporting/src/error/type.rs | 7 +- reporting/tests/test_reporting.rs | 99 +++++++++++++++++++++- 4 files changed, 240 insertions(+), 49 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 43c5d9104f..70f7e0aaf6 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1560,44 +1560,51 @@ fn correct_mutual_recursive_type_alias<'a>( // TODO investigate should this be in a loop? let defined_symbols: Vec = aliases.keys().copied().collect(); - let cycles = &strongly_connected_components(&defined_symbols, all_successors_with_self); + let cycles = strongly_connected_components(&defined_symbols, all_successors_with_self); - for cycle in cycles { + 'next_cycle: for cycle in cycles { debug_assert!(!cycle.is_empty()); + + // 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; + + // Go through and mark every self- and mutually-recursive alias cycle recursive. if cycle.len() == 1 { let symbol = cycle[0]; let alias = aliases.get_mut(&symbol).unwrap(); - if alias.typ.contains_symbol(symbol) { - let mut can_still_report_error = true; - let mut opt_rec_var = None; - - let _made_recursive = make_tag_union_of_alias_recursive( - env, - symbol, - alias, - vec![], - var_store, - &mut can_still_report_error, - &mut opt_rec_var, - ); - - scope.add_alias( - symbol, - alias.region, - alias.type_variables.clone(), - alias.typ.clone(), - ); + if !alias.typ.contains_symbol(symbol) { + // This alias is neither self nor mutually recursive. + continue 'next_cycle; } - } else { - // This is a mutually recursive cycle - // make sure we report only one error for the cycle, not an error for every - // alias in the cycle. + // This is a self-recursive cycle. let mut can_still_report_error = true; - let mut opt_rec_var = None; - for symbol in cycle { + + let _made_recursive = make_tag_union_of_alias_recursive( + env, + symbol, + alias, + vec![], + var_store, + &mut can_still_report_error, + &mut opt_rec_var, + ); + + scope.add_alias( + symbol, + alias.region, + alias.type_variables.clone(), + alias.typ.clone(), + ); + } else { + // This is a mutually recursive cycle. + let mut opt_rec_var = None; + + // First mark everything in the cycle recursive, as it needs to be. + for symbol in cycle.iter() { let alias = aliases.get_mut(&symbol).unwrap(); let _made_recursive = make_tag_union_of_alias_recursive( @@ -1611,20 +1618,23 @@ fn correct_mutual_recursive_type_alias<'a>( ); } + // Now go through and instantiate references that are recursive, but we didn't know + // they were until now. + // // TODO use itertools to be more efficient here - for rec in cycle { + for &rec in cycle.iter() { let mut to_instantiate = ImMap::default(); let mut others = Vec::with_capacity(cycle.len() - 1); - for other in cycle { + for &other in cycle.iter() { if rec != other { - others.push(*other); - if let Some(alias) = aliases.get(other) { - to_instantiate.insert(*other, alias.clone()); + others.push(other); + if let Some(alias) = aliases.get(&other) { + to_instantiate.insert(other, alias.clone()); } } } - if let Some(alias) = aliases.get_mut(rec) { + if let Some(alias) = aliases.get_mut(&rec) { alias.typ.instantiate_aliases( alias.region, &to_instantiate, @@ -1634,6 +1644,34 @@ fn correct_mutual_recursive_type_alias<'a>( } } } + + // The cycle we just marked recursive and instantiated may still be illegal cycles, 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 = &aliases.get(sym).unwrap().typ; + typ.is_tag_union_like() && typ.is_narrow() + }); + if !all_are_narrow { + // We pass through at least one tag union that has a non-recursive variant, so this + // cycle is legal. + continue 'next_cycle; + } + + let mut rest = cycle; + let alias_name = rest.pop().unwrap(); + + let alias = aliases.get_mut(&alias_name).unwrap(); + + mark_cyclic_alias( + env, + &mut alias.typ, + alias_name, + alias.region, + rest, + can_still_report_error, + ) } } @@ -1741,17 +1779,27 @@ fn make_tag_union_recursive_help<'a>( opt_rec_var, ), _ => { - let problem = roc_types::types::Problem::CyclicAlias(symbol, region, others.clone()); - *typ = Type::Erroneous(problem); + mark_cyclic_alias(env, typ, symbol, region, others.clone(), *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/types/src/types.rs b/compiler/types/src/types.rs index 68c11f2630..86a5b9fdc2 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; @@ -869,6 +870,54 @@ 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: + /// + /// U8 + /// [ A I8 ] + /// [ A [ B [ C U8 ] ] ] + /// [ A (R a) ] as R a + /// + /// The following are not: + /// + /// [ 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() + } + // 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/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..7f31407536 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. "# ), ) @@ -7075,7 +7075,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 +7102,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 +7128,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 +7977,95 @@ 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. + "# + ), + ) + } } From 8c0e39211de54578056dbfb9f02e2a30f06371f0 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Fri, 11 Feb 2022 08:43:33 -0500 Subject: [PATCH 04/45] Instantiate recursive aliases to their smallest closures Now, when we have two aliases like ``` T a : [ A, B (U a) ] U a : [ C, D (T a) ] ``` during the first pass, we simply canonicalize them but add neither to the scope. This means that `T` will not be instantiated in the definition of `U`. Only in the second pass, during correction, do we instantiate both aliases **independently**: ``` T a : [ A, B [ C, D (T a) ] ] U a : [ C, D [ A, B (U a) ] ] ``` and now we can mark each recursive, individually: ``` T a : [ A, B [ C, D ] ] as U a : [ C, D [ A, B ] ] as ``` This means that the surface types shown to users might be a bit larger, but it has the benefit that everything needed to understand a layout of a type in later passes is stored on the type directly, and we don't need to keep alias mappings. Since we sort by connected components, this should be complete. Closes #2458 --- compiler/can/src/def.rs | 160 +++++++++++++---------------- compiler/can/src/scope.rs | 82 ++++++++------- compiler/solve/tests/solve_expr.rs | 20 +++- compiler/test_gen/src/gen_tags.rs | 44 ++++++++ reporting/tests/test_reporting.rs | 25 ++++- 5 files changed, 200 insertions(+), 131 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 70f7e0aaf6..344f4b8a16 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; @@ -302,14 +303,21 @@ pub fn canonicalize_defs<'a>( 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 scope, &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. @@ -1534,18 +1542,17 @@ fn pending_typed_body<'a>( /// Make aliases recursive fn correct_mutual_recursive_type_alias<'a>( env: &mut Env<'a>, - scope: &mut Scope, - 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 @@ -1558,58 +1565,57 @@ fn correct_mutual_recursive_type_alias<'a>( }; // 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(); let cycles = strongly_connected_components(&defined_symbols, all_successors_with_self); + let mut solved_aliases = SendMap::default(); - 'next_cycle: for cycle in cycles { + for cycle in cycles { debug_assert!(!cycle.is_empty()); + let mut pending_aliases: SendMap<_, _> = cycle + .iter() + .map(|&sym| (sym, original_aliases.get(&sym).unwrap().clone())) + .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; - // Go through and mark every self- and mutually-recursive alias cycle recursive. - if cycle.len() == 1 { - let symbol = cycle[0]; - let alias = aliases.get_mut(&symbol).unwrap(); - - if !alias.typ.contains_symbol(symbol) { - // This alias is neither self nor mutually recursive. - continue 'next_cycle; + 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()); + } + } } - // This is a self-recursive cycle. - let mut can_still_report_error = true; - let mut opt_rec_var = None; - - let _made_recursive = make_tag_union_of_alias_recursive( - env, - symbol, - alias, - vec![], - var_store, - &mut can_still_report_error, - &mut opt_rec_var, - ); - - scope.add_alias( - symbol, + let alias = pending_aliases.get_mut(&rec).unwrap(); + alias.typ.instantiate_aliases( alias.region, - alias.type_variables.clone(), - alias.typ.clone(), + &to_instantiate, + var_store, + &mut ImSet::default(), ); - } else { - // This is a mutually recursive cycle. - let mut opt_rec_var = None; - // First mark everything in the cycle recursive, as it needs to be. - for symbol in cycle.iter() { - let alias = aliases.get_mut(&symbol).unwrap(); + // 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 mut opt_rec_var = None; let _made_recursive = make_tag_union_of_alias_recursive( env, - *symbol, + rec, alias, vec![], var_store, @@ -1617,62 +1623,38 @@ fn correct_mutual_recursive_type_alias<'a>( &mut opt_rec_var, ); } - - // Now go through and instantiate references that are recursive, but we didn't know - // they were until now. - // - // TODO use itertools to be more efficient here - for &rec in cycle.iter() { - let mut to_instantiate = ImMap::default(); - let mut others = Vec::with_capacity(cycle.len() - 1); - for &other in cycle.iter() { - if rec != other { - others.push(other); - if let Some(alias) = aliases.get(&other) { - to_instantiate.insert(other, alias.clone()); - } - } - } - - if let Some(alias) = aliases.get_mut(&rec) { - alias.typ.instantiate_aliases( - alias.region, - &to_instantiate, - var_store, - &mut ImSet::default(), - ); - } - } } - // The cycle we just marked recursive and instantiated may still be illegal cycles, if + // 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 = &aliases.get(sym).unwrap().typ; + let typ = &pending_aliases.get(sym).unwrap().typ; typ.is_tag_union_like() && typ.is_narrow() }); - if !all_are_narrow { - // We pass through at least one tag union that has a non-recursive variant, so this - // cycle is legal. - continue 'next_cycle; + + 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, + ) } - let mut rest = cycle; - let alias_name = rest.pop().unwrap(); - - let alias = 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>( diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index de7b233b08..cfcb1153ff 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -17,7 +17,7 @@ pub struct Scope { symbols: SendMap, /// The type aliases currently in scope - aliases: SendMap, + pub aliases: SendMap, /// The current module being processed. This will be used to turn /// unqualified idents into Symbols. @@ -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/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index d8c5a38d6e..d41886aea0 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -3036,7 +3036,6 @@ mod solve_expr { } #[test] - #[ignore] fn typecheck_mutually_recursive_tag_union_2() { infer_eq_without_problem( indoc!( @@ -3064,7 +3063,6 @@ mod solve_expr { } #[test] - #[ignore] fn typecheck_mutually_recursive_tag_union_listabc() { infer_eq_without_problem( indoc!( @@ -5196,4 +5194,22 @@ 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", + ) + } } diff --git a/compiler/test_gen/src/gen_tags.rs b/compiler/test_gen/src/gen_tags.rs index 89251ac981..c19868f4bf 100644 --- a/compiler/test_gen/src/gen_tags.rs +++ b/compiler/test_gen/src/gen_tags.rs @@ -1474,3 +1474,47 @@ 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 + ) +} diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index 7f31407536..20011557c9 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -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 "# ), ) @@ -8068,4 +8071,20 @@ I need all branches in an `if` to have the same type! ), ) } + + #[test] + fn issue_2458() { + report_problem_as( + indoc!( + r#" + Foo a : [ Blah (Result (Bar a) []) ] + Bar a : Foo a + + v : Bar U8 + v + "# + ), + "", + ) + } } From c71854d5a4f258c373f3dcd337b4f631c9f71076 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Fri, 11 Feb 2022 08:51:08 -0500 Subject: [PATCH 05/45] Remove opt_rec_var --- compiler/can/src/def.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 344f4b8a16..406d5c3b40 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1612,7 +1612,6 @@ fn correct_mutual_recursive_type_alias<'a>( let is_mutually_recursive = cycle.len() > 1; if is_self_recursive || is_mutually_recursive { - let mut opt_rec_var = None; let _made_recursive = make_tag_union_of_alias_recursive( env, rec, @@ -1620,7 +1619,6 @@ fn correct_mutual_recursive_type_alias<'a>( vec![], var_store, &mut can_still_report_error, - &mut opt_rec_var, ); } } @@ -1664,7 +1662,6 @@ fn make_tag_union_of_alias_recursive<'a>( others: Vec, var_store: &mut VarStore, can_report_error: &mut bool, - opt_rec_var: &mut Option, ) -> Result<(), ()> { let alias_args = alias .type_variables @@ -1680,7 +1677,6 @@ fn make_tag_union_of_alias_recursive<'a>( &mut alias.typ, var_store, can_report_error, - opt_rec_var, ) } @@ -1713,7 +1709,6 @@ fn make_tag_union_recursive_help<'a>( typ: &mut Type, var_store: &mut VarStore, can_report_error: &mut bool, - opt_rec_var: &mut Option, ) -> Result<(), ()> { let Loc { value: (symbol, args), @@ -1722,10 +1717,7 @@ fn make_tag_union_recursive_help<'a>( let vars = args.iter().map(|(_, t)| t.clone()).collect::>(); match typ { Type::TagUnion(tags, ext) => { - if opt_rec_var.is_none() { - *opt_rec_var = Some(var_store.fresh()); - } - let rec_var = opt_rec_var.unwrap(); + let rec_var = var_store.fresh(); let mut pending_typ = Type::RecursiveTagUnion(rec_var, tags.to_vec(), ext.clone()); let substitution_result = pending_typ.substitute_alias(symbol, &vars, &Type::Variable(rec_var)); @@ -1758,7 +1750,6 @@ fn make_tag_union_recursive_help<'a>( actual, var_store, can_report_error, - opt_rec_var, ), _ => { mark_cyclic_alias(env, typ, symbol, region, others.clone(), *can_report_error); From 55769caad6f6d731bf05570ef8a4291a4c66585c Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Fri, 11 Feb 2022 08:52:37 -0500 Subject: [PATCH 06/45] Redundant clone --- compiler/can/src/def.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 406d5c3b40..7fe36e1504 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1752,7 +1752,7 @@ fn make_tag_union_recursive_help<'a>( can_report_error, ), _ => { - mark_cyclic_alias(env, typ, symbol, region, others.clone(), *can_report_error); + mark_cyclic_alias(env, typ, symbol, region, others, *can_report_error); *can_report_error = false; Ok(()) From 92e0f8714fa6c1b2bd542c6c7948d0bcc1c05d0b Mon Sep 17 00:00:00 2001 From: Jan Van Bruggen Date: Fri, 11 Feb 2022 15:53:28 -0700 Subject: [PATCH 07/45] Swap List.repeat args order to put the list first --- compiler/builtins/docs/List.roc | 2 +- compiler/builtins/src/std.rs | 4 ++-- compiler/gen_llvm/src/llvm/build.rs | 8 ++++---- compiler/gen_llvm/src/llvm/build_list.rs | 4 ++-- compiler/test_gen/src/gen_list.rs | 16 ++++++++-------- examples/benchmarks/Bytes/Encode.roc | 2 +- examples/cli/echo.roc | 2 +- examples/false-interpreter/Context.roc | 2 +- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/builtins/docs/List.roc b/compiler/builtins/docs/List.roc index 9d6a58bdbe..f900fa760c 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. diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index 15276e9e8b..dc35b4b712 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -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/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 9ca24e1739..41e2057867 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -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, Int -> 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 diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index 169db029c1..cf9d0924bf 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, Int -> 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); diff --git a/compiler/test_gen/src/gen_list.rs b/compiler/test_gen/src/gen_list.rs index 585013cc54..d023aa07c0 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 ); 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..51e476c430 100644 --- a/examples/cli/echo.roc +++ b/examples/cli/echo.roc @@ -18,7 +18,7 @@ echo = \shout -> silence = \length -> spaceInUtf8 = 32 - List.repeat length spaceInUtf8 + List.repeat spaceInUtf8 length shout |> Str.toUtf8 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 ]* From f47dbb51716fcc6241a270343c815b347db31dcd Mon Sep 17 00:00:00 2001 From: Jan Van Bruggen Date: Fri, 11 Feb 2022 15:58:50 -0700 Subject: [PATCH 08/45] Swap List.mapWithIndex arg1 args order to put the element first --- compiler/builtins/docs/List.roc | 2 +- compiler/builtins/src/std.rs | 4 ++-- compiler/can/src/builtins.rs | 2 +- compiler/gen_llvm/src/llvm/build.rs | 2 +- compiler/gen_llvm/src/llvm/build_list.rs | 2 +- compiler/test_gen/src/gen_list.rs | 2 +- examples/cli/echo.roc | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/builtins/docs/List.roc b/compiler/builtins/docs/List.roc index f900fa760c..25ddbc6344 100644 --- a/compiler/builtins/docs/List.roc +++ b/compiler/builtins/docs/List.roc @@ -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 dc35b4b712..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))), ) 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/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 41e2057867..fec8c92e2e 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!(); diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index cf9d0924bf..ace0fa71cc 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -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/test_gen/src/gen_list.rs b/compiler/test_gen/src/gen_list.rs index d023aa07c0..4fbe60537d 100644 --- a/compiler/test_gen/src/gen_list.rs +++ b/compiler/test_gen/src/gen_list.rs @@ -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/examples/cli/echo.roc b/examples/cli/echo.roc index 51e476c430..29cb05c17c 100644 --- a/examples/cli/echo.roc +++ b/examples/cli/echo.roc @@ -23,7 +23,7 @@ echo = \shout -> shout |> Str.toUtf8 |> List.mapWithIndex - (\i, _ -> + (\_, i -> length = (List.len (Str.toUtf8 shout) - i) phrase = (List.split (Str.toUtf8 shout) length).before From e1cc4cbde647014de1f0fbadadfea4f8d108d119 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 11 Feb 2022 21:25:08 -0500 Subject: [PATCH 09/45] Fix comment --- compiler/gen_llvm/src/llvm/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index fec8c92e2e..0d4ce09c3b 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -5484,7 +5484,7 @@ fn run_low_level<'a, 'ctx, 'env>( list_single(env, arg, arg_layout) } ListRepeat => { - // List.repeat : elem, Int -> List elem + // List.repeat : elem, Nat -> List elem debug_assert_eq!(args.len(), 2); let (elem, elem_layout) = load_symbol_and_layout(scope, &args[0]); From 71f2444397c8f654396361d0caed833e8659da9e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 11 Feb 2022 21:26:39 -0500 Subject: [PATCH 10/45] s/Int/Nat in a comment --- compiler/gen_llvm/src/llvm/build_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/gen_llvm/src/llvm/build_list.rs b/compiler/gen_llvm/src/llvm/build_list.rs index ace0fa71cc..968671d1af 100644 --- a/compiler/gen_llvm/src/llvm/build_list.rs +++ b/compiler/gen_llvm/src/llvm/build_list.rs @@ -119,7 +119,7 @@ pub fn list_single<'a, 'ctx, 'env>( ) } -/// List.repeat : elem, Int -> 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>, From 33148cb1a49c6dfa908db8c7470154c7dc3d62c3 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 10:54:57 -0500 Subject: [PATCH 11/45] Functions are compound types --- compiler/types/src/types.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 86a5b9fdc2..87410807f4 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -910,6 +910,9 @@ impl Type { 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!"), From 4dbdf3a58b2134c9a4d0c2feededf05f18687704 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 11:32:32 -0500 Subject: [PATCH 12/45] Check that instantiated lambda set vars indeed only have vars --- compiler/types/src/types.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 87410807f4..e3ae3e2f80 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -816,16 +816,17 @@ 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"); } } From b8defcbc75230a7bcfec5d8849c97c64580e30b3 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 20:03:39 -0500 Subject: [PATCH 13/45] Use freshly-instantiated lambda variables --- compiler/types/src/types.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index e3ae3e2f80..5b39672c45 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -830,9 +830,10 @@ impl Type { } } - 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(); @@ -844,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 { From e946b972f709eb228fe36c700f86fe6f67042a54 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 20:41:19 -0500 Subject: [PATCH 14/45] Only recursive tag unions are relevant in illegal cycles --- compiler/can/src/def.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 7fe36e1504..7e03f8e3b9 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1628,7 +1628,7 @@ fn correct_mutual_recursive_type_alias<'a>( // 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; - typ.is_tag_union_like() && typ.is_narrow() + matches!(typ, Type::RecursiveTagUnion(..)) && typ.is_narrow() }); if all_are_narrow { From 67bb5b14f6adae82ae242435dec9bb3806aa41f6 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 21:08:23 -0500 Subject: [PATCH 15/45] Examples as code block not doc --- compiler/types/src/types.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 5b39672c45..af99cb5303 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -886,16 +886,20 @@ impl Type { /// /// The following are narrow: /// - /// U8 - /// [ A I8 ] - /// [ A [ B [ C U8 ] ] ] - /// [ A (R a) ] as R a + /// ```roc + /// U8 + /// [ A I8 ] + /// [ A [ B [ C U8 ] ] ] + /// [ A (R a) ] as R a + /// ``` /// /// The following are not: /// - /// [ 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) + /// ```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) => { From ac680d07509a0c800d5d9d1f22fe8d35c6266f77 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 12 Feb 2022 21:23:44 -0500 Subject: [PATCH 16/45] Aliases should not be public --- compiler/can/src/scope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/can/src/scope.rs b/compiler/can/src/scope.rs index cfcb1153ff..56dc837441 100644 --- a/compiler/can/src/scope.rs +++ b/compiler/can/src/scope.rs @@ -17,7 +17,7 @@ pub struct Scope { symbols: SendMap, /// The type aliases currently in scope - pub aliases: SendMap, + aliases: SendMap, /// The current module being processed. This will be used to turn /// unqualified idents into Symbols. From c831b994320d1d622ca8affb9856196341badb67 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Feb 2022 14:02:39 -0500 Subject: [PATCH 17/45] Add test for issue #1162 This was fixed some time ago; add a regression test. Closes #1162 --- compiler/test_gen/src/gen_tags.rs | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/compiler/test_gen/src/gen_tags.rs b/compiler/test_gen/src/gen_tags.rs index c19868f4bf..c1643ecc9a 100644 --- a/compiler/test_gen/src/gen_tags.rs +++ b/compiler/test_gen/src/gen_tags.rs @@ -1518,3 +1518,39 @@ fn issue_2458_deep_recursion_var() { 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 + ) +} From 7106e03d4584bb021de50c0466f2d5f5b6661882 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sun, 13 Feb 2022 12:20:22 -0800 Subject: [PATCH 18/45] Stop printing host rebuilding message with --precompiled-host --- cli/src/build.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/src/build.rs b/cli/src/build.rs index 4b0daee242..5f04e48da4 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -309,10 +309,10 @@ fn spawn_rebuild_thread( ) -> std::thread::JoinHandle { let thread_local_target = target.clone(); std::thread::spawn(move || { - print!("🔨 Rebuilding host... "); - let rebuild_host_start = SystemTime::now(); if !precompiled { + print!("🔨 Rebuilding host... "); + if surgically_link { roc_linker::build_and_preprocess_host( opt_level, @@ -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() }) From f328ff5661fcee26e7e2cd7541c0fa942f12df28 Mon Sep 17 00:00:00 2001 From: Jan Van Bruggen Date: Sun, 13 Feb 2022 14:55:34 -0700 Subject: [PATCH 19/45] Remove leading newlines from code files --- compiler/parse/fuzz/.gitignore | 1 - compiler/parse/fuzz/Cargo.toml | 1 - .../parse/tests/snapshots/pass/interface_with_newline.header.roc | 1 - editor/.gitignore | 1 - editor/src/graphics/shaders/shader.wgsl | 1 - 5 files changed, 5 deletions(-) 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/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/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; From 6e5c1d59147758019cdd84d1c7fc2c1330fc8c30 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Feb 2022 20:20:25 -0500 Subject: [PATCH 20/45] Specialize `Num.toFloat` for different target float types Closes #2476 --- compiler/gen_llvm/src/llvm/build.rs | 14 ++++++++--- compiler/test_gen/src/gen_num.rs | 38 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 9ca24e1739..fbc50f5b88 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -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/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 + ) +} From 7b587dd6ae8f8d022bc1adbab171bca034642560 Mon Sep 17 00:00:00 2001 From: Brendan Hansknecht Date: Sun, 13 Feb 2022 17:56:57 -0800 Subject: [PATCH 21/45] Print before timing rebuilding of host --- cli/src/build.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/src/build.rs b/cli/src/build.rs index 5f04e48da4..29e4feab04 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -309,10 +309,12 @@ fn spawn_rebuild_thread( ) -> std::thread::JoinHandle { let thread_local_target = target.clone(); std::thread::spawn(move || { - let rebuild_host_start = SystemTime::now(); if !precompiled { print!("🔨 Rebuilding host... "); + } + let rebuild_host_start = SystemTime::now(); + if !precompiled { if surgically_link { roc_linker::build_and_preprocess_host( opt_level, From 886e4e07f62eeb28516ba6a93a0eb03f834b1d26 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 13 Feb 2022 22:50:27 -0500 Subject: [PATCH 22/45] Update alias analysis, implementation for List.mapWithIndex --- compiler/builtins/bitcode/src/list.zig | 4 +++- compiler/gen_llvm/src/llvm/build.rs | 2 +- compiler/mono/src/alias_analysis.rs | 4 +++- compiler/mono/src/borrow.rs | 8 +++++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/builtins/bitcode/src/list.zig b/compiler/builtins/bitcode/src/list.zig index 35e16c1489..70925554f1 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/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 0d4ce09c3b..c0c75a4500 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -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, 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]), From 45b1cae691f034b855faf8c4d07fa4ec953607e9 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 14 Feb 2022 09:30:21 -0500 Subject: [PATCH 23/45] Add .reuse/dep5 This makes the repo compatible with https://reuse.software/ --- .reuse/dep5 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .reuse/dep5 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 From 154d55985b29f3aa9356c3c55d64021b46ed2b5b Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 14 Feb 2022 18:16:35 +0100 Subject: [PATCH 24/45] move depencency tracking out of file.rs --- compiler/load/src/file.rs | 287 ++----------------------------------- compiler/load/src/lib.rs | 1 + compiler/load/src/work.rs | 290 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 276 deletions(-) create mode 100644 compiler/load/src/work.rs diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 13802dbcdf..686f5bde3c 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -45,6 +45,8 @@ use std::str::from_utf8_unchecked; use std::sync::Arc; use std::{env, fs}; +use crate::work::{Dependencies, Phase}; + #[cfg(not(target_family = "wasm"))] use std::time::{Duration, SystemTime}; #[cfg(target_family = "wasm")] @@ -71,253 +73,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 +106,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 = { 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/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)>), +} From 957140df64594e6e488e61524c5b3008de4adf08 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 14 Feb 2022 20:32:31 +0100 Subject: [PATCH 25/45] remove builtin lookup function being passed around --- ast/src/module.rs | 1 - cli/src/build.rs | 3 -- compiler/can/src/module.rs | 10 ++----- compiler/load/src/file.rs | 56 +++++++++----------------------------- docs/src/lib.rs | 2 -- repl_eval/src/gen.rs | 2 -- 6 files changed, 16 insertions(+), 58 deletions(-) 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 29e4feab04..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; @@ -376,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/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/load/src/file.rs b/compiler/load/src/file.rs index 686f5bde3c..6d338bc1c3 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::{ @@ -765,18 +765,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)?; @@ -789,7 +785,6 @@ where exposed_types, Phase::SolveTypes, target_info, - look_up_builtin, )? { Monomorphized(_) => unreachable!(""), TypeChecked(module) => Ok(module), @@ -797,18 +792,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)?; @@ -821,7 +812,6 @@ where exposed_types, Phase::MakeSpecializations, target_info, - look_up_builtin, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -829,7 +819,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, @@ -837,11 +827,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)?; @@ -854,7 +840,6 @@ where exposed_types, Phase::MakeSpecializations, target_info, - look_up_builtin, )? { Monomorphized(module) => Ok(module), TypeChecked(_) => unreachable!(""), @@ -1001,7 +986,7 @@ 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>, @@ -1010,11 +995,7 @@ fn load<'a, F>( 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>> { let LoadStart { arc_modules, ident_ids_by_module, @@ -1126,7 +1107,6 @@ where src_dir, msg_tx.clone(), target_info, - look_up_builtins, ); match result { @@ -2992,18 +2972,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 { @@ -3031,7 +3007,6 @@ where exposed_imports, &exposed_symbols, &mut var_store, - look_up_builtins, ); let canonicalize_end = SystemTime::now(); @@ -3512,17 +3487,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 { @@ -3554,7 +3525,6 @@ where exposed_symbols, aliases, parsed, - look_up_builtins, ), Solve { module, 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/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 { From 04adbe75ca71a811a1c71055d7deec2b8142ddbb Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 14 Feb 2022 21:09:51 +0100 Subject: [PATCH 26/45] fix test compilation --- cli_utils/Cargo.lock | 35 +------ compiler/load/src/file.rs | 132 ++++++++++++++------------ compiler/load/tests/test_load.rs | 4 - compiler/solve/tests/solve_expr.rs | 2 - compiler/test_gen/src/helpers/dev.rs | 2 - compiler/test_gen/src/helpers/llvm.rs | 5 - compiler/test_gen/src/helpers/wasm.rs | 2 - compiler/test_mono/src/tests.rs | 2 - 8 files changed, 77 insertions(+), 107 deletions(-) diff --git a/cli_utils/Cargo.lock b/cli_utils/Cargo.lock index f6cf4cf5a9..514966d291 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" @@ -2799,6 +2780,7 @@ dependencies = [ "bumpalo", "lazy_static", "roc_collections", + "roc_error_macros", "roc_ident", "roc_region", "snafu", @@ -2815,6 +2797,7 @@ dependencies = [ "roc_builtins", "roc_can", "roc_collections", + "roc_error_macros", "roc_module", "roc_problem", "roc_region", @@ -2865,6 +2848,7 @@ dependencies = [ "inkwell 0.1.0", "libloading 0.7.1", "roc_build", + "roc_builtins", "roc_collections", "roc_gen_llvm", "roc_load", @@ -2946,6 +2930,7 @@ version = "0.1.0" dependencies = [ "bumpalo", "roc_collections", + "roc_error_macros", "roc_module", "roc_region", "static_assertions", @@ -2956,6 +2941,7 @@ dependencies = [ name = "roc_unify" version = "0.1.0" dependencies = [ + "bitflags", "roc_collections", "roc_module", "roc_types", @@ -3402,17 +3388,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" diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 6d338bc1c3..bb38213777 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1074,71 +1074,24 @@ fn load<'a>( // (since other threads need to reference it too). let injector = &injector; + + // Record this thread's handle so the main thread can join it later. let res_join_handle = thread_scope .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, - ); - - 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(); @@ -1302,6 +1255,65 @@ fn load<'a>( .unwrap() } +#[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); + } + } + } + } + } + } + + Ok(()) +} + fn start_tasks<'a>( arena: &'a Bump, state: &mut State<'a>, 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/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index d41886aea0..ef03d2a000 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()?; 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 { From e56a5695ba84c6c504d7ffedac3ee1b630a8409f Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 14 Feb 2022 21:50:09 +0100 Subject: [PATCH 27/45] initial PoC --- compiler/load/src/file.rs | 254 +++++++++++++++++++++++++++++++++++++- 1 file changed, 253 insertions(+), 1 deletion(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index bb38213777..6c84a94d0a 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -40,6 +40,7 @@ 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; @@ -64,7 +65,7 @@ const PKG_CONFIG_FILE_NAME: &str = "Package-Config"; /// The . in between module names like Foo.Bar.Baz const MODULE_SEPARATOR: char = '.'; -const SHOW_MESSAGE_LOG: bool = false; +const SHOW_MESSAGE_LOG: bool = true; const EXPANDED_STACK_SIZE: usize = 8 * 1024 * 1024; @@ -1005,6 +1006,192 @@ fn load<'a>( 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 { + 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'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_lifo(); + let stealer = worker.stealer(); + let stealers = &[stealer]; + + 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), + } + + match worker_task_step( + arena, + &worker, + &injector, + stealers, + &worker_msg_rx, + &msg_tx, + src_dir, + target_info, + ) { + 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()); + + return Ok(ControlFlow::Break(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()); + + return Ok(ControlFlow::Break(LoadResult::Monomorphized( + finish_specialization(state, subs, exposed_to_host)?, + ))); + } + Msg::FailedToReadFile { filename, error } => { + let buf = to_file_problem_report(&filename, error); + return 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); + 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) => 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); + return Err(LoadingProblem::FormattedReport(buf)); + } + Err(e) => return Err(e), + } + } + } + } + Err(err) => match err { + crossbeam::channel::TryRecvError::Empty => Ok(ControlFlow::Continue(state)), + crossbeam::channel::TryRecvError::Disconnected => panic!(""), + }, + } +} + +#[allow(clippy::too_many_arguments)] +fn load_multithreaded<'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, +) -> Result, LoadingProblem<'a>> { + let LoadStart { + arc_modules, + ident_ids_by_module, + root_id, + 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) @@ -1255,6 +1442,71 @@ fn load<'a>( .unwrap() } +#[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! + return 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); + + 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); + } + } + } + + 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, From 3e511acbccf9b0c1aad10e1896cc1e8439d4551c Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Mon, 14 Feb 2022 21:10:45 +0000 Subject: [PATCH 28/45] Fix Wasm compile errors --- compiler/load/src/file.rs | 4 ++-- compiler/load/src/wasm_system_time.rs | 8 ++++---- compiler/types/src/subs.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 6c84a94d0a..0e515ec254 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -51,7 +51,7 @@ use crate::work::{Dependencies, Phase}; #[cfg(not(target_family = "wasm"))] use std::time::{Duration, SystemTime}; #[cfg(target_family = "wasm")] -use wasm_system_time::{Duration, SystemTime}; +use crate::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"; @@ -650,7 +650,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)? 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/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 { From e95d32e821c005ac09028665c8e38f190ad86fa4 Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Mon, 14 Feb 2022 22:24:51 +0000 Subject: [PATCH 29/45] DEBUG HACKS, DO NOT MERGE --- Cargo.lock | 3 ++ compiler/load/Cargo.toml | 1 + compiler/load/src/file.rs | 58 ++++++++++++++++++++++++++++++++---- compiler/solve/Cargo.toml | 1 + compiler/solve/src/module.rs | 19 ++++++++++++ compiler/solve/src/solve.rs | 18 +++++++++++ repl_eval/Cargo.toml | 1 + repl_eval/src/gen.rs | 18 +++++++++++ repl_wasm/src/lib.rs | 4 ++- repl_www/build.sh | 4 ++- 10 files changed, 120 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1414f20e6f..f7b31922f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3584,6 +3584,7 @@ dependencies = [ "roc_unify", "tempfile", "ven_pretty", + "wasm-bindgen", ] [[package]] @@ -3698,6 +3699,7 @@ dependencies = [ "roc_reporting", "roc_target", "roc_types", + "wasm-bindgen", ] [[package]] @@ -3763,6 +3765,7 @@ dependencies = [ "roc_types", "roc_unify", "tempfile", + "wasm-bindgen", ] [[package]] diff --git a/compiler/load/Cargo.toml b/compiler/load/Cargo.toml index 57f4ffd530..81d84b7890 100644 --- a/compiler/load/Cargo.toml +++ b/compiler/load/Cargo.toml @@ -26,6 +26,7 @@ bumpalo = { version = "3.8.0", features = ["collections"] } parking_lot = { version = "0.11.2" } crossbeam = "0.8.1" num_cpus = "1.13.0" +wasm-bindgen = "0.2.79" [dev-dependencies] tempfile = "3.2.0" diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 0e515ec254..8fa886f43a 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -45,13 +45,26 @@ use std::path::{Path, PathBuf}; use std::str::from_utf8_unchecked; use std::sync::Arc; use std::{env, fs}; +use wasm_bindgen::prelude::wasm_bindgen; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = console)] + fn log(s: &str); +} + +// In-browser debugging +#[allow(unused_macros)] +macro_rules! console_log { + ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) +} use crate::work::{Dependencies, Phase}; -#[cfg(not(target_family = "wasm"))] -use std::time::{Duration, SystemTime}; #[cfg(target_family = "wasm")] use crate::wasm_system_time::{Duration, SystemTime}; +#[cfg(not(target_family = "wasm"))] +use std::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"; @@ -718,6 +731,7 @@ enum BuildTask<'a> { }, } +#[derive(Debug)] enum WorkerMsg { Shutdown, TaskAdded, @@ -831,6 +845,8 @@ pub fn load_and_monomorphize_from_str<'a>( ) -> Result, LoadingProblem<'a>> { use LoadResult::*; + console_log!("load_and_monomorphize_from_str"); + let load_start = LoadStart::from_str(arena, filename, src)?; match load( @@ -997,6 +1013,8 @@ fn load<'a>( goal_phase: Phase, target_info: TargetInfo, ) -> Result, LoadingProblem<'a>> { + console_log!("load start"); + let LoadStart { arc_modules, ident_ids_by_module, @@ -1008,10 +1026,14 @@ fn load<'a>( let (msg_tx, msg_rx) = bounded(1024); + console_log!("before msg_tx.send"); + msg_tx .send(root_msg) .map_err(|_| LoadingProblem::MsgChannelDied)?; + console_log!("after msg_tx.send"); + let mut state = State { root_id, target_info, @@ -1038,15 +1060,21 @@ fn load<'a>( // We'll add tasks to this, and then worker threads will take tasks from it. let injector = Injector::new(); + console_log!("after injector"); + let (worker_msg_tx, worker_msg_rx) = bounded(1024); let worker_listener = worker_msg_tx; let worker_listeners = arena.alloc([worker_listener]); + console_log!("before Worker lifo"); + let worker = Worker::new_lifo(); let stealer = worker.stealer(); let stealers = &[stealer]; + console_log!("before loop"); loop { + console_log!("inside 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)) => { @@ -1054,8 +1082,9 @@ fn load<'a>( } Err(e) => return Err(e), } + console_log!("after match"); - match worker_task_step( + let control_flow = worker_task_step( arena, &worker, &injector, @@ -1064,7 +1093,11 @@ fn load<'a>( &msg_tx, src_dir, target_info, - ) { + ); + + console_log!("control flow {:?}", control_flow); + + match control_flow { Ok(ControlFlow::Break(())) => panic!("the worker should not break!"), Ok(ControlFlow::Continue(())) => { // progress was made @@ -1453,7 +1486,10 @@ fn worker_task_step<'a>( src_dir: &Path, target_info: TargetInfo, ) -> Result, LoadingProblem<'a>> { - match worker_msg_rx.try_recv() { + console_log!("worker_task_step"); + let recv = worker_msg_rx.try_recv(); + console_log!("recv {:?}", &recv); + match recv { Ok(msg) => { match msg { WorkerMsg::Shutdown => { @@ -1474,8 +1510,10 @@ fn worker_task_step<'a>( // added. In that case, do nothing, and keep waiting // until we receive a Shutdown message. if let Some(task) = find_task(&worker, injector, stealers) { + console_log!("found Some task {:?}", task); let result = run_task(task, worker_arena, src_dir, msg_tx.clone(), target_info); + console_log!("run_task result {:?}", &result); match result { Ok(()) => {} @@ -1495,6 +1533,7 @@ fn worker_task_step<'a>( } } } + console_log!("after if let Some(task)"); Ok(ControlFlow::Continue(())) } @@ -3129,12 +3168,15 @@ fn run_solve<'a>( dep_idents: MutMap, unused_imports: MutMap, ) -> Msg<'a> { + + console_log!("run_solve"); // We have more constraining work to do now, so we'll add it to our timings. let constrain_start = SystemTime::now(); // Finish constraining the module by wrapping the existing Constraint // in the ones we just computed. We can do this off the main thread. let constraint = constrain_imports(imported_symbols, constraint, &mut var_store); + console_log!("after constrain_imports"); let constrain_end = SystemTime::now(); @@ -3154,12 +3196,16 @@ fn run_solve<'a>( let (solved_subs, solved_env, problems) = roc_solve::module::run_solve(aliases, rigid_variables, constraint, var_store); + console_log!("after run_solve"); + let mut exposed_vars_by_symbol: MutMap = solved_env.vars_by_symbol.clone(); exposed_vars_by_symbol.retain(|k, _| exposed_symbols.contains(k)); let solved_types = roc_solve::module::make_solved_types(&solved_env, &solved_subs, &exposed_vars_by_symbol); + console_log!("after make_solved_types"); + let solved_module = SolvedModule { exposed_vars_by_symbol, exposed_symbols: exposed_symbols.into_iter().collect::>(), @@ -3175,6 +3221,8 @@ fn run_solve<'a>( module_timing.constrain += constrain_elapsed; module_timing.solve = solve_end.duration_since(constrain_end).unwrap(); + console_log!("creating Msg::SolvedTypes"); + // Send the subs to the main thread for processing, Msg::SolvedTypes { module_id, diff --git a/compiler/solve/Cargo.toml b/compiler/solve/Cargo.toml index ebb9e4a615..7c27ea42af 100644 --- a/compiler/solve/Cargo.toml +++ b/compiler/solve/Cargo.toml @@ -14,6 +14,7 @@ roc_can = { path = "../can" } roc_unify = { path = "../unify" } arrayvec = "0.7.2" bumpalo = { version = "3.8.0", features = ["collections"] } +wasm-bindgen = "0.2.79" [dev-dependencies] roc_load = { path = "../load" } diff --git a/compiler/solve/src/module.rs b/compiler/solve/src/module.rs index 920cbbdd58..36922e5295 100644 --- a/compiler/solve/src/module.rs +++ b/compiler/solve/src/module.rs @@ -7,6 +7,20 @@ use roc_types::solved_types::{Solved, SolvedType}; use roc_types::subs::{Subs, VarStore, Variable}; use roc_types::types::Alias; +use wasm_bindgen::prelude::wasm_bindgen; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = console)] + fn log(s: &str); +} + +// In-browser debugging +#[allow(unused_macros)] +macro_rules! console_log { + ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) +} + #[derive(Debug)] pub struct SolvedModule { pub solved_types: MutMap, @@ -22,16 +36,20 @@ pub fn run_solve( constraint: Constraint, var_store: VarStore, ) -> (Solved, solve::Env, Vec) { + console_log!("run_solve"); + let env = solve::Env { vars_by_symbol: MutMap::default(), aliases, }; let mut subs = Subs::new_from_varstore(var_store); + console_log!("after new_from_varstore"); for (var, name) in rigid_variables { subs.rigid_var(var, name); } + console_log!("after rigid_var"); // Now that the module is parsed, canonicalized, and constrained, // we need to type check it. @@ -39,6 +57,7 @@ pub fn run_solve( // Run the solver to populate Subs. let (solved_subs, solved_env) = solve::run(&env, &mut problems, subs, &constraint); + console_log!("after solve::run"); (solved_subs, solved_env, problems) } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 8c2baee3e5..fa3632c2c1 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -15,6 +15,22 @@ use roc_types::types::{gather_fields_unsorted_iter, Alias, Category, ErrorType, use roc_unify::unify::{unify, Mode, Unified::*}; use std::collections::hash_map::Entry; + +use wasm_bindgen::prelude::wasm_bindgen; + +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = console)] + fn log(s: &str); +} + +// In-browser debugging +#[allow(unused_macros)] +macro_rules! console_log { + ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) +} + + // Type checking system adapted from Elm by Evan Czaplicki, BSD-3-Clause Licensed // https://github.com/elm/compiler // Thank you, Evan! @@ -155,6 +171,7 @@ pub fn run_in_place( subs: &mut Subs, constraint: &Constraint, ) -> Env { + console_log!("run_in_place"); let mut pools = Pools::default(); let state = State { env: env.clone(), @@ -186,6 +203,7 @@ fn solve( subs: &mut Subs, constraint: &Constraint, ) -> State { + console_log!("solve constraint {:?}", constraint); match constraint { True => state, SaveTheEnvironment => { diff --git a/repl_eval/Cargo.toml b/repl_eval/Cargo.toml index 562cb27c9e..ca81ab5994 100644 --- a/repl_eval/Cargo.toml +++ b/repl_eval/Cargo.toml @@ -7,6 +7,7 @@ version = "0.1.0" [dependencies] bumpalo = {version = "3.8.0", features = ["collections"]} +wasm-bindgen = "0.2.79" roc_builtins = {path = "../compiler/builtins"} roc_can = {path = "../compiler/can"} diff --git a/repl_eval/src/gen.rs b/repl_eval/src/gen.rs index a11ba966b8..453d9cc6df 100644 --- a/repl_eval/src/gen.rs +++ b/repl_eval/src/gen.rs @@ -8,9 +8,22 @@ use roc_load::file::{LoadingProblem, MonomorphizedModule}; use roc_parse::ast::Expr; use roc_region::all::LineInfo; use roc_target::TargetInfo; +use wasm_bindgen::prelude::wasm_bindgen; use crate::eval::ToAstProblem; +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(js_namespace = console)] + fn log(s: &str); +} + +// In-browser debugging +#[allow(unused_macros)] +macro_rules! console_log { + ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) +} + pub enum ReplOutput { Problems(Vec), NoProblems { expr: String, expr_type: String }, @@ -45,6 +58,8 @@ pub fn compile_to_mono<'a>( src: &str, target_info: TargetInfo, ) -> Result, Vec> { + console_log!("compile_to_mono"); + use roc_reporting::report::{ can_problem, mono_problem, type_problem, RocDocAllocator, DEFAULT_PALETTE, }; @@ -56,6 +71,9 @@ pub fn compile_to_mono<'a>( let module_src = arena.alloc(promote_expr_to_module(src)); let exposed_types = MutMap::default(); + + console_log!("before load_and_monomorphize_from_str"); + let loaded = roc_load::file::load_and_monomorphize_from_str( arena, filename, diff --git a/repl_wasm/src/lib.rs b/repl_wasm/src/lib.rs index 48910defba..037572ba7f 100644 --- a/repl_wasm/src/lib.rs +++ b/repl_wasm/src/lib.rs @@ -31,7 +31,7 @@ extern "C" { fn js_get_result_and_memory(buffer_alloc_addr: *mut u8) -> usize; #[wasm_bindgen(js_namespace = console)] - fn log(s: &str); + pub fn log(s: &str); } // In-browser debugging @@ -158,8 +158,10 @@ impl<'a> ReplApp<'a> for WasmReplApp<'a> { #[wasm_bindgen] pub async fn entrypoint_from_js(src: String) -> Result { + console_log!("entrypoint_from_js"); let arena = &Bump::new(); let pre_linked_binary: &'static [u8] = include_bytes!("../data/pre_linked_binary.o"); + console_log!("pre_linked_binary {}", pre_linked_binary.len()); // Compile the app let target_info = TargetInfo::default_wasm32(); diff --git a/repl_www/build.sh b/repl_www/build.sh index 2b9c6e4023..f8ee4c63a0 100755 --- a/repl_www/build.sh +++ b/repl_www/build.sh @@ -17,7 +17,9 @@ 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) +# 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!) wasm-pack build --target web "$@" repl_wasm cp repl_wasm/pkg/*.wasm $WWW_DIR From 23e498ca3b22e27bda1e98cc83a21171b2ca1c15 Mon Sep 17 00:00:00 2001 From: Brian Carroll Date: Mon, 14 Feb 2022 23:05:33 +0000 Subject: [PATCH 30/45] repl_www: faster build script, without wasm-opt --- repl_www/build.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/repl_www/build.sh b/repl_www/build.sh index f8ee4c63a0..549dbfaf1c 100755 --- a/repl_www/build.sh +++ b/repl_www/build.sh @@ -20,7 +20,9 @@ cp repl_www/public/* $WWW_DIR # 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!) -wasm-pack build --target web "$@" repl_wasm +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 From 180be067729261e5c6ab0001bcb10aa41751b600 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 15 Feb 2022 00:29:48 +0100 Subject: [PATCH 31/45] check for empty string on IdentStr drop impl --- compiler/ident/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); From 56375ef69d9270aabec13ae69994bdf53faecc45 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 15 Feb 2022 00:29:48 +0100 Subject: [PATCH 32/45] check for empty string on IdentStr drop impl --- compiler/ident/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); From 5c0849ea82486576cb6e53141953d807ea25b3a1 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 15 Feb 2022 22:58:29 +0100 Subject: [PATCH 33/45] fix referencing uninitialized memory --- roc_std/src/lib.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) 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 From c241d3465518fe72da758ed8d92159de104405f4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 15 Feb 2022 18:15:50 -0500 Subject: [PATCH 34/45] Add Tom Dohrmann to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) 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 From f440d53e7bebd5ce6acf3812d8846a03c071c461 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Tue, 15 Feb 2022 20:48:04 -0800 Subject: [PATCH 35/45] Refactor: introduce trait SpaceProblem to remove a bunch of redundant args in ::Space errors --- compiler/parse/src/blankspace.rs | 38 ++++++--------- compiler/parse/src/expr.rs | 63 +++++++------------------ compiler/parse/src/header.rs | 6 +-- compiler/parse/src/module.rs | 46 +++++------------- compiler/parse/src/parser.rs | 54 +++++++++++++++++++-- compiler/parse/src/pattern.rs | 12 ++--- compiler/parse/src/type_annotation.rs | 68 +++++++++------------------ 7 files changed, 117 insertions(+), 170 deletions(-) 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") From 4d0b7557f6da54fbfa485d3308076311c6d60bf6 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Wed, 16 Feb 2022 11:16:33 +0100 Subject: [PATCH 36/45] added NoRedInk sponsor to README --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) 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. From 45217b8074b4a949e409d7e64c5914e5020dab81 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 14:09:23 +0100 Subject: [PATCH 37/45] pick the single-threaded load when target-family=wasm --- cli_utils/Cargo.lock | 23 ++--- compiler/load/src/file.rs | 159 +++++++++++++++++------------------ compiler/solve/src/module.rs | 19 ----- compiler/solve/src/solve.rs | 18 ---- 4 files changed, 91 insertions(+), 128 deletions(-) diff --git a/cli_utils/Cargo.lock b/cli_utils/Cargo.lock index 514966d291..c25e9072c9 100644 --- a/cli_utils/Cargo.lock +++ b/cli_utils/Cargo.lock @@ -2771,6 +2771,7 @@ dependencies = [ "roc_types", "roc_unify", "ven_pretty", + "wasm-bindgen", ] [[package]] @@ -2879,6 +2880,7 @@ dependencies = [ "roc_reporting", "roc_target", "roc_types", + "wasm-bindgen", ] [[package]] @@ -2911,6 +2913,7 @@ dependencies = [ "roc_region", "roc_types", "roc_unify", + "wasm-bindgen", ] [[package]] @@ -3558,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", @@ -3568,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", @@ -3595,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", @@ -3605,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", @@ -3618,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/load/src/file.rs b/compiler/load/src/file.rs index 8fa886f43a..2b03eb90d4 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -45,19 +45,6 @@ use std::path::{Path, PathBuf}; use std::str::from_utf8_unchecked; use std::sync::Arc; use std::{env, fs}; -use wasm_bindgen::prelude::wasm_bindgen; - -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(js_namespace = console)] - fn log(s: &str); -} - -// In-browser debugging -#[allow(unused_macros)] -macro_rules! console_log { - ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) -} use crate::work::{Dependencies, Phase}; @@ -611,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, @@ -845,8 +869,6 @@ pub fn load_and_monomorphize_from_str<'a>( ) -> Result, LoadingProblem<'a>> { use LoadResult::*; - console_log!("load_and_monomorphize_from_str"); - let load_start = LoadStart::from_str(arena, filename, src)?; match load( @@ -1005,7 +1027,6 @@ enum LoadResult<'a> { #[allow(clippy::too_many_arguments)] fn load<'a>( arena: &'a Bump, - //filename: PathBuf, load_start: LoadStart<'a>, stdlib: &'a StdLib, src_dir: &Path, @@ -1013,8 +1034,42 @@ fn load<'a>( goal_phase: Phase, target_info: TargetInfo, ) -> Result, LoadingProblem<'a>> { - console_log!("load start"); + // 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, @@ -1022,59 +1077,34 @@ fn load<'a>( root_msg, } = load_start; - let arc_shorthands = Arc::new(Mutex::new(MutMap::default())); - let (msg_tx, msg_rx) = bounded(1024); - console_log!("before msg_tx.send"); - msg_tx .send(root_msg) .map_err(|_| LoadingProblem::MsgChannelDied)?; - console_log!("after msg_tx.send"); - - let mut state = State { + let mut state = State::new( 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'll add tasks to this, and then worker threads will take tasks from it. let injector = Injector::new(); - console_log!("after injector"); - let (worker_msg_tx, worker_msg_rx) = bounded(1024); let worker_listener = worker_msg_tx; let worker_listeners = arena.alloc([worker_listener]); - console_log!("before Worker lifo"); - let worker = Worker::new_lifo(); let stealer = worker.stealer(); let stealers = &[stealer]; - console_log!("before loop"); loop { - console_log!("inside 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)) => { @@ -1082,7 +1112,6 @@ fn load<'a>( } Err(e) => return Err(e), } - console_log!("after match"); let control_flow = worker_task_step( arena, @@ -1095,8 +1124,6 @@ fn load<'a>( target_info, ); - console_log!("control flow {:?}", control_flow); - match control_flow { Ok(ControlFlow::Break(())) => panic!("the worker should not break!"), Ok(ControlFlow::Continue(())) => { @@ -1193,7 +1220,7 @@ fn state_thread_step<'a>( to_parse_problem_report(problem, module_ids, constrained_ident_ids); return Err(LoadingProblem::FormattedReport(buf)); } - Err(e) => return Err(e), + Err(e) => Err(e), } } } @@ -1206,9 +1233,8 @@ fn state_thread_step<'a>( } #[allow(clippy::too_many_arguments)] -fn load_multithreaded<'a>( +fn load_multi_threaded<'a>( arena: &'a Bump, - //filename: PathBuf, load_start: LoadStart<'a>, stdlib: &'a StdLib, src_dir: &Path, @@ -1223,8 +1249,6 @@ fn load_multithreaded<'a>( 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) @@ -1317,28 +1341,15 @@ fn load_multithreaded<'a>( res_join_handle.unwrap(); } - let mut state = State { + let mut state = State::new( 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! @@ -1486,9 +1497,7 @@ fn worker_task_step<'a>( src_dir: &Path, target_info: TargetInfo, ) -> Result, LoadingProblem<'a>> { - console_log!("worker_task_step"); let recv = worker_msg_rx.try_recv(); - console_log!("recv {:?}", &recv); match recv { Ok(msg) => { match msg { @@ -1510,10 +1519,8 @@ fn worker_task_step<'a>( // added. In that case, do nothing, and keep waiting // until we receive a Shutdown message. if let Some(task) = find_task(&worker, injector, stealers) { - console_log!("found Some task {:?}", task); let result = run_task(task, worker_arena, src_dir, msg_tx.clone(), target_info); - console_log!("run_task result {:?}", &result); match result { Ok(()) => {} @@ -1533,7 +1540,6 @@ fn worker_task_step<'a>( } } } - console_log!("after if let Some(task)"); Ok(ControlFlow::Continue(())) } @@ -3168,15 +3174,12 @@ fn run_solve<'a>( dep_idents: MutMap, unused_imports: MutMap, ) -> Msg<'a> { - - console_log!("run_solve"); // We have more constraining work to do now, so we'll add it to our timings. let constrain_start = SystemTime::now(); // Finish constraining the module by wrapping the existing Constraint // in the ones we just computed. We can do this off the main thread. let constraint = constrain_imports(imported_symbols, constraint, &mut var_store); - console_log!("after constrain_imports"); let constrain_end = SystemTime::now(); @@ -3196,16 +3199,12 @@ fn run_solve<'a>( let (solved_subs, solved_env, problems) = roc_solve::module::run_solve(aliases, rigid_variables, constraint, var_store); - console_log!("after run_solve"); - let mut exposed_vars_by_symbol: MutMap = solved_env.vars_by_symbol.clone(); exposed_vars_by_symbol.retain(|k, _| exposed_symbols.contains(k)); let solved_types = roc_solve::module::make_solved_types(&solved_env, &solved_subs, &exposed_vars_by_symbol); - console_log!("after make_solved_types"); - let solved_module = SolvedModule { exposed_vars_by_symbol, exposed_symbols: exposed_symbols.into_iter().collect::>(), @@ -3221,8 +3220,6 @@ fn run_solve<'a>( module_timing.constrain += constrain_elapsed; module_timing.solve = solve_end.duration_since(constrain_end).unwrap(); - console_log!("creating Msg::SolvedTypes"); - // Send the subs to the main thread for processing, Msg::SolvedTypes { module_id, diff --git a/compiler/solve/src/module.rs b/compiler/solve/src/module.rs index 36922e5295..920cbbdd58 100644 --- a/compiler/solve/src/module.rs +++ b/compiler/solve/src/module.rs @@ -7,20 +7,6 @@ use roc_types::solved_types::{Solved, SolvedType}; use roc_types::subs::{Subs, VarStore, Variable}; use roc_types::types::Alias; -use wasm_bindgen::prelude::wasm_bindgen; - -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(js_namespace = console)] - fn log(s: &str); -} - -// In-browser debugging -#[allow(unused_macros)] -macro_rules! console_log { - ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) -} - #[derive(Debug)] pub struct SolvedModule { pub solved_types: MutMap, @@ -36,20 +22,16 @@ pub fn run_solve( constraint: Constraint, var_store: VarStore, ) -> (Solved, solve::Env, Vec) { - console_log!("run_solve"); - let env = solve::Env { vars_by_symbol: MutMap::default(), aliases, }; let mut subs = Subs::new_from_varstore(var_store); - console_log!("after new_from_varstore"); for (var, name) in rigid_variables { subs.rigid_var(var, name); } - console_log!("after rigid_var"); // Now that the module is parsed, canonicalized, and constrained, // we need to type check it. @@ -57,7 +39,6 @@ pub fn run_solve( // Run the solver to populate Subs. let (solved_subs, solved_env) = solve::run(&env, &mut problems, subs, &constraint); - console_log!("after solve::run"); (solved_subs, solved_env, problems) } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index fa3632c2c1..8c2baee3e5 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -15,22 +15,6 @@ use roc_types::types::{gather_fields_unsorted_iter, Alias, Category, ErrorType, use roc_unify::unify::{unify, Mode, Unified::*}; use std::collections::hash_map::Entry; - -use wasm_bindgen::prelude::wasm_bindgen; - -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(js_namespace = console)] - fn log(s: &str); -} - -// In-browser debugging -#[allow(unused_macros)] -macro_rules! console_log { - ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) -} - - // Type checking system adapted from Elm by Evan Czaplicki, BSD-3-Clause Licensed // https://github.com/elm/compiler // Thank you, Evan! @@ -171,7 +155,6 @@ pub fn run_in_place( subs: &mut Subs, constraint: &Constraint, ) -> Env { - console_log!("run_in_place"); let mut pools = Pools::default(); let state = State { env: env.clone(), @@ -203,7 +186,6 @@ fn solve( subs: &mut Subs, constraint: &Constraint, ) -> State { - console_log!("solve constraint {:?}", constraint); match constraint { True => state, SaveTheEnvironment => { From 599a0e5dc72dc487cfeee9f8cb0b8597daf8c126 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 14:18:28 +0100 Subject: [PATCH 38/45] move things out of thread scope --- compiler/load/src/file.rs | 40 ++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 2b03eb90d4..6fbaa47a8e 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1290,18 +1290,29 @@ fn load_multi_threaded<'a>( let it = worker_arenas.iter_mut(); + let mut state = State::new( + root_id, + target_info, + goal_phase, + stdlib, + exposed_types, + arc_modules, + ident_ids_by_module, + ); + + 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(); + { 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); @@ -1341,15 +1352,6 @@ fn load_multi_threaded<'a>( res_join_handle.unwrap(); } - let mut state = State::new( - root_id, - target_info, - goal_phase, - stdlib, - exposed_types, - arc_modules, - ident_ids_by_module, - ); // We've now distributed one worker queue to each thread. // There should be no queues left to distribute! From 400598a01352ee98e78853b6358e38598e88c974 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 14:35:07 +0100 Subject: [PATCH 39/45] cleanup --- compiler/load/src/file.rs | 44 ++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 6fbaa47a8e..0d72eb6272 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1100,7 +1100,7 @@ fn load_single_threaded<'a>( let worker_listener = worker_msg_tx; let worker_listeners = arena.alloc([worker_listener]); - let worker = Worker::new_lifo(); + let worker = Worker::new_fifo(); let stealer = worker.stealer(); let stealers = &[stealer]; @@ -1249,6 +1249,16 @@ fn load_multi_threaded<'a>( 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 .send(root_msg) @@ -1269,14 +1279,9 @@ fn load_multi_threaded<'a>( 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(); @@ -1288,20 +1293,8 @@ fn load_multi_threaded<'a>( 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(); - - let mut state = State::new( - root_id, - target_info, - goal_phase, - stdlib, - exposed_types, - arc_modules, - ident_ids_by_module, - ); - for _ in 0..num_workers { - let worker = Worker::new_lifo(); + let worker = Worker::new_fifo(); stealers.push(worker.stealer()); worker_queues.push(worker); @@ -1311,6 +1304,7 @@ fn load_multi_threaded<'a>( // 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| { @@ -1320,8 +1314,8 @@ fn load_multi_threaded<'a>( 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 @@ -1329,8 +1323,6 @@ fn load_multi_threaded<'a>( // (since other threads need to reference it too). let injector = &injector; - - // Record this thread's handle so the main thread can join it later. let res_join_handle = thread_scope .builder() @@ -1508,7 +1500,7 @@ fn worker_task_step<'a>( // shut down the thread, so when the main thread // blocks on joining with all the worker threads, // it can finally exit too! - return Ok(ControlFlow::Break(())); + Ok(ControlFlow::Break(())) } WorkerMsg::TaskAdded => { // Find a task - either from this thread's queue, From ff260692954d82e132d477bacecd28adf28c2507 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 14:49:00 +0100 Subject: [PATCH 40/45] use single-threaded stepper in multithreaded file.rs --- compiler/load/src/file.rs | 117 +++++--------------------------------- 1 file changed, 13 insertions(+), 104 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 0d72eb6272..8262d466a6 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -65,7 +65,7 @@ const PKG_CONFIG_FILE_NAME: &str = "Package-Config"; /// The . in between module names like Foo.Bar.Baz const MODULE_SEPARATOR: char = '.'; -const SHOW_MESSAGE_LOG: bool = true; +const SHOW_MESSAGE_LOG: bool = false; const EXPANDED_STACK_SIZE: usize = 8 * 1024 * 1024; @@ -1307,7 +1307,6 @@ fn load_multi_threaded<'a>( let it = worker_arenas.iter_mut(); { thread::scope(|thread_scope| { - let mut worker_listeners = bumpalo::collections::Vec::with_capacity_in(num_workers, arena); @@ -1344,7 +1343,6 @@ fn load_multi_threaded<'a>( res_join_handle.unwrap(); } - // We've now distributed one worker queue to each thread. // There should be no queues left to distribute! debug_assert!(worker_queues.is_empty()); @@ -1367,114 +1365,25 @@ fn load_multi_threaded<'a>( // 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)); - } - - Msg::FailedToParse(problem) => { - shut_down_worker_threads!(); - - 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; - } - 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)); - } - Err(e) => return Err(e), - } + return Err(e); } } } - - // The msg_rx receiver closed unexpectedly before we finished solving everything - Err(LoadingProblem::MsgChannelDied) }) } .unwrap() @@ -1512,7 +1421,7 @@ fn worker_task_step<'a>( // 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) { + if let Some(task) = find_task(worker, injector, stealers) { let result = run_task(task, worker_arena, src_dir, msg_tx.clone(), target_info); From 95bfc3b34224bfb87bbc78379a0c86b09910a02e Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 15:00:42 +0100 Subject: [PATCH 41/45] cleanup --- compiler/load/src/file.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 8262d466a6..6ff23b86ef 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1104,6 +1104,7 @@ fn load_single_threaded<'a>( 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), @@ -1113,6 +1114,7 @@ fn load_single_threaded<'a>( Err(e) => return Err(e), } + // then check if the worker can step let control_flow = worker_task_step( arena, &worker, @@ -1156,7 +1158,7 @@ fn state_thread_step<'a>( // We're done! There should be no more messages pending. debug_assert!(msg_rx.is_empty()); - return Ok(ControlFlow::Break(LoadResult::TypeChecked(finish( + let typechecked = finish( state, solved_subs, exposed_values, @@ -1164,7 +1166,9 @@ fn state_thread_step<'a>( exposed_vars_by_symbol, dep_idents, documentation, - )))); + ); + + Ok(ControlFlow::Break(LoadResult::TypeChecked(typechecked))) } Msg::FinishedAllSpecialization { subs, @@ -1173,20 +1177,20 @@ fn state_thread_step<'a>( // We're done! There should be no more messages pending. debug_assert!(msg_rx.is_empty()); - return Ok(ControlFlow::Break(LoadResult::Monomorphized( - finish_specialization(state, subs, exposed_to_host)?, - ))); + 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); - return Err(LoadingProblem::FormattedReport(buf)); + 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); - return Err(LoadingProblem::FormattedReport(buf)); + Err(LoadingProblem::FormattedReport(buf)) } msg => { // This is where most of the main thread's work gets done. @@ -1218,7 +1222,7 @@ fn state_thread_step<'a>( let buf = to_parse_problem_report(problem, module_ids, constrained_ident_ids); - return Err(LoadingProblem::FormattedReport(buf)); + Err(LoadingProblem::FormattedReport(buf)) } Err(e) => Err(e), } @@ -1227,7 +1231,7 @@ fn state_thread_step<'a>( } Err(err) => match err { crossbeam::channel::TryRecvError::Empty => Ok(ControlFlow::Continue(state)), - crossbeam::channel::TryRecvError::Disconnected => panic!(""), + crossbeam::channel::TryRecvError::Disconnected => Err(LoadingProblem::MsgChannelDied), }, } } From ae998e2504515fa8da6bc14528dff7816dd2cdc6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 16 Feb 2022 15:08:10 +0100 Subject: [PATCH 42/45] Revert "DEBUG HACKS, DO NOT MERGE" This reverts commit e95d32e821c005ac09028665c8e38f190ad86fa4. --- Cargo.lock | 3 --- compiler/load/Cargo.toml | 1 - compiler/load/src/file.rs | 4 +--- compiler/solve/Cargo.toml | 1 - repl_eval/Cargo.toml | 1 - repl_eval/src/gen.rs | 18 ------------------ repl_wasm/src/lib.rs | 4 +--- 7 files changed, 2 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7b31922f9..1414f20e6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3584,7 +3584,6 @@ dependencies = [ "roc_unify", "tempfile", "ven_pretty", - "wasm-bindgen", ] [[package]] @@ -3699,7 +3698,6 @@ dependencies = [ "roc_reporting", "roc_target", "roc_types", - "wasm-bindgen", ] [[package]] @@ -3765,7 +3763,6 @@ dependencies = [ "roc_types", "roc_unify", "tempfile", - "wasm-bindgen", ] [[package]] diff --git a/compiler/load/Cargo.toml b/compiler/load/Cargo.toml index 81d84b7890..57f4ffd530 100644 --- a/compiler/load/Cargo.toml +++ b/compiler/load/Cargo.toml @@ -26,7 +26,6 @@ bumpalo = { version = "3.8.0", features = ["collections"] } parking_lot = { version = "0.11.2" } crossbeam = "0.8.1" num_cpus = "1.13.0" -wasm-bindgen = "0.2.79" [dev-dependencies] tempfile = "3.2.0" diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 6ff23b86ef..0a119ecb23 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -755,7 +755,6 @@ enum BuildTask<'a> { }, } -#[derive(Debug)] enum WorkerMsg { Shutdown, TaskAdded, @@ -1404,8 +1403,7 @@ fn worker_task_step<'a>( src_dir: &Path, target_info: TargetInfo, ) -> Result, LoadingProblem<'a>> { - let recv = worker_msg_rx.try_recv(); - match recv { + match worker_msg_rx.try_recv() { Ok(msg) => { match msg { WorkerMsg::Shutdown => { diff --git a/compiler/solve/Cargo.toml b/compiler/solve/Cargo.toml index 7c27ea42af..ebb9e4a615 100644 --- a/compiler/solve/Cargo.toml +++ b/compiler/solve/Cargo.toml @@ -14,7 +14,6 @@ roc_can = { path = "../can" } roc_unify = { path = "../unify" } arrayvec = "0.7.2" bumpalo = { version = "3.8.0", features = ["collections"] } -wasm-bindgen = "0.2.79" [dev-dependencies] roc_load = { path = "../load" } diff --git a/repl_eval/Cargo.toml b/repl_eval/Cargo.toml index ca81ab5994..562cb27c9e 100644 --- a/repl_eval/Cargo.toml +++ b/repl_eval/Cargo.toml @@ -7,7 +7,6 @@ version = "0.1.0" [dependencies] bumpalo = {version = "3.8.0", features = ["collections"]} -wasm-bindgen = "0.2.79" roc_builtins = {path = "../compiler/builtins"} roc_can = {path = "../compiler/can"} diff --git a/repl_eval/src/gen.rs b/repl_eval/src/gen.rs index 453d9cc6df..a11ba966b8 100644 --- a/repl_eval/src/gen.rs +++ b/repl_eval/src/gen.rs @@ -8,22 +8,9 @@ use roc_load::file::{LoadingProblem, MonomorphizedModule}; use roc_parse::ast::Expr; use roc_region::all::LineInfo; use roc_target::TargetInfo; -use wasm_bindgen::prelude::wasm_bindgen; use crate::eval::ToAstProblem; -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(js_namespace = console)] - fn log(s: &str); -} - -// In-browser debugging -#[allow(unused_macros)] -macro_rules! console_log { - ($($t:tt)*) => (log(&format_args!($($t)*).to_string())) -} - pub enum ReplOutput { Problems(Vec), NoProblems { expr: String, expr_type: String }, @@ -58,8 +45,6 @@ pub fn compile_to_mono<'a>( src: &str, target_info: TargetInfo, ) -> Result, Vec> { - console_log!("compile_to_mono"); - use roc_reporting::report::{ can_problem, mono_problem, type_problem, RocDocAllocator, DEFAULT_PALETTE, }; @@ -71,9 +56,6 @@ pub fn compile_to_mono<'a>( let module_src = arena.alloc(promote_expr_to_module(src)); let exposed_types = MutMap::default(); - - console_log!("before load_and_monomorphize_from_str"); - let loaded = roc_load::file::load_and_monomorphize_from_str( arena, filename, diff --git a/repl_wasm/src/lib.rs b/repl_wasm/src/lib.rs index 037572ba7f..48910defba 100644 --- a/repl_wasm/src/lib.rs +++ b/repl_wasm/src/lib.rs @@ -31,7 +31,7 @@ extern "C" { fn js_get_result_and_memory(buffer_alloc_addr: *mut u8) -> usize; #[wasm_bindgen(js_namespace = console)] - pub fn log(s: &str); + fn log(s: &str); } // In-browser debugging @@ -158,10 +158,8 @@ impl<'a> ReplApp<'a> for WasmReplApp<'a> { #[wasm_bindgen] pub async fn entrypoint_from_js(src: String) -> Result { - console_log!("entrypoint_from_js"); let arena = &Bump::new(); let pre_linked_binary: &'static [u8] = include_bytes!("../data/pre_linked_binary.o"); - console_log!("pre_linked_binary {}", pre_linked_binary.len()); // Compile the app let target_info = TargetInfo::default_wasm32(); From 60b5465de207c870b07f619c9988781d5dc28aa2 Mon Sep 17 00:00:00 2001 From: Ryan Olson Date: Wed, 16 Feb 2022 10:55:02 -0700 Subject: [PATCH 43/45] Add --check flag for format command Closes #2495 --- cli/src/format.rs | 20 +++++++++++++++++--- cli/src/lib.rs | 12 ++++++++++++ cli/src/main.rs | 19 +++++++++++++++---- 3 files changed, 44 insertions(+), 7 deletions(-) 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")); From 909fae5b6c23396b3b8f034e189a7167e7a71bb9 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Thu, 17 Feb 2022 23:04:33 -0500 Subject: [PATCH 44/45] Generalize recursion variables properly Closes #2379 Closes #2481 --- compiler/solve/src/solve.rs | 24 ++++++++++++++++- compiler/solve/tests/solve_expr.rs | 41 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) 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 ef03d2a000..f524adca8a 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -5210,4 +5210,45 @@ mod solve_expr { "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 }", + ) + } } From 391068016037b9f5c131fd7146115c811edbe45d Mon Sep 17 00:00:00 2001 From: Jan Van Bruggen Date: Fri, 18 Feb 2022 02:54:34 -0700 Subject: [PATCH 45/45] Fix some extra-space typos --- TUTORIAL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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)" ```