From 52d6ab8b7ca321f28c97ef91a3fddefa78165aae Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 23 Dec 2020 23:59:54 +0100 Subject: [PATCH 01/26] WIP --- editor/src/def.rs | 17 +++ editor/src/expr.rs | 28 +++- editor/src/lib.rs | 1 + editor/src/module.rs | 314 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 359 insertions(+), 1 deletion(-) create mode 100644 editor/src/module.rs diff --git a/editor/src/def.rs b/editor/src/def.rs index 50b2f043c8..af27edc0f1 100644 --- a/editor/src/def.rs +++ b/editor/src/def.rs @@ -43,6 +43,23 @@ pub enum Def { Function(FunctionDef), } +impl Def { + pub fn pattern_vars(&self, pool: &Pool) -> impl Iterator<(Symbol, Variable)> { + use Def::*; + + match self { + AnnotationOnly { .. } => todo!("we lost pattern information!"), + Value(ValueDef { .. }) => symbols_and_variables_from_pattern(pool, pattern, var).iter(), + Function(FunctionDef::NoAnnotation { name, expr_var, .. }) => { + std::iter::once((name, expr_var)) + } + Function(FunctionDef::WithAnnotation { name, expr_var, .. }) => { + std::iter::once((name, expr_var)) + } + } + } +} + impl ShallowClone for Def { fn shallow_clone(&self) -> Self { match self { diff --git a/editor/src/expr.rs b/editor/src/expr.rs index 8cd7df43f8..acbf52b0d8 100644 --- a/editor/src/expr.rs +++ b/editor/src/expr.rs @@ -43,7 +43,7 @@ impl Output { pub struct Env<'a> { pub home: ModuleId, - pub var_store: VarStore, + pub var_store: &'a mut VarStore, pub pool: &'a mut Pool, pub arena: &'a Bump, @@ -63,6 +63,32 @@ pub struct Env<'a> { } impl<'a> Env<'a> { + pub fn new( + home: ModuleId, + arena: &'a Bump, + pool: &'a mut Pool, + var_store: &'a mut VarStore, + dep_idents: MutMap, + module_ids: &'a ModuleIds, + exposed_ident_ids: IdentIds, + ) -> Env<'a> { + Env { + home, + arena, + pool, + var_store, + dep_idents, + module_ids, + ident_ids: exposed_ident_ids.clone(), // we start with these, but will add more later + exposed_ident_ids, + closures: MutMap::default(), + qualified_lookups: MutSet::default(), + tailcallable_symbol: None, + closure_name_symbol: None, + top_level_symbols: MutSet::default(), + } + } + pub fn add(&mut self, item: T, region: Region) -> NodeId { let id = self.pool.add(item); self.set_region(id, region); diff --git a/editor/src/lib.rs b/editor/src/lib.rs index fa847f69e3..b62ae193e3 100644 --- a/editor/src/lib.rs +++ b/editor/src/lib.rs @@ -25,6 +25,7 @@ mod def; pub mod expr; pub mod file; mod keyboard_input; +mod module; mod ortho; mod pattern; pub mod pool; diff --git a/editor/src/module.rs b/editor/src/module.rs new file mode 100644 index 0000000000..7f19ca804d --- /dev/null +++ b/editor/src/module.rs @@ -0,0 +1,314 @@ +#![allow(clippy::all)] +#![allow(dead_code)] +#![allow(unused_imports)] +#![allow(unused_variables)] +use crate::def::{canonicalize_defs, sort_can_defs}; +use crate::expr::Env; +use crate::expr::Output; +use crate::pool::{Pool, PoolStr, PoolVec}; +use crate::scope::Scope; +use bumpalo::Bump; +use roc_can::operator::desugar_def; +use roc_collections::all::{default_hasher, ImMap, ImSet, MutMap, MutSet, SendMap}; +use roc_module::ident::Ident; +use roc_module::ident::Lowercase; +use roc_module::symbol::{IdentIds, ModuleId, ModuleIds, Symbol}; +use roc_parse::ast; +use roc_parse::pattern::PatternType; +use roc_problem::can::{Problem, RuntimeError}; +use roc_region::all::{Located, Region}; +use roc_types::subs::{VarStore, Variable}; +use roc_types::types::{Alias, Type}; + +pub struct ModuleOutput {} + +// TODO trim these down +#[allow(clippy::too_many_arguments)] +pub fn canonicalize_module_defs<'a>( + arena: &Bump, + loc_defs: &'a [Located>], + home: ModuleId, + module_ids: &ModuleIds, + exposed_ident_ids: IdentIds, + dep_idents: MutMap, + aliases: MutMap, + exposed_imports: MutMap, + mut exposed_symbols: MutSet, + var_store: &mut VarStore, +) -> Result { + let mut pool = Pool::with_capacity(1 << 10); + let mut can_exposed_imports = MutMap::default(); + let mut scope = Scope::new(home, &mut pool, var_store); + let num_deps = dep_idents.len(); + + // for (name, alias) in aliases.into_iter() { + // let vars = PoolVec::with_capacity(alias.vars.len() as u32, &mut pool); + // + // for (node_id, (lowercase, var)) in vars.iter_node_ids().zip(alias.vars) { + // let poolstr = PoolStr::new(lowercase.as_str(), &mut pool); + // + // pool[node_id] = (poolstr, var); + // } + // + // scope.add_alias(&mut pool, name, vars, alias.typ); + // } + + // Desugar operators (convert them to Apply calls, taking into account + // operator precedence and associativity rules), before doing other canonicalization. + // + // If we did this *during* canonicalization, then each time we + // visited a BinOp node we'd recursively try to apply this to each of its nested + // operators, and then again on *their* nested operators, ultimately applying the + // rules multiple times unnecessarily. + let mut desugared = + bumpalo::collections::Vec::with_capacity_in(loc_defs.len() + num_deps, arena); + + for loc_def in loc_defs.iter() { + desugared.push(&*arena.alloc(Located { + value: desugar_def(arena, &loc_def.value), + region: loc_def.region, + })); + } + + let mut env = Env::new( + home, + arena, + &mut pool, + &mut var_store, + dep_idents, + module_ids, + exposed_ident_ids, + ); + let mut lookups = Vec::with_capacity(num_deps); + let mut rigid_variables = MutMap::default(); + + // Exposed values are treated like defs that appear before any others, e.g. + // + // imports [ Foo.{ bar, baz } ] + // + // ...is basically the same as if we'd added these extra defs at the start of the module: + // + // bar = Foo.bar + // baz = Foo.baz + // + // Here we essentially add those "defs" to "the beginning of the module" + // by canonicalizing them right before we canonicalize the actual ast::Def nodes. + for (ident, (symbol, region)) in exposed_imports { + let first_char = ident.as_inline_str().chars().next().unwrap(); + + if first_char.is_lowercase() { + // this is a value definition + let expr_var = var_store.fresh(); + + match scope.import(ident, symbol, region) { + Ok(()) => { + // Add an entry to exposed_imports using the current module's name + // as the key; e.g. if this is the Foo module and we have + // exposes [ Bar.{ baz } ] then insert Foo.baz as the key, so when + // anything references `baz` in this Foo module, it will resolve to Bar.baz. + can_exposed_imports.insert(symbol, expr_var); + + // This will be used during constraint generation, + // to add the usual Lookup constraint as if this were a normal def. + lookups.push((symbol, expr_var, region)); + } + Err((_shadowed_symbol, _region)) => { + panic!("TODO gracefully handle shadowing in imports.") + } + } + } else { + // This is a type alias + + // the should already be added to the scope when this module is canonicalized + debug_assert!(scope.contains_alias(symbol)); + } + } + + let (defs, _scope, output, symbols_introduced) = canonicalize_defs( + &mut env, + Output::default(), + var_store, + &scope, + &desugared, + PatternType::TopLevelDef, + ); + + // See if any of the new idents we defined went unused. + // If any were unused and also not exposed, report it. + for (symbol, region) in symbols_introduced { + if !output.references.has_lookup(symbol) && !exposed_symbols.contains(&symbol) { + env.problem(Problem::UnusedDef(symbol, region)); + } + } + + // TODO register rigids + // for (var, lowercase) in output.introduced_variables.name_by_var.clone() { + // rigid_variables.insert(var, lowercase); + // } + + let mut references = MutSet::default(); + + // Gather up all the symbols that were referenced across all the defs' lookups. + for symbol in output.references.lookups.iter() { + references.insert(*symbol); + } + + // Gather up all the symbols that were referenced across all the defs' calls. + for symbol in output.references.calls.iter() { + references.insert(*symbol); + } + + // Gather up all the symbols that were referenced from other modules. + for symbol in env.qualified_lookups.iter() { + references.insert(*symbol); + } + + // NOTE previously we inserted builtin defs into the list of defs here + // this is now done later, in file.rs. + + match sort_can_defs(&mut env, defs, Output::default()) { + (Ok(mut declarations), output) => { + use crate::def::Declaration::*; + + // Record the variables for all exposed symbols. + let mut exposed_vars_by_symbol = Vec::with_capacity(exposed_symbols.len()); + + for decl in declarations.iter() { + match decl { + Declare(def) => { + for (symbol, variable) in def.pattern_vars(env.pool) { + if exposed_symbols.contains(symbol) { + // This is one of our exposed symbols; + // record the corresponding variable! + exposed_vars_by_symbol.push((*symbol, *variable)); + + // Remove this from exposed_symbols, + // so that at the end of the process, + // we can see if there were any + // exposed symbols which did not have + // corresponding defs. + exposed_symbols.remove(symbol); + } + } + } + DeclareRec(defs) => { + for def in defs { + for (symbol, variable) in def.pattern_vars(env.pool) { + if exposed_symbols.contains(symbol) { + // This is one of our exposed symbols; + // record the corresponding variable! + exposed_vars_by_symbol.push((*symbol, *variable)); + + // Remove this from exposed_symbols, + // so that at the end of the process, + // we can see if there were any + // exposed symbols which did not have + // corresponding defs. + exposed_symbols.remove(symbol); + } + } + } + } + + InvalidCycle(identifiers, _) => { + panic!("TODO gracefully handle potentially attempting to expose invalid cyclic defs {:?}" , identifiers); + } + Builtin(def) => { + // Builtins cannot be exposed in module declarations. + // This should never happen! + debug_assert!(def + .pattern_vars + .iter() + .all(|(symbol, _)| !exposed_symbols.contains(symbol))); + } + } + } + + let mut aliases = MutMap::default(); + + for (symbol, alias) in output.aliases { + // Remove this from exposed_symbols, + // so that at the end of the process, + // we can see if there were any + // exposed symbols which did not have + // corresponding defs. + exposed_symbols.remove(&symbol); + + aliases.insert(symbol, alias); + } + + // By this point, all exposed symbols should have been removed from + // exposed_symbols and added to exposed_vars_by_symbol. If any were + // not, that means they were declared as exposed but there was + // no actual declaration with that name! + for symbol in exposed_symbols { + env.problem(Problem::ExposedButNotDefined(symbol)); + + // In case this exposed value is referenced by other modules, + // create a decl for it whose implementation is a runtime error. + let mut pattern_vars = SendMap::default(); + pattern_vars.insert(symbol, var_store.fresh()); + + let runtime_error = RuntimeError::ExposedButNotDefined(symbol); + let def = Def { + loc_pattern: Located::new(0, 0, 0, 0, Pattern::Identifier(symbol)), + loc_expr: Located::new(0, 0, 0, 0, Expr::RuntimeError(runtime_error)), + expr_var: var_store.fresh(), + pattern_vars, + annotation: None, + }; + + declarations.push(Declaration::Declare(def)); + } + + // Incorporate any remaining output.lookups entries into references. + for symbol in output.references.lookups { + references.insert(symbol); + } + + // Incorporate any remaining output.calls entries into references. + for symbol in output.references.calls { + references.insert(symbol); + } + + // Gather up all the symbols that were referenced from other modules. + for symbol in env.qualified_lookups.iter() { + references.insert(*symbol); + } + + for declaration in declarations.iter_mut() { + match declaration { + Declare(def) => fix_values_captured_in_closure_def(def, &mut MutSet::default()), + DeclareRec(defs) => { + fix_values_captured_in_closure_defs(defs, &mut MutSet::default()) + } + InvalidCycle(_, _) | Builtin(_) => {} + } + } + + // TODO this loops over all symbols in the module, we can speed it up by having an + // iterator over all builtin symbols + 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) = builtins::builtin_defs_map(*symbol, var_store) { + declarations.push(Declaration::Builtin(def)); + } + } + } + + Ok(ModuleOutput { + aliases, + rigid_variables, + declarations, + references, + exposed_imports: can_exposed_imports, + problems: env.problems, + lookups, + exposed_vars_by_symbol, + ident_ids: env.ident_ids, + }) + } + (Err(runtime_error), _) => Err(runtime_error), + } +} From a6ee72dbacc26dc7fca8992cf7a78f3ca5f282d0 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 27 Dec 2020 16:22:34 +0100 Subject: [PATCH 02/26] basic version of editor/module --- editor/src/def.rs | 24 ++++++---- editor/src/module.rs | 111 ++++++++++++++++++++++--------------------- 2 files changed, 72 insertions(+), 63 deletions(-) diff --git a/editor/src/def.rs b/editor/src/def.rs index af27edc0f1..e8af973209 100644 --- a/editor/src/def.rs +++ b/editor/src/def.rs @@ -44,19 +44,24 @@ pub enum Def { } impl Def { - pub fn pattern_vars(&self, pool: &Pool) -> impl Iterator<(Symbol, Variable)> { - use Def::*; + pub fn symbols(&self, pool: &Pool) -> MutSet { + let mut output = MutSet::default(); match self { - AnnotationOnly { .. } => todo!("we lost pattern information!"), - Value(ValueDef { .. }) => symbols_and_variables_from_pattern(pool, pattern, var).iter(), - Function(FunctionDef::NoAnnotation { name, expr_var, .. }) => { - std::iter::once((name, expr_var)) - } - Function(FunctionDef::WithAnnotation { name, expr_var, .. }) => { - std::iter::once((name, expr_var)) + Def::AnnotationOnly { .. } => todo!("lost pattern information here ... "), + Def::Value(ValueDef { pattern, .. }) => { + let pattern2 = &pool[*pattern]; + output.extend(symbols_from_pattern(pool, pattern2)); } + Def::Function(function_def) => match function_def { + FunctionDef::NoAnnotation { name, .. } + | FunctionDef::WithAnnotation { name, .. } => { + output.insert(*name); + } + }, } + + output } } @@ -804,7 +809,6 @@ pub struct CanDefs { pub fn canonicalize_defs<'a>( env: &mut Env<'a>, mut output: Output, - var_store: &mut VarStore, original_scope: &Scope, loc_defs: &'a [&'a Located>], pattern_type: PatternType, diff --git a/editor/src/module.rs b/editor/src/module.rs index 7f19ca804d..80cd467536 100644 --- a/editor/src/module.rs +++ b/editor/src/module.rs @@ -2,11 +2,14 @@ #![allow(dead_code)] #![allow(unused_imports)] #![allow(unused_variables)] -use crate::def::{canonicalize_defs, sort_can_defs}; +use crate::ast::{FunctionDef, ValueDef}; +use crate::def::{canonicalize_defs, sort_can_defs, Declaration, Def}; use crate::expr::Env; use crate::expr::Output; -use crate::pool::{Pool, PoolStr, PoolVec}; +use crate::pattern::Pattern2; +use crate::pool::{NodeId, Pool, PoolStr, PoolVec}; use crate::scope::Scope; +use crate::types::Alias; use bumpalo::Bump; use roc_can::operator::desugar_def; use roc_collections::all::{default_hasher, ImMap, ImSet, MutMap, MutSet, SendMap}; @@ -18,9 +21,17 @@ use roc_parse::pattern::PatternType; use roc_problem::can::{Problem, RuntimeError}; use roc_region::all::{Located, Region}; use roc_types::subs::{VarStore, Variable}; -use roc_types::types::{Alias, Type}; -pub struct ModuleOutput {} +pub struct ModuleOutput { + pub aliases: MutMap>, + pub rigid_variables: MutMap, + pub declarations: Vec, + pub exposed_imports: MutMap, + pub lookups: Vec<(Symbol, Variable, Region)>, + pub problems: Vec, + pub ident_ids: IdentIds, + pub references: MutSet, +} // TODO trim these down #[allow(clippy::too_many_arguments)] @@ -74,13 +85,13 @@ pub fn canonicalize_module_defs<'a>( home, arena, &mut pool, - &mut var_store, + var_store, dep_idents, module_ids, exposed_ident_ids, ); let mut lookups = Vec::with_capacity(num_deps); - let mut rigid_variables = MutMap::default(); + let rigid_variables = MutMap::default(); // Exposed values are treated like defs that appear before any others, e.g. // @@ -98,7 +109,7 @@ pub fn canonicalize_module_defs<'a>( if first_char.is_lowercase() { // this is a value definition - let expr_var = var_store.fresh(); + let expr_var = env.var_store.fresh(); match scope.import(ident, symbol, region) { Ok(()) => { @@ -127,7 +138,6 @@ pub fn canonicalize_module_defs<'a>( let (defs, _scope, output, symbols_introduced) = canonicalize_defs( &mut env, Output::default(), - var_store, &scope, &desugared, PatternType::TopLevelDef, @@ -170,41 +180,30 @@ pub fn canonicalize_module_defs<'a>( (Ok(mut declarations), output) => { use crate::def::Declaration::*; - // Record the variables for all exposed symbols. - let mut exposed_vars_by_symbol = Vec::with_capacity(exposed_symbols.len()); - for decl in declarations.iter() { match decl { Declare(def) => { - for (symbol, variable) in def.pattern_vars(env.pool) { - if exposed_symbols.contains(symbol) { - // This is one of our exposed symbols; - // record the corresponding variable! - exposed_vars_by_symbol.push((*symbol, *variable)); - + for symbol in def.symbols(env.pool) { + if exposed_symbols.contains(&symbol) { // Remove this from exposed_symbols, // so that at the end of the process, // we can see if there were any // exposed symbols which did not have // corresponding defs. - exposed_symbols.remove(symbol); + exposed_symbols.remove(&symbol); } } } DeclareRec(defs) => { for def in defs { - for (symbol, variable) in def.pattern_vars(env.pool) { - if exposed_symbols.contains(symbol) { - // This is one of our exposed symbols; - // record the corresponding variable! - exposed_vars_by_symbol.push((*symbol, *variable)); - + for symbol in def.symbols(env.pool) { + if exposed_symbols.contains(&symbol) { // Remove this from exposed_symbols, // so that at the end of the process, // we can see if there were any // exposed symbols which did not have // corresponding defs. - exposed_symbols.remove(symbol); + exposed_symbols.remove(&symbol); } } } @@ -217,9 +216,9 @@ pub fn canonicalize_module_defs<'a>( // Builtins cannot be exposed in module declarations. // This should never happen! debug_assert!(def - .pattern_vars + .symbols(env.pool) .iter() - .all(|(symbol, _)| !exposed_symbols.contains(symbol))); + .all(|symbol| !exposed_symbols.contains(symbol))); } } } @@ -247,17 +246,21 @@ pub fn canonicalize_module_defs<'a>( // In case this exposed value is referenced by other modules, // create a decl for it whose implementation is a runtime error. let mut pattern_vars = SendMap::default(); - pattern_vars.insert(symbol, var_store.fresh()); + pattern_vars.insert(symbol, env.var_store.fresh()); let runtime_error = RuntimeError::ExposedButNotDefined(symbol); - let def = Def { - loc_pattern: Located::new(0, 0, 0, 0, Pattern::Identifier(symbol)), - loc_expr: Located::new(0, 0, 0, 0, Expr::RuntimeError(runtime_error)), - expr_var: var_store.fresh(), - pattern_vars, - annotation: None, + + let value_def = { + let pattern = env.pool.add(Pattern2::Identifier(symbol)); + ValueDef { + pattern, + expr_type: None, + expr_var: env.var_store.fresh(), + } }; + let def = Def::Value(value_def); + declarations.push(Declaration::Declare(def)); } @@ -276,26 +279,29 @@ pub fn canonicalize_module_defs<'a>( references.insert(*symbol); } - for declaration in declarations.iter_mut() { - match declaration { - Declare(def) => fix_values_captured_in_closure_def(def, &mut MutSet::default()), - DeclareRec(defs) => { - fix_values_captured_in_closure_defs(defs, &mut MutSet::default()) - } - InvalidCycle(_, _) | Builtin(_) => {} - } - } + // TODO find captured variables + // for declaration in declarations.iter_mut() { + // match declaration { + // Declare(def) => fix_values_captured_in_closure_def(def, &mut MutSet::default()), + // DeclareRec(defs) => { + // fix_values_captured_in_closure_defs(defs, &mut MutSet::default()) + // } + // InvalidCycle(_, _) | Builtin(_) => {} + // } + // } // TODO this loops over all symbols in the module, we can speed it up by having an // iterator over all builtin symbols - 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) = builtins::builtin_defs_map(*symbol, var_store) { - declarations.push(Declaration::Builtin(def)); - } - } - } + + // TODO move over the builtins + // 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) = builtins::builtin_defs_map(*symbol, var_store) { + // declarations.push(Declaration::Builtin(def)); + // } + // } + // } Ok(ModuleOutput { aliases, @@ -303,9 +309,8 @@ pub fn canonicalize_module_defs<'a>( declarations, references, exposed_imports: can_exposed_imports, - problems: env.problems, + problems: vec![], // TODO env.problems, lookups, - exposed_vars_by_symbol, ident_ids: env.ident_ids, }) } From 2f3d74e8bdfbd18e2cb00ec341ad464ad2f8a750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 17:37:47 +0100 Subject: [PATCH 03/26] fix empty record with only comments in it --- compiler/fmt/src/expr.rs | 2 +- compiler/fmt/tests/test_fmt.rs | 21 +++++++++++++++++++++ compiler/parse/src/ast.rs | 9 +++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index ec386bc54e..31df63d86c 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -809,7 +809,7 @@ pub fn fmt_record<'a>( final_comments: &'a [CommentOrNewline<'a>], indent: u16, ) { - if loc_fields.is_empty() { + if loc_fields.is_empty() && final_comments.iter().all(|c| c.is_newline()) { buf.push_str("{}"); } else { buf.push('{'); diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 5daac4b348..3e7586b453 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -1271,6 +1271,27 @@ mod test_fmt { expr_formats_same("{}"); } + #[test] + fn empty_record_with_comment() { + expr_formats_same(indoc!( + r#" + { + # comment + }"# + )); + } + + #[test] + fn empty_record_with_newline() { + expr_formats_to( + indoc!( + r#" + { + }"# + ), + "{}", + ); + } #[test] fn one_field() { expr_formats_same("{ x: 4 }"); diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index d707775ab9..e26de1b37d 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -300,6 +300,15 @@ impl<'a> CommentOrNewline<'a> { DocComment(_) => true, } } + + pub fn is_newline(&self) -> bool { + use CommentOrNewline::*; + match self { + Newline => true, + LineComment(_) => false, + DocComment(_) => false, + } + } } #[derive(Clone, Debug, PartialEq)] From 04d0711251f55100ecb8e71482400dd6c6b6f976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 17:53:14 +0100 Subject: [PATCH 04/26] multiline tests update for trailing comma This reverts commit 4dbde30c4f6584871d3e8c54fd89b3ee27a82b72. --- compiler/fmt/tests/test_fmt.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 3e7586b453..1f487eefb1 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -1022,7 +1022,7 @@ mod test_fmt { [ 7, 8, - 9 + 9, ] "# )); @@ -1041,7 +1041,7 @@ mod test_fmt { [ 17, 18, - 19 + 19, ] "# ), @@ -1064,7 +1064,7 @@ mod test_fmt { [ 27, 28, - 29 + 29, ] "# ), @@ -1084,7 +1084,7 @@ mod test_fmt { [ 157, 158, - 159 + 159, ] "# ), @@ -1105,7 +1105,7 @@ mod test_fmt { 557, 648, 759, - 837 + 837, ] "# ), @@ -1127,7 +1127,7 @@ mod test_fmt { 257, 358, # Hey! - 459 + 459, ] "# ), @@ -1155,7 +1155,7 @@ mod test_fmt { 37, # Thirty Eight 38, - 39 + 39, ] "# ), @@ -1189,7 +1189,7 @@ mod test_fmt { 48, # Bottom 48 # Top 49 - 49 + 49, # Bottom 49 # 49! ] @@ -1228,7 +1228,7 @@ mod test_fmt { results = [ Ok 4, - Ok 5 + Ok 5, ] allOks results From 83aa5c1642b0d2b1ff5d2a5299df0059bb066a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 18:13:54 +0100 Subject: [PATCH 05/26] add final_comments to List's AST --- compiler/fmt/src/def.rs | 2 +- compiler/fmt/src/expr.rs | 18 +++++++++++++----- compiler/parse/src/ast.rs | 5 ++++- compiler/parse/src/expr.rs | 15 +++++++++++---- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/compiler/fmt/src/def.rs b/compiler/fmt/src/def.rs index ef83d39241..f6d9558ede 100644 --- a/compiler/fmt/src/def.rs +++ b/compiler/fmt/src/def.rs @@ -122,7 +122,7 @@ pub fn fmt_body<'a>( Expr::SpaceBefore(_, _) => { body.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent + INDENT); } - Expr::Record { .. } | Expr::List(_) => { + Expr::Record { .. } | Expr::List { .. } => { newline(buf, indent + INDENT); body.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent + INDENT); } diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index 31df63d86c..8a7b9ba4e8 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -38,7 +38,7 @@ impl<'a> Formattable<'a> for Expr<'a> { // These expressions always have newlines Defs(_, _) | When(_, _) => true, - List(elems) => elems.iter().any(|loc_expr| loc_expr.is_multiline()), + List { items, .. } => items.iter().any(|loc_expr| loc_expr.is_multiline()), Str(literal) => { use roc_parse::ast::StrLiteral::*; @@ -261,8 +261,11 @@ impl<'a> Formattable<'a> for Expr<'a> { fmt_if(buf, loc_condition, loc_then, loc_else, indent); } When(loc_condition, branches) => fmt_when(buf, loc_condition, branches, indent), - List(loc_items) => { - fmt_list(buf, &loc_items, indent); + List { + items, + final_comments, + } => { + fmt_list(buf, &items, final_comments, indent); } BinOp((loc_left_side, bin_op, loc_right_side)) => fmt_bin_op( buf, @@ -396,7 +399,12 @@ fn fmt_bin_op<'a>( } } -pub fn fmt_list<'a>(buf: &mut String<'a>, loc_items: &[&Located>], indent: u16) { +pub fn fmt_list<'a>( + buf: &mut String<'a>, + loc_items: &[&Located>], + _final_comments: &'a [CommentOrNewline<'a>], + indent: u16, +) { buf.push('['); let mut iter = loc_items.iter().peekable(); @@ -846,7 +854,7 @@ pub fn fmt_record<'a>( newline(buf, indent); } else { - // is_multiline == false */ + // is_multiline == false buf.push(' '); let field_indent = indent; let mut iter = loc_fields.iter().peekable(); diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index e26de1b37d..d4961543e0 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -93,7 +93,10 @@ pub enum Expr<'a> { AccessorFunction(&'a str), // Collection Literals - List(&'a [&'a Loc>]), + List { + items: &'a [&'a Loc>], + final_comments: &'a [CommentOrNewline<'a>], + }, Record { update: Option<&'a Loc>>, diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index ee66a5bc9c..743d32262b 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -306,7 +306,7 @@ fn expr_to_pattern<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result, // These would not have parsed as patterns Expr::AccessorFunction(_) | Expr::Access(_, _) - | Expr::List(_) + | Expr::List { .. } | Expr::Closure(_, _) | Expr::BinOp(_) | Expr::Defs(_, _) @@ -1848,7 +1848,7 @@ fn binop<'a>() -> impl Parser<'a, BinOp> { } pub fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>> { - let elems = collection!( + let elems = collection_trailing_sep!( ascii_char(b'['), loc!(expr(min_indent)), ascii_char(b','), @@ -1858,14 +1858,21 @@ pub fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>> { parser::attempt( Attempting::List, - map_with_arena!(elems, |arena, parsed_elems: Vec<'a, Located>>| { + map_with_arena!(elems, |arena, + (parsed_elems, final_comments): ( + Vec<'a, Located>>, + &'a [CommentOrNewline<'a>] + )| { let mut allocated = Vec::with_capacity_in(parsed_elems.len(), arena); for parsed_elem in parsed_elems { allocated.push(&*arena.alloc(parsed_elem)); } - Expr::List(allocated.into_bump_slice()) + Expr::List { + items: allocated.into_bump_slice(), + final_comments: final_comments, + } }), ) } From 3d0a5aa89a5e641f8c5545a24e736a56b5c12949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 18:37:41 +0100 Subject: [PATCH 06/26] format multline litera list with trailing comma --- compiler/fmt/src/expr.rs | 118 ++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 64 deletions(-) diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index 8a7b9ba4e8..fa2f1cd8b7 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -402,94 +402,84 @@ fn fmt_bin_op<'a>( pub fn fmt_list<'a>( buf: &mut String<'a>, loc_items: &[&Located>], - _final_comments: &'a [CommentOrNewline<'a>], + final_comments: &'a [CommentOrNewline<'a>], indent: u16, ) { - buf.push('['); - - let mut iter = loc_items.iter().peekable(); - - let is_multiline = loc_items.iter().any(|item| (&item.value).is_multiline()); - - let item_indent = if is_multiline { - indent + INDENT + if loc_items.is_empty() && final_comments.iter().all(|c| c.is_newline()) { + buf.push_str("[]"); } else { - indent - }; - - while let Some(item) = iter.next() { + buf.push('['); + let is_multiline = loc_items.iter().any(|item| (&item.value).is_multiline()); if is_multiline { - match &item.value { - Expr::SpaceBefore(expr_below, spaces_above_expr) => { - newline(buf, item_indent); - fmt_comments_only( - buf, - spaces_above_expr.iter(), - NewlineAt::Bottom, - item_indent, - ); + let item_indent = indent + INDENT; + for item in loc_items.iter() { + match &item.value { + // TODO?? These SpaceAfter/SpaceBefore litany seems overcomplicated + // Can we simplify this? + Expr::SpaceBefore(expr_below, spaces_above_expr) => { + newline(buf, item_indent); + fmt_comments_only( + buf, + spaces_above_expr.iter(), + NewlineAt::Bottom, + item_indent, + ); - match &expr_below { - Expr::SpaceAfter(expr_above, spaces_below_expr) => { - expr_above.format(buf, item_indent); - - if iter.peek().is_some() { + match &expr_below { + Expr::SpaceAfter(expr_above, spaces_below_expr) => { + expr_above.format(buf, item_indent); buf.push(','); - } - fmt_comments_only( - buf, - spaces_below_expr.iter(), - NewlineAt::Top, - item_indent, - ); - } - _ => { - expr_below.format(buf, item_indent); - if iter.peek().is_some() { + fmt_comments_only( + buf, + spaces_below_expr.iter(), + NewlineAt::Top, + item_indent, + ); + } + _ => { + expr_below.format(buf, item_indent); buf.push(','); } } } - } - Expr::SpaceAfter(sub_expr, spaces) => { - newline(buf, item_indent); + Expr::SpaceAfter(sub_expr, spaces) => { + newline(buf, item_indent); - sub_expr.format(buf, item_indent); - - if iter.peek().is_some() { + sub_expr.format(buf, item_indent); buf.push(','); + + fmt_comments_only(buf, spaces.iter(), NewlineAt::Top, item_indent); } - fmt_comments_only(buf, spaces.iter(), NewlineAt::Top, item_indent); - } - - _ => { - newline(buf, item_indent); - item.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, item_indent); - if iter.peek().is_some() { + _ => { + newline(buf, item_indent); + item.format_with_options( + buf, + Parens::NotNeeded, + Newlines::Yes, + item_indent, + ); buf.push(','); } } } + newline(buf, indent); + buf.push(']'); } else { - buf.push(' '); - item.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, item_indent); - if iter.peek().is_some() { - buf.push(','); + // is_multiline == false + let mut iter = loc_items.iter().peekable(); + while let Some(item) = iter.next() { + buf.push(' '); + item.format_with_options(buf, Parens::NotNeeded, Newlines::Yes, indent); + if iter.peek().is_some() { + buf.push(','); + } } + buf.push_str(" ]"); } } - - if is_multiline { - newline(buf, indent); - } - - if !loc_items.is_empty() && !is_multiline { - buf.push(' '); - } - buf.push(']'); } pub fn empty_line_before_expr<'a>(expr: &'a Expr<'a>) -> bool { From 7692bd567167bfae403d2a368bc2cc38e2d5e0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 18:41:48 +0100 Subject: [PATCH 07/26] add test... and fix missing final comments! --- compiler/fmt/src/expr.rs | 1 + compiler/fmt/tests/test_fmt.rs | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index fa2f1cd8b7..623326ac19 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -465,6 +465,7 @@ pub fn fmt_list<'a>( } } } + fmt_comments_only(buf, final_comments.iter(), NewlineAt::Top, item_indent); newline(buf, indent); buf.push(']'); } else { diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 1f487eefb1..8b7004fbcb 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -1197,7 +1197,31 @@ mod test_fmt { ), ); } - + #[test] + fn ending_comments_in_list() { + expr_formats_to( + indoc!( + r#" + [ # Top 49 + 49 + # Bottom 49 + , + # 49! + ] + "# + ), + indoc!( + r#" + [ + # Top 49 + 49, + # Bottom 49 + # 49! + ] + "# + ), + ); + } #[test] fn multi_line_list_def() { // expr_formats_same(indoc!( From 965d151168f3c58c4463ea4fd1eaf220f108c694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 18:57:45 +0100 Subject: [PATCH 08/26] add tag union tests for trailing commas formatting --- compiler/fmt/src/expr.rs | 2 +- compiler/fmt/tests/test_fmt.rs | 62 ++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/compiler/fmt/src/expr.rs b/compiler/fmt/src/expr.rs index 623326ac19..1be56ee514 100644 --- a/compiler/fmt/src/expr.rs +++ b/compiler/fmt/src/expr.rs @@ -415,7 +415,7 @@ pub fn fmt_list<'a>( for item in loc_items.iter() { match &item.value { // TODO?? These SpaceAfter/SpaceBefore litany seems overcomplicated - // Can we simplify this? + // Can we simplify this? Or at least move this in a separate function. Expr::SpaceBefore(expr_below, spaces_above_expr) => { newline(buf, item_indent); fmt_comments_only( diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index 8b7004fbcb..adcbfd46b7 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -2452,22 +2452,54 @@ mod test_fmt { )); } - // TODO This raises a parse error: - // NotYetImplemented("TODO the : in this declaration seems outdented") - // #[test] - // fn multiline_tag_union_annotation() { - // expr_formats_same(indoc!( - // r#" - // b : - // [ - // True, - // False, - // ] + #[test] + fn multiline_tag_union_annotation() { + expr_formats_same(indoc!( + r#" + b : + [ + True, + False, + ] - // b - // "# - // )); - // } + b + "# + )); + } + + #[test] + fn multiline_tag_union_annotation_with_final_comment() { + expr_formats_to( + indoc!( + r#" + b : + [ + True, + # comment 1 + False # comment 2 + , + # comment 3 + ] + + b + "# + ), + indoc!( + r#" + b : + [ + True, + # comment 1 + False, + # comment 2 + # comment 3 + ] + + b + "# + ), + ); + } #[test] fn tag_union() { From 69c38e5bf7969018c394fb30a9d42d69ed805412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 19:12:37 +0100 Subject: [PATCH 09/26] add some commented tests -- it doesn't involve trialing commas too much... --- compiler/fmt/tests/test_fmt.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/compiler/fmt/tests/test_fmt.rs b/compiler/fmt/tests/test_fmt.rs index adcbfd46b7..bd99d58dd2 100644 --- a/compiler/fmt/tests/test_fmt.rs +++ b/compiler/fmt/tests/test_fmt.rs @@ -2513,6 +2513,28 @@ mod test_fmt { )); } + // TODO: the current formatting seems a bit odd for multiline function annotations + // (beside weird indentation, note the trailing space after the "->") + // #[test] + // fn multiline_tag_union_function_annotation() { + // expr_formats_same(indoc!( + // r#" + // f : + // [ + // True, + // False, + // ] -> + // [ + // True, + // False, + // ] + // f = \x -> x + + // a + // "# + // )); + // } + #[test] fn recursive_tag_union() { expr_formats_same(indoc!( From 07d4f8dc15b9b88bf40eb89df9bfaf8cbeafc69c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 19:54:17 +0100 Subject: [PATCH 10/26] make clippy happy --- compiler/parse/src/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 743d32262b..f584b16d33 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -1871,7 +1871,7 @@ pub fn list_literal<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>> { Expr::List { items: allocated.into_bump_slice(), - final_comments: final_comments, + final_comments, } }), ) From 4e9387cbdabc2f6b396c35b5d7625439df94c7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 21:17:27 +0100 Subject: [PATCH 11/26] fix compile issues --- cli/src/repl/eval.rs | 15 ++++++++++++--- compiler/can/src/expr.rs | 4 +++- compiler/can/src/operator.rs | 22 ++++++++++++++++------ editor/src/expr.rs | 8 ++++---- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/cli/src/repl/eval.rs b/cli/src/repl/eval.rs index 5e4d07e6bc..7e437ba1e7 100644 --- a/cli/src/repl/eval.rs +++ b/cli/src/repl/eval.rs @@ -93,7 +93,10 @@ fn jit_to_ast_help<'a>( ), Layout::Builtin(Builtin::EmptyList) => { Ok(run_jit_function!(lib, main_fn_name, &'static str, |_| { - Expr::List(&[]) + Expr::List { + items: &[], + final_comments: &[], + } })) } Layout::Builtin(Builtin::List(_, elem_layout)) => Ok(run_jit_function!( @@ -251,7 +254,10 @@ fn ptr_to_ast<'a>( num_to_ast(env, f64_to_ast(env.arena, num), content) } - Layout::Builtin(Builtin::EmptyList) => Expr::List(&[]), + Layout::Builtin(Builtin::EmptyList) => Expr::List { + items: &[], + final_comments: &[], + }, Layout::Builtin(Builtin::List(_, elem_layout)) => { // Turn the (ptr, len) wrapper struct into actual ptr and len values. let len = unsafe { *(ptr.offset(env.ptr_bytes as isize) as *const usize) }; @@ -331,7 +337,10 @@ fn list_to_ast<'a>( let output = output.into_bump_slice(); - Expr::List(output) + Expr::List { + items: output, + final_comments: &[], + } } fn single_tag_union_to_ast<'a>( diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index cf1cc2d02b..777e51f599 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -285,7 +285,9 @@ pub fn canonicalize_expr<'a>( } } ast::Expr::Str(literal) => flatten_str_literal(env, var_store, scope, literal), - ast::Expr::List(loc_elems) => { + ast::Expr::List { + items: loc_elems, .. + } => { if loc_elems.is_empty() { ( List { diff --git a/compiler/can/src/operator.rs b/compiler/can/src/operator.rs index 0ec2c82f72..5f9d73bb70 100644 --- a/compiler/can/src/operator.rs +++ b/compiler/can/src/operator.rs @@ -109,14 +109,24 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Located>) -> &'a arena.alloc(Located { region, value }) } - List(elems) | Nested(List(elems)) => { - let mut new_elems = Vec::with_capacity_in(elems.len(), arena); + List { + items, + final_comments, + } + | Nested(List { + items, + final_comments, + }) => { + let mut new_items = Vec::with_capacity_in(items.len(), arena); - for elem in elems.iter() { - new_elems.push(desugar_expr(arena, elem)); + for item in items.iter() { + new_items.push(desugar_expr(arena, item)); } - let new_elems = new_elems.into_bump_slice(); - let value: Expr<'a> = List(new_elems); + let new_items = new_items.into_bump_slice(); + let value: Expr<'a> = List { + items: new_items, + final_comments, + }; arena.alloc(Located { region: loc_expr.region, diff --git a/editor/src/expr.rs b/editor/src/expr.rs index 8cd7df43f8..1af1747c11 100644 --- a/editor/src/expr.rs +++ b/editor/src/expr.rs @@ -285,14 +285,14 @@ pub fn to_expr2<'a>( Str(literal) => flatten_str_literal(env, scope, &literal), - List(elements) => { + List { items, .. } => { let mut output = Output::default(); let output_ref = &mut output; - let elems = PoolVec::with_capacity(elements.len() as u32, env.pool); + let elems = PoolVec::with_capacity(items.len() as u32, env.pool); - for (node_id, element) in elems.iter_node_ids().zip(elements.iter()) { - let (expr, sub_output) = to_expr2(env, scope, &element.value, element.region); + for (node_id, item) in elems.iter_node_ids().zip(items.iter()) { + let (expr, sub_output) = to_expr2(env, scope, &item.value, item.region); output_ref.union(sub_output); From 70b3b77ac08e0d3ce3e6c33c249fcb897f4a99c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Besnier?= Date: Mon, 28 Dec 2020 21:36:22 +0100 Subject: [PATCH 12/26] fixing tests compiling issues --- compiler/parse/tests/test_parse.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/compiler/parse/tests/test_parse.rs b/compiler/parse/tests/test_parse.rs index 5f07bc73aa..cbfb86db3f 100644 --- a/compiler/parse/tests/test_parse.rs +++ b/compiler/parse/tests/test_parse.rs @@ -1017,8 +1017,10 @@ mod test_parse { #[test] fn empty_list() { let arena = Bump::new(); - let elems = &[]; - let expected = List(elems); + let expected = List { + items: &[], + final_comments: &[], + }; let actual = parse_expr_with(&arena, "[]"); assert_eq!(Ok(expected), actual); @@ -1028,8 +1030,10 @@ mod test_parse { fn spaces_inside_empty_list() { // This is a regression test! let arena = Bump::new(); - let elems = &[]; - let expected = List(elems); + let expected = List { + items: &[], + final_comments: &[], + }; let actual = parse_expr_with(&arena, "[ ]"); assert_eq!(Ok(expected), actual); @@ -1038,8 +1042,11 @@ mod test_parse { #[test] fn packed_singleton_list() { let arena = Bump::new(); - let elems = &[&*arena.alloc(Located::new(0, 0, 1, 2, Num("1")))]; - let expected = List(elems); + let items = &[&*arena.alloc(Located::new(0, 0, 1, 2, Num("1")))]; + let expected = List { + items, + final_comments: &[], + }; let actual = parse_expr_with(&arena, "[1]"); assert_eq!(Ok(expected), actual); @@ -1048,8 +1055,11 @@ mod test_parse { #[test] fn spaced_singleton_list() { let arena = Bump::new(); - let elems = &[&*arena.alloc(Located::new(0, 0, 2, 3, Num("1")))]; - let expected = List(elems); + let items = &[&*arena.alloc(Located::new(0, 0, 2, 3, Num("1")))]; + let expected = List { + items, + final_comments: &[], + }; let actual = parse_expr_with(&arena, "[ 1 ]"); assert_eq!(Ok(expected), actual); From a5d58c4a7f316bacbf2a10f1ea9faae755c50ed0 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 28 Dec 2020 22:18:16 +0100 Subject: [PATCH 13/26] introduce aliases into scope --- editor/src/module.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/editor/src/module.rs b/editor/src/module.rs index 80cd467536..73b1fc7086 100644 --- a/editor/src/module.rs +++ b/editor/src/module.rs @@ -7,7 +7,7 @@ use crate::def::{canonicalize_defs, sort_can_defs, Declaration, Def}; use crate::expr::Env; use crate::expr::Output; use crate::pattern::Pattern2; -use crate::pool::{NodeId, Pool, PoolStr, PoolVec}; +use crate::pool::{NodeId, Pool, PoolStr, PoolVec, ShallowClone}; use crate::scope::Scope; use crate::types::Alias; use bumpalo::Bump; @@ -52,17 +52,15 @@ pub fn canonicalize_module_defs<'a>( let mut scope = Scope::new(home, &mut pool, var_store); let num_deps = dep_idents.len(); - // for (name, alias) in aliases.into_iter() { - // let vars = PoolVec::with_capacity(alias.vars.len() as u32, &mut pool); - // - // for (node_id, (lowercase, var)) in vars.iter_node_ids().zip(alias.vars) { - // let poolstr = PoolStr::new(lowercase.as_str(), &mut pool); - // - // pool[node_id] = (poolstr, var); - // } - // - // scope.add_alias(&mut pool, name, vars, alias.typ); - // } + for (name, alias) in aliases.into_iter() { + let vars = PoolVec::with_capacity(alias.targs.len() as u32, &mut pool); + + for (node_id, targ_id) in vars.iter_node_ids().zip(alias.targs.iter_node_ids()) { + let (poolstr, var) = &pool[targ_id]; + pool[node_id] = (poolstr.shallow_clone(), *var); + } + scope.add_alias(&mut pool, name, vars, alias.actual); + } // Desugar operators (convert them to Apply calls, taking into account // operator precedence and associativity rules), before doing other canonicalization. From 4cdde12823698fe211692f570e0863f968282495 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 28 Dec 2020 22:34:27 +0100 Subject: [PATCH 14/26] add tests --- compiler/gen/tests/gen_primitives.rs | 37 ++++++++++++++++++++++++++++ compiler/gen/tests/helpers/eval.rs | 1 + 2 files changed, 38 insertions(+) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index 56c39caef0..9685c33bc4 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1748,4 +1748,41 @@ mod gen_primitives { u8 ); } + + #[test] + #[should_panic(expected = "shadowed")] + fn pattern_shadowing() { + assert_evals_to!( + indoc!( + r#" + x = 4 + + when 4 is + x -> x + "# + ), + 0, + i64 + ); + } + + #[test] + #[ignore] + #[should_panic(expected = "shadowed")] + fn unsupported_pattern_tag() { + assert_evals_to!( + indoc!( + r#" + x : Result I64 F64 + x = Ok 4 + + (Ok y) = x + + y + "# + ), + 0, + i64 + ); + } } diff --git a/compiler/gen/tests/helpers/eval.rs b/compiler/gen/tests/helpers/eval.rs index fe22eebc85..18781cbc98 100644 --- a/compiler/gen/tests/helpers/eval.rs +++ b/compiler/gen/tests/helpers/eval.rs @@ -106,6 +106,7 @@ pub fn helper<'a>( | UnusedArgument(_, _, _) | UnusedImport(_, _) | RuntimeError(_) + | UnsupportedPattern(_, _) | ExposedButNotDefined(_) => { delayed_errors.push(problem.clone()); From 794f8c4d4125aba54fa919a055021221d124ecd7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 28 Dec 2020 23:14:04 +0100 Subject: [PATCH 15/26] remove Shadowed variant from mono pattern --- compiler/gen/tests/gen_primitives.rs | 6 +- compiler/mono/src/decision_tree.rs | 6 +- compiler/mono/src/exhaustive.rs | 5 -- compiler/mono/src/ir.rs | 125 ++++++++++++++++++--------- 4 files changed, 90 insertions(+), 52 deletions(-) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index 9685c33bc4..d784f8b741 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1750,7 +1750,9 @@ mod gen_primitives { } #[test] - #[should_panic(expected = "shadowed")] + #[should_panic( + expected = "Roc failed with message: \"Shadowing { original_region: |L 3-3, C 4-5|, shadow: |L 6-6, C 8-9| Ident(\\\"x\\\") }\"" + )] fn pattern_shadowing() { assert_evals_to!( indoc!( @@ -1768,7 +1770,7 @@ mod gen_primitives { #[test] #[ignore] - #[should_panic(expected = "shadowed")] + #[should_panic(expected = "foo")] fn unsupported_pattern_tag() { assert_evals_to!( indoc!( diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index ec9fff2c5f..4983332d9a 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -379,7 +379,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: &Branch<'a>, all_tests: &mut V match pattern { // TODO use guard! - Identifier(_) | Underscore | Shadowed(_, _) | UnsupportedPattern(_) => { + Identifier(_) | Underscore | UnsupportedPattern(_) => { if let Guard::Guard { symbol, id, stmt } = guard { all_tests.push(Guarded { opt_test: None, @@ -531,7 +531,7 @@ fn to_relevant_branch_help<'a>( use Test::*; match pattern { - Identifier(_) | Underscore | Shadowed(_, _) | UnsupportedPattern(_) => Some(branch.clone()), + Identifier(_) | Underscore | UnsupportedPattern(_) => Some(branch.clone()), RecordDestructure(destructs, _) => match test { IsCtor { @@ -742,7 +742,7 @@ fn needs_tests<'a>(pattern: &Pattern<'a>) -> bool { use Pattern::*; match pattern { - Identifier(_) | Underscore | Shadowed(_, _) | UnsupportedPattern(_) => false, + Identifier(_) | Underscore | UnsupportedPattern(_) => false, RecordDestructure(_, _) | AppliedTag { .. } diff --git a/compiler/mono/src/exhaustive.rs b/compiler/mono/src/exhaustive.rs index 70077c2199..1c253b5a97 100644 --- a/compiler/mono/src/exhaustive.rs +++ b/compiler/mono/src/exhaustive.rs @@ -84,11 +84,6 @@ fn simplify<'a>(pattern: &crate::ir::Pattern<'a>) -> Pattern { Ctor(union, tag_id, patterns) } - Shadowed(_region, _ident) => { - // Treat as an Anything - // code-gen will make a runtime error out of the branch - Anything - } UnsupportedPattern(_region) => { // Treat as an Anything // code-gen will make a runtime error out of the branch diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index f8138d74d5..ed4bcbadd7 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -4,7 +4,7 @@ use crate::layout::{Builtin, ClosureLayout, Layout, LayoutCache, LayoutProblem}; use bumpalo::collections::Vec; use bumpalo::Bump; use roc_collections::all::{default_hasher, MutMap, MutSet}; -use roc_module::ident::{ForeignSymbol, Ident, Lowercase, TagName}; +use roc_module::ident::{ForeignSymbol, Lowercase, TagName}; use roc_module::low_level::LowLevel; use roc_module::symbol::{IdentIds, ModuleId, Symbol}; use roc_problem::can::RuntimeError; @@ -1264,7 +1264,22 @@ fn patterns_to_when<'a>( // this must be fixed when moving exhaustiveness checking to the new canonical AST for (pattern_var, pattern) in patterns.into_iter() { let context = crate::exhaustive::Context::BadArg; - let mono_pattern = from_can_pattern(env, layout_cache, &pattern.value); + let mono_pattern = match from_can_pattern(env, layout_cache, &pattern.value) { + Ok(v) => v, + Err(runtime_error) => { + // Even if the body was Ok, replace it with this Err. + // If it was already an Err, leave it at that Err, so the first + // RuntimeError we encountered remains the first. + body = body.and({ + Err(Located { + region: pattern.region, + value: runtime_error, + }) + }); + + continue; + } + }; match crate::exhaustive::check( pattern.region, @@ -2392,7 +2407,14 @@ pub fn with_hole<'a>( } } else { // this may be a destructure pattern - let mono_pattern = from_can_pattern(env, layout_cache, &def.loc_pattern.value); + let mono_pattern = match from_can_pattern(env, layout_cache, &def.loc_pattern.value) + { + Ok(v) => v, + Err(_runtime_error) => { + // todo + panic!(); + } + }; let context = crate::exhaustive::Context::BadDestruct; match crate::exhaustive::check( @@ -3923,7 +3945,10 @@ pub fn from_can<'a>( } // this may be a destructure pattern - let mono_pattern = from_can_pattern(env, layout_cache, &def.loc_pattern.value); + let mono_pattern = match from_can_pattern(env, layout_cache, &def.loc_pattern.value) { + Ok(v) => v, + Err(_) => todo!(), + }; if let Pattern::Identifier(symbol) = mono_pattern { let hole = @@ -4015,19 +4040,34 @@ fn to_opt_branches<'a>( }; for loc_pattern in when_branch.patterns { - let mono_pattern = from_can_pattern(env, layout_cache, &loc_pattern.value); + match from_can_pattern(env, layout_cache, &loc_pattern.value) { + Ok(mono_pattern) => { + loc_branches.push(( + Located::at(loc_pattern.region, mono_pattern.clone()), + exhaustive_guard.clone(), + )); - loc_branches.push(( - Located::at(loc_pattern.region, mono_pattern.clone()), - exhaustive_guard.clone(), - )); + // TODO remove clone? + opt_branches.push(( + mono_pattern, + when_branch.guard.clone(), + when_branch.value.value.clone(), + )); + } + Err(runtime_error) => { + loc_branches.push(( + Located::at(loc_pattern.region, Pattern::Underscore), + exhaustive_guard.clone(), + )); - // TODO remove clone? - opt_branches.push(( - mono_pattern, - when_branch.guard.clone(), - when_branch.value.value.clone(), - )); + // TODO remove clone? + opt_branches.push(( + Pattern::Underscore, + when_branch.guard.clone(), + roc_can::expr::Expr::RuntimeError(runtime_error), + )); + } + } } } @@ -4659,10 +4699,6 @@ fn store_pattern<'a>( unreachable!("a record destructure must always occur on a struct layout"); } - Shadowed(_region, _ident) => { - return Err(&"shadowed"); - } - UnsupportedPattern(_region) => { return Err(&"unsupported pattern"); } @@ -5431,7 +5467,7 @@ pub enum Pattern<'a> { }, // Runtime Exceptions - Shadowed(Region, Located), + // Shadowed(Region, Located), // Example: (5 = 1 + 2) is an unsupported pattern in an assignment; Int patterns aren't allowed in assignments! UnsupportedPattern(Region), } @@ -5463,23 +5499,26 @@ pub fn from_can_pattern<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, can_pattern: &roc_can::pattern::Pattern, -) -> Pattern<'a> { +) -> Result, RuntimeError> { use roc_can::pattern::Pattern::*; match can_pattern { - Underscore => Pattern::Underscore, - Identifier(symbol) => Pattern::Identifier(*symbol), - IntLiteral(v) => Pattern::IntLiteral(*v), - FloatLiteral(v) => Pattern::FloatLiteral(f64::to_bits(*v)), - StrLiteral(v) => Pattern::StrLiteral(v.clone()), - Shadowed(region, ident) => Pattern::Shadowed(*region, ident.clone()), - UnsupportedPattern(region) => Pattern::UnsupportedPattern(*region), + Underscore => Ok(Pattern::Underscore), + Identifier(symbol) => Ok(Pattern::Identifier(*symbol)), + IntLiteral(v) => Ok(Pattern::IntLiteral(*v)), + FloatLiteral(v) => Ok(Pattern::FloatLiteral(f64::to_bits(*v))), + StrLiteral(v) => Ok(Pattern::StrLiteral(v.clone())), + Shadowed(region, ident) => Err(RuntimeError::Shadowing { + original_region: *region, + shadow: ident.clone(), + }), + UnsupportedPattern(region) => Ok(Pattern::UnsupportedPattern(*region)), MalformedPattern(_problem, region) => { // TODO preserve malformed problem information here? - Pattern::UnsupportedPattern(*region) + Ok(Pattern::UnsupportedPattern(*region)) } NumLiteral(var, num) => match num_argument_to_int_or_float(env.subs, *var) { - IntOrFloat::IntType => Pattern::IntLiteral(*num), - IntOrFloat::FloatType => Pattern::FloatLiteral(*num as u64), + IntOrFloat::IntType => Ok(Pattern::IntLiteral(*num)), + IntOrFloat::FloatType => Ok(Pattern::FloatLiteral(*num as u64)), }, AppliedTag { @@ -5493,7 +5532,7 @@ pub fn from_can_pattern<'a>( let variant = crate::layout::union_sorted_tags(env.arena, *whole_var, env.subs); - match variant { + let result = match variant { Never => unreachable!( "there is no pattern of type `[]`, union var {:?}", *whole_var @@ -5582,7 +5621,7 @@ pub fn from_can_pattern<'a>( let mut mono_args = Vec::with_capacity_in(arguments.len(), env.arena); for ((_, loc_pat), layout) in arguments.iter().zip(field_layouts.iter()) { mono_args.push(( - from_can_pattern(env, layout_cache, &loc_pat.value), + from_can_pattern(env, layout_cache, &loc_pat.value)?, layout.clone(), )); } @@ -5642,7 +5681,7 @@ pub fn from_can_pattern<'a>( let it = argument_layouts[1..].iter(); for ((_, loc_pat), layout) in arguments.iter().zip(it) { mono_args.push(( - from_can_pattern(env, layout_cache, &loc_pat.value), + from_can_pattern(env, layout_cache, &loc_pat.value)?, layout.clone(), )); } @@ -5664,7 +5703,9 @@ pub fn from_can_pattern<'a>( layout, } } - } + }; + + Ok(result) } RecordDestructure { @@ -5704,7 +5745,7 @@ pub fn from_can_pattern<'a>( layout_cache, &destruct.value, field_layout.clone(), - )); + )?); } None => { // this field is not destructured by the pattern @@ -5783,10 +5824,10 @@ pub fn from_can_pattern<'a>( } } - Pattern::RecordDestructure( + Ok(Pattern::RecordDestructure( mono_destructs, Layout::Struct(field_layouts.into_bump_slice()), - ) + )) } } } @@ -5796,8 +5837,8 @@ fn from_can_record_destruct<'a>( layout_cache: &mut LayoutCache<'a>, can_rd: &roc_can::pattern::RecordDestruct, field_layout: Layout<'a>, -) -> RecordDestruct<'a> { - RecordDestruct { +) -> Result, RuntimeError> { + Ok(RecordDestruct { label: can_rd.label.clone(), symbol: can_rd.symbol, variable: can_rd.var, @@ -5810,10 +5851,10 @@ fn from_can_record_destruct<'a>( DestructType::Required } roc_can::pattern::DestructType::Guard(_, loc_pattern) => { - DestructType::Guard(from_can_pattern(env, layout_cache, &loc_pattern.value)) + DestructType::Guard(from_can_pattern(env, layout_cache, &loc_pattern.value)?) } }, - } + }) } /// Potentially translate LowLevel operations into more efficient ones based on From fefb1f37393fa847eb9d70085660f03a174b8a32 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 28 Dec 2020 23:22:54 +0100 Subject: [PATCH 16/26] error on non-exhaustive pattern in let --- compiler/gen/tests/gen_primitives.rs | 5 ++--- compiler/mono/src/ir.rs | 5 +++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index d784f8b741..858fd94cc7 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1769,9 +1769,8 @@ mod gen_primitives { } #[test] - #[ignore] - #[should_panic(expected = "foo")] - fn unsupported_pattern_tag() { + #[should_panic(expected = "TODO non-exhaustive pattern")] + fn non_exhaustive_pattern_let() { assert_evals_to!( indoc!( r#" diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index ed4bcbadd7..743bcb5c6c 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -3979,8 +3979,9 @@ pub fn from_can<'a>( for error in errors { env.problems.push(MonoProblem::PatternProblem(error)) } - } // TODO make all variables bound in the pattern evaluate to a runtime error - // return Stmt::RuntimeError("TODO non-exhaustive pattern"); + + return Stmt::RuntimeError("TODO non-exhaustive pattern"); + } } // convert the continuation From f8e04619b83638230bfbcdf2ccb99a4b0c7bb0ed Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 00:48:56 +0100 Subject: [PATCH 17/26] remove UnsupportedPattern variant in mono patterns --- compiler/gen/tests/gen_primitives.rs | 17 +++++++++++++++++ compiler/gen/tests/helpers/eval.rs | 13 ++++++++----- compiler/mono/src/decision_tree.rs | 6 +++--- compiler/mono/src/exhaustive.rs | 6 ------ compiler/mono/src/ir.rs | 14 +++----------- 5 files changed, 31 insertions(+), 25 deletions(-) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index 858fd94cc7..b30b4cfa4e 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1786,4 +1786,21 @@ mod gen_primitives { i64 ); } + + #[test] + #[ignore] + #[should_panic(expected = "")] + fn unsupported_pattern_str_interp() { + assert_evals_to!( + indoc!( + r#" + { x: 4 } = { x : 4 } + + x + "# + ), + 0, + i64 + ); + } } diff --git a/compiler/gen/tests/helpers/eval.rs b/compiler/gen/tests/helpers/eval.rs index 18781cbc98..b6b14883a5 100644 --- a/compiler/gen/tests/helpers/eval.rs +++ b/compiler/gen/tests/helpers/eval.rs @@ -64,11 +64,14 @@ pub fn helper<'a>( debug_assert_eq!(exposed_to_host.len(), 1); let main_fn_symbol = exposed_to_host.keys().copied().nth(0).unwrap(); - let (_, main_fn_layout) = procedures - .keys() - .find(|(s, _)| *s == main_fn_symbol) - .unwrap() - .clone(); + let (_, main_fn_layout) = match procedures.keys().find(|(s, _)| *s == main_fn_symbol) { + Some(found) => found.clone(), + None => panic!( + "The main function symbol {:?} does not have a procedure in {:?}", + main_fn_symbol, + &procedures.keys() + ), + }; let target = target_lexicon::Triple::host(); let ptr_bytes = target.pointer_width().unwrap().bytes() as u32; diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 4983332d9a..15eca105c8 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -379,7 +379,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: &Branch<'a>, all_tests: &mut V match pattern { // TODO use guard! - Identifier(_) | Underscore | UnsupportedPattern(_) => { + Identifier(_) | Underscore => { if let Guard::Guard { symbol, id, stmt } = guard { all_tests.push(Guarded { opt_test: None, @@ -531,7 +531,7 @@ fn to_relevant_branch_help<'a>( use Test::*; match pattern { - Identifier(_) | Underscore | UnsupportedPattern(_) => Some(branch.clone()), + Identifier(_) | Underscore => Some(branch.clone()), RecordDestructure(destructs, _) => match test { IsCtor { @@ -742,7 +742,7 @@ fn needs_tests<'a>(pattern: &Pattern<'a>) -> bool { use Pattern::*; match pattern { - Identifier(_) | Underscore | UnsupportedPattern(_) => false, + Identifier(_) | Underscore => false, RecordDestructure(_, _) | AppliedTag { .. } diff --git a/compiler/mono/src/exhaustive.rs b/compiler/mono/src/exhaustive.rs index 1c253b5a97..a27227e646 100644 --- a/compiler/mono/src/exhaustive.rs +++ b/compiler/mono/src/exhaustive.rs @@ -84,12 +84,6 @@ fn simplify<'a>(pattern: &crate::ir::Pattern<'a>) -> Pattern { Ctor(union, tag_id, patterns) } - UnsupportedPattern(_region) => { - // Treat as an Anything - // code-gen will make a runtime error out of the branch - Anything - } - AppliedTag { tag_id, arguments, diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 743bcb5c6c..eece1217da 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -4699,10 +4699,6 @@ fn store_pattern<'a>( RecordDestructure(_, _) => { unreachable!("a record destructure must always occur on a struct layout"); } - - UnsupportedPattern(_region) => { - return Err(&"unsupported pattern"); - } } Ok(stmt) @@ -5466,11 +5462,6 @@ pub enum Pattern<'a> { layout: Layout<'a>, union: crate::exhaustive::Union, }, - - // Runtime Exceptions - // Shadowed(Region, Located), - // Example: (5 = 1 + 2) is an unsupported pattern in an assignment; Int patterns aren't allowed in assignments! - UnsupportedPattern(Region), } #[derive(Clone, Debug, PartialEq)] @@ -5512,10 +5503,11 @@ pub fn from_can_pattern<'a>( original_region: *region, shadow: ident.clone(), }), - UnsupportedPattern(region) => Ok(Pattern::UnsupportedPattern(*region)), + UnsupportedPattern(region) => Err(RuntimeError::UnsupportedPattern(*region)), + MalformedPattern(_problem, region) => { // TODO preserve malformed problem information here? - Ok(Pattern::UnsupportedPattern(*region)) + Err(RuntimeError::UnsupportedPattern(*region)) } NumLiteral(var, num) => match num_argument_to_int_or_float(env.subs, *var) { IntOrFloat::IntType => Ok(Pattern::IntLiteral(*num)), From 40b514a26d5cf4a25753d1659031aad434a8444e Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 01:04:55 +0100 Subject: [PATCH 18/26] extract optional record field default assignments --- compiler/mono/src/ir.rs | 59 ++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index eece1217da..0ba737ca5a 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5487,12 +5487,27 @@ pub struct WhenBranch<'a> { pub guard: Option>, } -pub fn from_can_pattern<'a>( +fn from_can_pattern<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, can_pattern: &roc_can::pattern::Pattern, + // ) -> Result<(Pattern<'a>, &'a [(Symbol, Layout<'a>, roc_can::expr::Expr)]), RuntimeError> { +) -> Result, RuntimeError> { + let mut assignments = Vec::new_in(env.arena); + let pattern = from_can_pattern_help(env, layout_cache, can_pattern, &mut assignments)?; + + // Ok((pattern, assignments.into_bump_slice())) + Ok(pattern) +} + +fn from_can_pattern_help<'a>( + env: &mut Env<'a, '_>, + layout_cache: &mut LayoutCache<'a>, + can_pattern: &roc_can::pattern::Pattern, + assignments: &mut Vec<'a, (Symbol, Layout<'a>, roc_can::expr::Expr)>, ) -> Result, RuntimeError> { use roc_can::pattern::Pattern::*; + match can_pattern { Underscore => Ok(Pattern::Underscore), Identifier(symbol) => Ok(Pattern::Identifier(*symbol)), @@ -5614,7 +5629,7 @@ pub fn from_can_pattern<'a>( let mut mono_args = Vec::with_capacity_in(arguments.len(), env.arena); for ((_, loc_pat), layout) in arguments.iter().zip(field_layouts.iter()) { mono_args.push(( - from_can_pattern(env, layout_cache, &loc_pat.value)?, + from_can_pattern_help(env, layout_cache, &loc_pat.value, assignments)?, layout.clone(), )); } @@ -5674,7 +5689,7 @@ pub fn from_can_pattern<'a>( let it = argument_layouts[1..].iter(); for ((_, loc_pat), layout) in arguments.iter().zip(it) { mono_args.push(( - from_can_pattern(env, layout_cache, &loc_pat.value)?, + from_can_pattern_help(env, layout_cache, &loc_pat.value, assignments)?, layout.clone(), )); } @@ -5738,6 +5753,7 @@ pub fn from_can_pattern<'a>( layout_cache, &destruct.value, field_layout.clone(), + assignments, )?); } None => { @@ -5761,22 +5777,20 @@ pub fn from_can_pattern<'a>( match destructs_by_label.remove(&label) { Some(destruct) => { // this field is destructured by the pattern - mono_destructs.push(RecordDestruct { - label: destruct.value.label.clone(), - symbol: destruct.value.symbol, - layout: field_layout, - variable, - typ: match &destruct.value.typ { - roc_can::pattern::DestructType::Optional(_, loc_expr) => { - // if we reach this stage, the optional field is not present - // so use the default - DestructType::Optional(loc_expr.value.clone()) - } - _ => unreachable!( - "only optional destructs can be optional fields" - ), - }, - }); + match &destruct.value.typ { + roc_can::pattern::DestructType::Optional(_, loc_expr) => { + // if we reach this stage, the optional field is not present + // so we push the default assignment into the branch + assignments.push(( + destruct.value.symbol, + field_layout, + loc_expr.value.clone(), + )); + } + _ => unreachable!( + "only optional destructs can be optional fields" + ), + }; } None => { // this field is not destructured by the pattern @@ -5830,6 +5844,7 @@ fn from_can_record_destruct<'a>( layout_cache: &mut LayoutCache<'a>, can_rd: &roc_can::pattern::RecordDestruct, field_layout: Layout<'a>, + assignments: &mut Vec<'a, (Symbol, Layout<'a>, roc_can::expr::Expr)>, ) -> Result, RuntimeError> { Ok(RecordDestruct { label: can_rd.label.clone(), @@ -5843,9 +5858,9 @@ fn from_can_record_destruct<'a>( // DestructType::Optional(loc_expr.value.clone()) DestructType::Required } - roc_can::pattern::DestructType::Guard(_, loc_pattern) => { - DestructType::Guard(from_can_pattern(env, layout_cache, &loc_pattern.value)?) - } + roc_can::pattern::DestructType::Guard(_, loc_pattern) => DestructType::Guard( + from_can_pattern_help(env, layout_cache, &loc_pattern.value, assignments)?, + ), }, }) } From edd23dc1d87755ab13278552704657f698915a51 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 01:07:42 +0100 Subject: [PATCH 19/26] expose default assignments --- compiler/mono/src/ir.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 0ba737ca5a..686c3ed2c5 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -1264,7 +1264,8 @@ fn patterns_to_when<'a>( // this must be fixed when moving exhaustiveness checking to the new canonical AST for (pattern_var, pattern) in patterns.into_iter() { let context = crate::exhaustive::Context::BadArg; - let mono_pattern = match from_can_pattern(env, layout_cache, &pattern.value) { + let (mono_pattern, assignments) = match from_can_pattern(env, layout_cache, &pattern.value) + { Ok(v) => v, Err(runtime_error) => { // Even if the body was Ok, replace it with this Err. @@ -2407,14 +2408,14 @@ pub fn with_hole<'a>( } } else { // this may be a destructure pattern - let mono_pattern = match from_can_pattern(env, layout_cache, &def.loc_pattern.value) - { - Ok(v) => v, - Err(_runtime_error) => { - // todo - panic!(); - } - }; + let (mono_pattern, assignments) = + match from_can_pattern(env, layout_cache, &def.loc_pattern.value) { + Ok(v) => v, + Err(_runtime_error) => { + // todo + panic!(); + } + }; let context = crate::exhaustive::Context::BadDestruct; match crate::exhaustive::check( @@ -3945,10 +3946,11 @@ pub fn from_can<'a>( } // this may be a destructure pattern - let mono_pattern = match from_can_pattern(env, layout_cache, &def.loc_pattern.value) { - Ok(v) => v, - Err(_) => todo!(), - }; + let (mono_pattern, assignments) = + match from_can_pattern(env, layout_cache, &def.loc_pattern.value) { + Ok(v) => v, + Err(_) => todo!(), + }; if let Pattern::Identifier(symbol) = mono_pattern { let hole = @@ -4042,7 +4044,7 @@ fn to_opt_branches<'a>( for loc_pattern in when_branch.patterns { match from_can_pattern(env, layout_cache, &loc_pattern.value) { - Ok(mono_pattern) => { + Ok((mono_pattern, assignments)) => { loc_branches.push(( Located::at(loc_pattern.region, mono_pattern.clone()), exhaustive_guard.clone(), @@ -5491,13 +5493,11 @@ fn from_can_pattern<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, can_pattern: &roc_can::pattern::Pattern, - // ) -> Result<(Pattern<'a>, &'a [(Symbol, Layout<'a>, roc_can::expr::Expr)]), RuntimeError> { -) -> Result, RuntimeError> { +) -> Result<(Pattern<'a>, &'a [(Symbol, Layout<'a>, roc_can::expr::Expr)]), RuntimeError> { let mut assignments = Vec::new_in(env.arena); let pattern = from_can_pattern_help(env, layout_cache, can_pattern, &mut assignments)?; - // Ok((pattern, assignments.into_bump_slice())) - Ok(pattern) + Ok((pattern, assignments.into_bump_slice())) } fn from_can_pattern_help<'a>( From a7efffa5423d46b710d53141e9d245e857c3d940 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 01:48:31 +0100 Subject: [PATCH 20/26] comment out tests that are blocked on a mono issue (Issue 786) --- compiler/gen/tests/gen_records.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/gen/tests/gen_records.rs b/compiler/gen/tests/gen_records.rs index b744222235..bed2cad4ea 100644 --- a/compiler/gen/tests/gen_records.rs +++ b/compiler/gen/tests/gen_records.rs @@ -561,7 +561,9 @@ mod gen_records { } #[test] + #[ignore] fn optional_field_function_no_use_default() { + // blocked on https://github.com/rtfeldman/roc/issues/786 assert_evals_to!( indoc!( r#" @@ -579,7 +581,9 @@ mod gen_records { } #[test] + #[ignore] fn optional_field_function_no_use_default_nested() { + // blocked on https://github.com/rtfeldman/roc/issues/786 assert_evals_to!( indoc!( r#" From eb501f90a21771a3733f4b6267ceeeb5e33c0ee7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 01:49:15 +0100 Subject: [PATCH 21/26] push optional field assignments into the branch --- compiler/mono/src/ir.rs | 98 +++++++++++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 14 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 686c3ed2c5..1f45b5fcf0 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -1264,9 +1264,36 @@ fn patterns_to_when<'a>( // this must be fixed when moving exhaustiveness checking to the new canonical AST for (pattern_var, pattern) in patterns.into_iter() { let context = crate::exhaustive::Context::BadArg; - let (mono_pattern, assignments) = match from_can_pattern(env, layout_cache, &pattern.value) - { - Ok(v) => v, + let mono_pattern = match from_can_pattern(env, layout_cache, &pattern.value) { + Ok((pat, assignments)) => { + for (symbol, variable, expr) in assignments.into_iter().rev() { + if let Ok(old_body) = body { + let def = roc_can::def::Def { + annotation: None, + expr_var: variable, + loc_expr: Located::at(pattern.region, expr), + loc_pattern: Located::at( + pattern.region, + roc_can::pattern::Pattern::Identifier(symbol), + ), + pattern_vars: std::iter::once((symbol, variable)).collect(), + }; + let new_expr = roc_can::expr::Expr::LetNonRec( + Box::new(def), + Box::new(old_body), + variable, + ); + let new_body = Located { + region: pattern.region, + value: new_expr, + }; + + body = Ok(new_body); + } + } + + pat + } Err(runtime_error) => { // Even if the body was Ok, replace it with this Err. // If it was already an Err, leave it at that Err, so the first @@ -2435,6 +2462,14 @@ pub fn with_hole<'a>( // return Stmt::RuntimeError("TODO non-exhaustive pattern"); } + let mut hole = hole; + + for (symbol, variable, expr) in assignments { + let stmt = with_hole(env, expr, variable, procs, layout_cache, symbol, hole); + + hole = env.arena.alloc(stmt); + } + // convert the continuation let mut stmt = with_hole( env, @@ -3953,10 +3988,16 @@ pub fn from_can<'a>( }; if let Pattern::Identifier(symbol) = mono_pattern { - let hole = + let mut hole = env.arena .alloc(from_can(env, variable, cont.value, procs, layout_cache)); + for (symbol, variable, expr) in assignments { + let stmt = with_hole(env, expr, variable, procs, layout_cache, symbol, hole); + + hole = env.arena.alloc(stmt); + } + with_hole( env, def.loc_expr.value, @@ -3989,6 +4030,12 @@ pub fn from_can<'a>( // convert the continuation let mut stmt = from_can(env, variable, cont.value, procs, layout_cache); + // layer on any default record fields + for (symbol, variable, expr) in assignments { + let hole = env.arena.alloc(stmt); + stmt = with_hole(env, expr, variable, procs, layout_cache, symbol, hole); + } + if let roc_can::expr::Expr::Var(outer_symbol) = def.loc_expr.value { store_pattern(env, procs, layout_cache, &mono_pattern, outer_symbol, stmt) .unwrap() @@ -4050,12 +4097,29 @@ fn to_opt_branches<'a>( exhaustive_guard.clone(), )); + let mut loc_expr = when_branch.value.clone(); + let region = loc_pattern.region; + for (symbol, variable, expr) in assignments.into_iter().rev() { + let def = roc_can::def::Def { + annotation: None, + expr_var: variable, + loc_expr: Located::at(region, expr), + loc_pattern: Located::at( + region, + roc_can::pattern::Pattern::Identifier(symbol), + ), + pattern_vars: std::iter::once((symbol, variable)).collect(), + }; + let new_expr = roc_can::expr::Expr::LetNonRec( + Box::new(def), + Box::new(loc_expr), + variable, + ); + loc_expr = Located::at(region, new_expr); + } + // TODO remove clone? - opt_branches.push(( - mono_pattern, - when_branch.guard.clone(), - when_branch.value.value.clone(), - )); + opt_branches.push((mono_pattern, when_branch.guard.clone(), loc_expr.value)); } Err(runtime_error) => { loc_branches.push(( @@ -5493,18 +5557,24 @@ fn from_can_pattern<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, can_pattern: &roc_can::pattern::Pattern, -) -> Result<(Pattern<'a>, &'a [(Symbol, Layout<'a>, roc_can::expr::Expr)]), RuntimeError> { +) -> Result< + ( + Pattern<'a>, + Vec<'a, (Symbol, Variable, roc_can::expr::Expr)>, + ), + RuntimeError, +> { let mut assignments = Vec::new_in(env.arena); let pattern = from_can_pattern_help(env, layout_cache, can_pattern, &mut assignments)?; - Ok((pattern, assignments.into_bump_slice())) + Ok((pattern, assignments)) } fn from_can_pattern_help<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, can_pattern: &roc_can::pattern::Pattern, - assignments: &mut Vec<'a, (Symbol, Layout<'a>, roc_can::expr::Expr)>, + assignments: &mut Vec<'a, (Symbol, Variable, roc_can::expr::Expr)>, ) -> Result, RuntimeError> { use roc_can::pattern::Pattern::*; @@ -5783,7 +5853,7 @@ fn from_can_pattern_help<'a>( // so we push the default assignment into the branch assignments.push(( destruct.value.symbol, - field_layout, + variable, loc_expr.value.clone(), )); } @@ -5844,7 +5914,7 @@ fn from_can_record_destruct<'a>( layout_cache: &mut LayoutCache<'a>, can_rd: &roc_can::pattern::RecordDestruct, field_layout: Layout<'a>, - assignments: &mut Vec<'a, (Symbol, Layout<'a>, roc_can::expr::Expr)>, + assignments: &mut Vec<'a, (Symbol, Variable, roc_can::expr::Expr)>, ) -> Result, RuntimeError> { Ok(RecordDestruct { label: can_rd.label.clone(), From b9f92851a4f771f67a0c96b21acfc5095090dddc Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 02:10:27 +0100 Subject: [PATCH 22/26] remove optional fields destruct in mono pattern --- compiler/mono/src/decision_tree.rs | 38 +++++++++++---------------- compiler/mono/src/exhaustive.rs | 2 +- compiler/mono/src/ir.rs | 42 ++++++++++++------------------ 3 files changed, 32 insertions(+), 50 deletions(-) diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 15eca105c8..4bb8d08d09 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -411,9 +411,6 @@ fn test_at_path<'a>(selected_path: &Path, branch: &Branch<'a>, all_tests: &mut V DestructType::Required => { arguments.push((Pattern::Underscore, destruct.layout.clone())); } - DestructType::Optional(_expr) => { - // do nothing - } } } @@ -540,27 +537,22 @@ fn to_relevant_branch_help<'a>( .. } => { debug_assert!(test_name == &TagName::Global(RECORD_TAG_NAME.into())); - let sub_positions = destructs - .into_iter() - .filter(|destruct| !matches!(destruct.typ, DestructType::Optional(_))) - .enumerate() - .map(|(index, destruct)| { - let pattern = match destruct.typ { - DestructType::Guard(guard) => guard.clone(), - DestructType::Required => Pattern::Underscore, - DestructType::Optional(_expr) => unreachable!("because of the filter"), - }; + let sub_positions = destructs.into_iter().enumerate().map(|(index, destruct)| { + let pattern = match destruct.typ { + DestructType::Guard(guard) => guard.clone(), + DestructType::Required => Pattern::Underscore, + }; - ( - Path::Index { - index: index as u64, - tag_id: *tag_id, - path: Box::new(path.clone()), - }, - Guard::NoGuard, - pattern, - ) - }); + ( + Path::Index { + index: index as u64, + tag_id: *tag_id, + path: Box::new(path.clone()), + }, + Guard::NoGuard, + pattern, + ) + }); start.extend(sub_positions); start.extend(end); diff --git a/compiler/mono/src/exhaustive.rs b/compiler/mono/src/exhaustive.rs index a27227e646..51b61a4c07 100644 --- a/compiler/mono/src/exhaustive.rs +++ b/compiler/mono/src/exhaustive.rs @@ -67,7 +67,7 @@ fn simplify<'a>(pattern: &crate::ir::Pattern<'a>) -> Pattern { field_names.push(destruct.label.clone()); match &destruct.typ { - DestructType::Required | DestructType::Optional(_) => patterns.push(Anything), + DestructType::Required => patterns.push(Anything), DestructType::Guard(guard) => patterns.push(simplify(guard)), } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 1f45b5fcf0..de8b90a9f2 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -4802,17 +4802,6 @@ fn store_record_destruct<'a>( env.arena.alloc(stmt), ); } - DestructType::Optional(expr) => { - stmt = with_hole( - env, - expr.clone(), - destruct.variable, - procs, - layout_cache, - destruct.symbol, - env.arena.alloc(stmt), - ); - } DestructType::Guard(guard_pattern) => match &guard_pattern { Identifier(symbol) => { stmt = Stmt::Let( @@ -5535,14 +5524,13 @@ pub struct RecordDestruct<'a> { pub label: Lowercase, pub variable: Variable, pub layout: Layout<'a>, - pub symbol: Symbol, pub typ: DestructType<'a>, + pub symbol: Symbol, } #[derive(Clone, Debug, PartialEq)] pub enum DestructType<'a> { Required, - Optional(roc_can::expr::Expr), Guard(Pattern<'a>), } @@ -5883,19 +5871,21 @@ fn from_can_pattern_help<'a>( // it must be an optional field, and we will use the default match &destruct.value.typ { roc_can::pattern::DestructType::Optional(field_var, loc_expr) => { - let field_layout = layout_cache - .from_var(env.arena, *field_var, env.subs) - .unwrap_or_else(|err| { - panic!("TODO turn fn_var into a RuntimeError {:?}", err) - }); - - mono_destructs.push(RecordDestruct { - label: destruct.value.label.clone(), - symbol: destruct.value.symbol, - variable: destruct.value.var, - layout: field_layout, - typ: DestructType::Optional(loc_expr.value.clone()), - }) + // TODO these don't match up in the uniqueness inference; when we remove + // that, reinstate this assert! + // + // dbg!(&env.subs.get_without_compacting(*field_var).content); + // dbg!(&env.subs.get_without_compacting(destruct.value.var).content); + // debug_assert_eq!( + // env.subs.get_root_key_without_compacting(*field_var), + // env.subs.get_root_key_without_compacting(destruct.value.var) + // ); + assignments.push(( + destruct.value.symbol, + // destruct.value.var, + *field_var, + loc_expr.value.clone(), + )); } _ => unreachable!("only optional destructs can be optional fields"), } From 8438b91633e2a174b5c71b7cc3777626b2c62962 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 02:16:21 +0100 Subject: [PATCH 23/26] move the symbol field into the Required tag --- compiler/mono/src/decision_tree.rs | 4 ++-- compiler/mono/src/exhaustive.rs | 2 +- compiler/mono/src/ir.rs | 15 +++++---------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 4bb8d08d09..2f8580bf67 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -408,7 +408,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: &Branch<'a>, all_tests: &mut V DestructType::Guard(guard) => { arguments.push((guard.clone(), destruct.layout.clone())); } - DestructType::Required => { + DestructType::Required(_) => { arguments.push((Pattern::Underscore, destruct.layout.clone())); } } @@ -540,7 +540,7 @@ fn to_relevant_branch_help<'a>( let sub_positions = destructs.into_iter().enumerate().map(|(index, destruct)| { let pattern = match destruct.typ { DestructType::Guard(guard) => guard.clone(), - DestructType::Required => Pattern::Underscore, + DestructType::Required(_) => Pattern::Underscore, }; ( diff --git a/compiler/mono/src/exhaustive.rs b/compiler/mono/src/exhaustive.rs index 51b61a4c07..018c448098 100644 --- a/compiler/mono/src/exhaustive.rs +++ b/compiler/mono/src/exhaustive.rs @@ -67,7 +67,7 @@ fn simplify<'a>(pattern: &crate::ir::Pattern<'a>) -> Pattern { field_names.push(destruct.label.clone()); match &destruct.typ { - DestructType::Required => patterns.push(Anything), + DestructType::Required(_) => patterns.push(Anything), DestructType::Guard(guard) => patterns.push(simplify(guard)), } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index de8b90a9f2..6ebd2dad0a 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -4794,9 +4794,9 @@ fn store_record_destruct<'a>( }; match &destruct.typ { - DestructType::Required => { + DestructType::Required(symbol) => { stmt = Stmt::Let( - destruct.symbol, + *symbol, load, destruct.layout.clone(), env.arena.alloc(stmt), @@ -5525,12 +5525,11 @@ pub struct RecordDestruct<'a> { pub variable: Variable, pub layout: Layout<'a>, pub typ: DestructType<'a>, - pub symbol: Symbol, } #[derive(Clone, Debug, PartialEq)] pub enum DestructType<'a> { - Required, + Required(Symbol), Guard(Pattern<'a>), } @@ -5819,7 +5818,6 @@ fn from_can_pattern_help<'a>( // put in an underscore mono_destructs.push(RecordDestruct { label: label.clone(), - symbol: env.unique_symbol(), variable, layout: field_layout.clone(), typ: DestructType::Guard(Pattern::Underscore), @@ -5855,7 +5853,6 @@ fn from_can_pattern_help<'a>( // put in an underscore mono_destructs.push(RecordDestruct { label: label.clone(), - symbol: env.unique_symbol(), variable, layout: field_layout.clone(), typ: DestructType::Guard(Pattern::Underscore), @@ -5908,15 +5905,13 @@ fn from_can_record_destruct<'a>( ) -> Result, RuntimeError> { Ok(RecordDestruct { label: can_rd.label.clone(), - symbol: can_rd.symbol, variable: can_rd.var, layout: field_layout, typ: match &can_rd.typ { - roc_can::pattern::DestructType::Required => DestructType::Required, + roc_can::pattern::DestructType::Required => DestructType::Required(can_rd.symbol), roc_can::pattern::DestructType::Optional(_, _) => { // if we reach this stage, the optional field is present - // DestructType::Optional(loc_expr.value.clone()) - DestructType::Required + DestructType::Required(can_rd.symbol) } roc_can::pattern::DestructType::Guard(_, loc_pattern) => DestructType::Guard( from_can_pattern_help(env, layout_cache, &loc_pattern.value, assignments)?, From a5af8178a26ebd33862d1eb489ca99b791c31c2e Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 02:37:16 +0100 Subject: [PATCH 24/26] clippy --- compiler/mono/src/ir.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 6ebd2dad0a..231901d73b 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5540,6 +5540,7 @@ pub struct WhenBranch<'a> { pub guard: Option>, } +#[allow(clippy::type_complexity)] fn from_can_pattern<'a>( env: &mut Env<'a, '_>, layout_cache: &mut LayoutCache<'a>, From 5e25e28033819fcbb05687185ba37fb0eab13561 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 29 Dec 2020 03:51:46 +0100 Subject: [PATCH 25/26] defer mono errors so we can test that non-exhaustive patterns throw a runtime exception --- compiler/gen/src/run_roc.rs | 11 +++-------- compiler/gen/tests/helpers/eval.rs | 8 ++++---- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/compiler/gen/src/run_roc.rs b/compiler/gen/src/run_roc.rs index 024a5198ca..f5e75c4b1a 100644 --- a/compiler/gen/src/run_roc.rs +++ b/compiler/gen/src/run_roc.rs @@ -29,7 +29,7 @@ impl Into> for RocCallResult { #[macro_export] macro_rules! run_jit_function { ($lib: expr, $main_fn_name: expr, $ty:ty, $transform:expr) => {{ - let v: std::vec::Vec = std::vec::Vec::new(); + let v: String = String::new(); run_jit_function!($lib, $main_fn_name, $ty, $transform, v) }}; @@ -52,12 +52,7 @@ macro_rules! run_jit_function { match result.assume_init().into() { Ok(success) => { // only if there are no exceptions thrown, check for errors - assert_eq!( - $errors, - std::vec::Vec::new(), - "Encountered errors: {:?}", - $errors - ); + assert!($errors.is_empty(), "Encountered errors:\n{}", $errors); $transform(success) } @@ -73,7 +68,7 @@ macro_rules! run_jit_function { #[macro_export] macro_rules! run_jit_function_dynamic_type { ($lib: expr, $main_fn_name: expr, $bytes:expr, $transform:expr) => {{ - let v: std::vec::Vec = std::vec::Vec::new(); + let v: String = String::new(); run_jit_function_dynamic_type!($lib, $main_fn_name, $bytes, $transform, v) }}; diff --git a/compiler/gen/tests/helpers/eval.rs b/compiler/gen/tests/helpers/eval.rs index b6b14883a5..eeb7117e13 100644 --- a/compiler/gen/tests/helpers/eval.rs +++ b/compiler/gen/tests/helpers/eval.rs @@ -22,7 +22,7 @@ pub fn helper<'a>( stdlib: roc_builtins::std::StdLib, leak: bool, context: &'a inkwell::context::Context, -) -> (&'static str, Vec, Library) { +) -> (&'static str, String, Library) { use roc_gen::llvm::build::{build_proc, build_proc_header, Scope}; use std::path::{Path, PathBuf}; @@ -111,13 +111,12 @@ pub fn helper<'a>( | RuntimeError(_) | UnsupportedPattern(_, _) | ExposedButNotDefined(_) => { - delayed_errors.push(problem.clone()); - let report = can_problem(&alloc, module_path.clone(), problem); let mut buf = String::new(); report.render_color_terminal(&mut buf, &alloc, &palette); + delayed_errors.push(buf.clone()); lines.push(buf); } _ => { @@ -146,6 +145,7 @@ pub fn helper<'a>( report.render_color_terminal(&mut buf, &alloc, &palette); + delayed_errors.push(buf.clone()); lines.push(buf); } } @@ -287,7 +287,7 @@ pub fn helper<'a>( let lib = module_to_dylib(&env.module, &target, opt_level) .expect("Error loading compiled dylib for test"); - (main_fn_name, delayed_errors, lib) + (main_fn_name, delayed_errors.join("\n"), lib) } // TODO this is almost all code duplication with assert_llvm_evals_to From 8937a5d87272a1f4d4c504ed36a35d560a3dc00c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 30 Dec 2020 08:20:47 -0500 Subject: [PATCH 26/26] Fix test_mono tests --- compiler/mono/tests/test_mono.rs | 36 +++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index be4bdc2950..5cdd8ce616 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -251,9 +251,9 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.5 = 1i64; - let Test.6 = 3.14f64; - let Test.2 = Struct {Test.5, Test.6}; + let Test.4 = 1i64; + let Test.5 = 3.14f64; + let Test.2 = Struct {Test.4, Test.5}; let Test.1 = Index 0 Test.2; ret Test.1; "# @@ -853,10 +853,10 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.5 = 2i64; - let Test.6 = 3.14f64; - let Test.4 = Struct {Test.5, Test.6}; - let Test.1 = Index 0 Test.4; + let Test.4 = 2i64; + let Test.5 = 3.14f64; + let Test.3 = Struct {Test.4, Test.5}; + let Test.1 = Index 0 Test.3; ret Test.1; "# ), @@ -874,15 +874,15 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.7 = 1i64; - let Test.8 = 3i64; - let Test.9 = 4i64; - let Test.5 = Array [Test.7, Test.8, Test.9]; - let Test.6 = 3.14f64; - let Test.4 = Struct {Test.5, Test.6}; - let Test.1 = Index 0 Test.4; + let Test.6 = 1i64; + let Test.7 = 3i64; + let Test.8 = 4i64; + let Test.4 = Array [Test.6, Test.7, Test.8]; + let Test.5 = 3.14f64; + let Test.3 = Struct {Test.4, Test.5}; + let Test.1 = Index 0 Test.3; inc Test.1; - dec Test.4; + dec Test.3; ret Test.1; "# ), @@ -1096,8 +1096,8 @@ mod test_mono { ret Test.8; procedure Test.1 (Test.2): + let Test.4 = Index 0 Test.2; let Test.3 = 10i64; - let Test.4 = Index 1 Test.2; let Test.7 = CallByName Num.24 Test.3 Test.4; ret Test.7; @@ -1131,6 +1131,7 @@ mod test_mono { procedure Test.1 (Test.4): let Test.2 = Index 0 Test.4; let Test.3 = Index 1 Test.4; + let Test.2 = 10i64; let Test.7 = CallByName Num.24 Test.2 Test.3; ret Test.7; @@ -1163,8 +1164,9 @@ mod test_mono { ret Test.8; procedure Test.1 (Test.4): + let Test.3 = Index 0 Test.4; + let Test.2 = 10i64; let Test.2 = 10i64; - let Test.3 = Index 1 Test.4; let Test.7 = CallByName Num.24 Test.2 Test.3; ret Test.7;