From 4c9f2c1b6e5c1cea0163c7898e48e090b3aa3729 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 11 Mar 2020 15:24:44 +0100 Subject: [PATCH 01/31] monomorphize addition --- compiler/module/src/symbol.rs | 2 ++ compiler/mono/src/expr.rs | 18 ++++++++++--- compiler/mono/tests/test_mono.rs | 43 ++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index dea7b585d6..4ab921501f 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -575,6 +575,7 @@ define_builtins! { 4 INT_MOD: "mod" 5 INT_HIGHEST: "highest" 6 INT_LOWEST: "lowest" + 7 INT_ADD: "add" } 3 FLOAT: "Float" => { 0 FLOAT_FLOAT: "Float" imported // the Float.Float type alias @@ -585,6 +586,7 @@ define_builtins! { 5 FLOAT_SQRT: "sqrt" 6 FLOAT_HIGHEST: "highest" 7 FLOAT_LOWEST: "lowest" + 8 FLOAT_ADD: "add" } 4 BOOL: "Bool" => { 0 BOOL_BOOL: "Bool" imported // the Bool.Bool type alias diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index d9b6c9a928..a45af92586 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -264,13 +264,23 @@ fn from_can<'a>( } Call(boxed, loc_args, _) => { - let (fn_var, loc_expr, _) = *boxed; + let (fn_var, loc_expr, ret_var) = *boxed; + + let specialize_builtin_functions = { + |symbol, subs: &Subs| match dbg!(symbol) { + Symbol::NUM_ADD => match to_int_or_float(subs, ret_var) { + IntOrFloat::FloatType => Symbol::FLOAT_ADD, + IntOrFloat::IntType => Symbol::INT_ADD, + }, + _ => symbol, + } + }; match from_can(env, loc_expr.value, procs, None) { Expr::Load(proc_name) => { // Some functions can potentially mutate in-place. // If we have one of those, switch to the in-place version if appropriate. - match proc_name { + match specialize_builtin_functions(proc_name, &env.subs) { Symbol::LIST_SET => { let subs = &env.subs; // The first arg is the one with the List in it. @@ -302,7 +312,9 @@ fn from_can<'a>( _ => call_by_name(env, procs, proc_name, loc_args), } } - _ => call_by_name(env, procs, proc_name, loc_args), + specialized_proc_symbol => { + call_by_name(env, procs, specialized_proc_symbol, loc_args) + } } } ptr => { diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 04e8156933..47a7efedce 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -78,6 +78,49 @@ mod test_mono { compiles_to("0.5", Float(0.5)); } + #[test] + fn float_addition() { + compiles_to( + "3.0 + 4", + CallByName( + Symbol::FLOAT_ADD, + &[ + (Float(3.0), Layout::Builtin(Builtin::Float64)), + (Float(4.0), Layout::Builtin(Builtin::Float64)), + ], + ), + ); + } + + #[test] + fn int_addition() { + compiles_to( + "0xDEADBEEF + 4", + CallByName( + Symbol::INT_ADD, + &[ + (Int(3735928559), Layout::Builtin(Builtin::Int64)), + (Int(4), Layout::Builtin(Builtin::Int64)), + ], + ), + ); + } + + #[test] + fn num_addition() { + // Default to Int for `Num *` + compiles_to( + "3 + 5", + CallByName( + Symbol::INT_ADD, + &[ + (Int(3), Layout::Builtin(Builtin::Int64)), + (Int(5), Layout::Builtin(Builtin::Int64)), + ], + ), + ); + } + #[test] fn bool_literal() { let arena = Bump::new(); From 5c9cf0ef3773ef4186316e11dadd501cbb082104 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 11 Mar 2020 22:01:56 +0100 Subject: [PATCH 02/31] add type hash function --- compiler/types/src/pretty_print.rs | 6 +-- compiler/types/src/subs.rs | 85 +++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/compiler/types/src/pretty_print.rs b/compiler/types/src/pretty_print.rs index ad89737b40..cf5e8a3588 100644 --- a/compiler/types/src/pretty_print.rs +++ b/compiler/types/src/pretty_print.rs @@ -504,13 +504,13 @@ fn write_flat_type( } } -fn chase_ext_tag_union( - subs: &mut Subs, +pub fn chase_ext_tag_union( + subs: &Subs, var: Variable, fields: &mut Vec<(TagName, Vec)>, ) -> Option { use FlatType::*; - match subs.get(var).content { + match subs.get_without_compacting(var).content { Content::Structure(EmptyTagUnion) => None, Content::Structure(TagUnion(tags, ext_var)) | Content::Structure(RecursiveTagUnion(_, tags, ext_var)) => { diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 9ac431e55c..44cfa0ea84 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -42,7 +42,7 @@ struct NameState { normals: u32, } -#[derive(Default)] +#[derive(Default, Clone)] pub struct Subs { utable: UnificationTable>, } @@ -542,6 +542,89 @@ pub enum FlatType { Boolean(boolean_algebra::Bool), } +#[derive(Clone, Debug, Hash, PartialEq, Eq, Copy)] +pub struct ContentHash(u64); + +impl ContentHash { + pub fn from_var(var: Variable, subs: &Subs) -> Self { + use std::hash::Hasher; + + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + Self::from_var_help(var, subs, &mut hasher); + + ContentHash(hasher.finish()) + } + + pub fn from_var_help(var: Variable, subs: &Subs, hasher: &mut T) + where + T: std::hash::Hasher, + { + Self::from_content_help(&subs.get_without_compacting(var).content, subs, hasher) + } + + pub fn from_content_help(content: &Content, subs: &Subs, hasher: &mut T) + where + T: std::hash::Hasher, + { + match content { + Content::Alias(_, _, actual) => Self::from_var_help(*actual, subs, hasher), + Content::Structure(flat_type) => { + hasher.write_u8(0x10); + Self::from_flat_type_help(flat_type, subs, hasher) + } + Content::FlexVar(_) | Content::RigidVar(_) => { + hasher.write_u8(0x11); + } + Content::Error => { + hasher.write_u8(0x12); + } + } + } + + pub fn from_flat_type_help(flat_type: &FlatType, subs: &Subs, hasher: &mut T) + where + T: std::hash::Hasher, + { + use std::hash::Hash; + + match flat_type { + FlatType::Func(arguments, ret) => { + hasher.write_u8(0); + + for var in arguments { + Self::from_var_help(*var, subs, hasher); + } + + Self::from_var_help(*ret, subs, hasher); + } + + FlatType::TagUnion(tags, ext) => { + hasher.write_u8(1); + + // We have to sort by the key, so this clone seems to be required + let mut tag_vec = Vec::with_capacity(tags.len()); + tag_vec.extend(tags.clone().into_iter()); + + match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { + Some(_) => panic!("Tag union with non-empty ext var"), + None => { + tag_vec.sort(); + for (name, arguments) in tag_vec { + name.hash(hasher); + + for var in arguments { + Self::from_var_help(var, subs, hasher); + } + } + } + } + } + + _ => todo!(), + } + } +} + #[derive(PartialEq, Eq, Debug, Clone, Copy)] pub enum Builtin { Str, From f7a2be113ebf568053fc5a71011b9c097fdb8a67 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 11 Mar 2020 22:03:01 +0100 Subject: [PATCH 03/31] monomorphize closures --- compiler/mono/src/expr.rs | 169 ++++++++++++++++++++++++++++--- compiler/mono/tests/test_mono.rs | 38 +++++++ 2 files changed, 192 insertions(+), 15 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index a45af92586..0554575e0e 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -7,12 +7,20 @@ use roc_collections::all::MutMap; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::{IdentIds, ModuleId, Symbol}; use roc_region::all::Located; -use roc_types::subs::{Content, FlatType, Subs, Variable}; +use roc_types::subs::{Content, ContentHash, FlatType, Subs, Variable}; -pub type Procs<'a> = MutMap>>; +pub type Procs<'a> = MutMap>; + +#[derive(Clone, Debug, PartialEq)] +pub struct PartialProc<'a> { + pub annotation: Variable, + pub body: roc_can::expr::Expr, + pub specializations: MutMap>)>, +} #[derive(Clone, Debug, PartialEq)] pub struct Proc<'a> { + pub name: Symbol, pub args: &'a [(Layout<'a>, Symbol)], pub body: Expr<'a>, pub closes_over: Layout<'a>, @@ -255,19 +263,29 @@ fn from_can<'a>( Expr::Store(stored.into_bump_slice(), arena.alloc(ret)) } - Closure(_, _, _, loc_args, boxed_body) => { - let (loc_body, ret_var) = *boxed_body; + Closure(annotation, _, _, _loc_args, boxed_body) => { + let (loc_body, _ret_var) = *boxed_body; let symbol = name.unwrap_or_else(|| gen_closure_name(procs, &mut env.ident_ids, env.home)); - add_closure(env, symbol, loc_body.value, ret_var, &loc_args, procs) + procs.insert( + symbol, + PartialProc { + annotation, + body: loc_body.value, + specializations: MutMap::default(), + }, + ); + + // add_closure(env, symbol, loc_body.value, ret_var, &loc_args, procs) + Expr::FunctionPointer(symbol) } Call(boxed, loc_args, _) => { let (fn_var, loc_expr, ret_var) = *boxed; let specialize_builtin_functions = { - |symbol, subs: &Subs| match dbg!(symbol) { + |symbol, subs: &Subs| match symbol { Symbol::NUM_ADD => match to_int_or_float(subs, ret_var) { IntOrFloat::FloatType => Symbol::FLOAT_ADD, IntOrFloat::IntType => Symbol::INT_ADD, @@ -307,14 +325,19 @@ fn from_can<'a>( Symbol::LIST_SET }; - call_by_name(env, procs, new_name, loc_args) + call_by_name(env, procs, fn_var, ret_var, new_name, loc_args) } - _ => call_by_name(env, procs, proc_name, loc_args), + _ => call_by_name(env, procs, fn_var, ret_var, proc_name, loc_args), } } - specialized_proc_symbol => { - call_by_name(env, procs, specialized_proc_symbol, loc_args) - } + specialized_proc_symbol => call_by_name( + env, + procs, + fn_var, + ret_var, + specialized_proc_symbol, + loc_args, + ), } } ptr => { @@ -466,6 +489,7 @@ fn from_can<'a>( } } +/* fn add_closure<'a>( env: &mut Env<'a, '_>, symbol: Symbol, @@ -513,6 +537,7 @@ fn add_closure<'a>( Expr::FunctionPointer(symbol) } +*/ fn store_pattern<'a>( env: &mut Env<'a, '_>, @@ -819,19 +844,133 @@ fn from_can_when<'a>( fn call_by_name<'a>( env: &mut Env<'a, '_>, procs: &mut Procs<'a>, + fn_var: Variable, + ret_var: Variable, proc_name: Symbol, loc_args: std::vec::Vec<(Variable, Located)>, ) -> Expr<'a> { + // create specialized procedure to call + + // TODO this seems very wrong! + // something with mutable and immutable borrow of procs + let procs_clone = procs.clone(); + let op_partial_proc = procs_clone.get(&proc_name); + + let specialized_proc_name = if let Some(partial_proc) = op_partial_proc { + let content_hash = ContentHash::from_var(fn_var, env.subs); + + if let Some(specialization) = partial_proc.specializations.get(&content_hash) { + // a specialization with this type hash already exists, use its symbol + specialization.0 + } else { + // generate a symbol for this specialization + let spec_proc_name = gen_closure_name(procs, &mut env.ident_ids, env.home); + + // register proc, so specialization doesn't loop infinitely + // for recursive definitions + let mut temp = partial_proc.clone(); + temp.specializations + .insert(content_hash, (spec_proc_name, None)); + procs.insert(proc_name, temp); + + let proc = specialize_proc_body( + env, + procs, + fn_var, + ret_var, + spec_proc_name, + &loc_args, + partial_proc, + ); + + // we must be careful here, the specialization could have added different + // specializations of proc_name, so we must get the partial_proc again! + procs.get_mut(&proc_name).map(|partial_proc| { + partial_proc + .specializations + .insert(content_hash, (spec_proc_name, proc)) + }); + + spec_proc_name + } + } else { + // This happens for built-in symbols (they are never defined as a Closure) + proc_name + }; + + // generate actual call let mut args = Vec::with_capacity_in(loc_args.len(), env.arena); - let subs = env.subs; - let arena = env.arena; for (var, loc_arg) in loc_args { - let layout = Layout::from_var(arena, var, subs, env.pointer_size) + let layout = Layout::from_var(&env.arena, var, &env.subs, env.pointer_size) .unwrap_or_else(|err| panic!("TODO gracefully handle bad layout: {:?}", err)); args.push((from_can(env, loc_arg.value, procs, None), layout)); } - Expr::CallByName(proc_name, args.into_bump_slice()) + Expr::CallByName(specialized_proc_name, args.into_bump_slice()) +} + +fn specialize_proc_body<'a>( + env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + fn_var: Variable, + ret_var: Variable, + proc_name: Symbol, + loc_args: &[(Variable, Located)], + partial_proc: &PartialProc<'a>, +) -> Option> { + let mut subs: Subs = env.subs.clone(); + roc_unify::unify::unify(&mut subs, partial_proc.annotation, fn_var); + + // Swap in copied subs, specialize the body, put old subs back + let specialized_body = { + let mut env = Env { + subs: env.arena.alloc(subs), + arena: env.arena, + home: env.home, + ident_ids: &mut env.ident_ids, + pointer_size: env.pointer_size, + }; + + from_can(&mut env, partial_proc.body.clone(), procs, None) + }; + + let mut proc_args = Vec::with_capacity_in(loc_args.len(), &env.arena); + + for (arg_var, _loc_arg) in loc_args.iter() { + let layout = match Layout::from_var(&env.arena, *arg_var, env.subs, env.pointer_size) { + Ok(layout) => layout, + Err(()) => { + // Invalid closure! + return None; + } + }; + + // TODO FIXME what is the idea here? arguments don't map to identifiers one-to-one + // e.g. underscore and record patterns + let arg_name = proc_name; + + // let arg_name: Symbol = match &loc_arg.value { + // Pattern::Identifier(symbol) => *symbol, + // _ => { + // panic!("TODO determine arg_name for pattern {:?}", loc_arg.value); + // } + // }; + + proc_args.push((layout, arg_name)); + } + + let ret_layout = Layout::from_var(&env.arena, ret_var, env.subs, env.pointer_size) + .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)); + + let proc = Proc { + name: proc_name, + args: proc_args.into_bump_slice(), + body: specialized_body, + closes_over: Layout::Struct(&[]), + ret_layout, + }; + + Some(proc) } diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 47a7efedce..b3f4f1fd0e 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -62,6 +62,8 @@ mod test_mono { pointer_size, ); + dbg!(&procs); + // Put this module's ident_ids back in the interns interns.all_ident_ids.insert(home, ident_ids); @@ -121,6 +123,42 @@ mod test_mono { ); } + #[test] + fn specialize_closure() { + compiles_to( + r#" + f = \x -> x + 5 + + { x: f 0x4, y: f 3.14 } + "#, + { + use self::Builtin::*; + use Layout::Builtin; + let home = test_home(); + + let gen_symbol_3 = Interns::from_index(home, 3); + let gen_symbol_4 = Interns::from_index(home, 4); + + Struct { + fields: &[ + ( + "x".into(), + CallByName(gen_symbol_3, &[(Int(4), Builtin(Int64))]), + ), + ( + "y".into(), + CallByName(gen_symbol_4, &[(Float(3.14), Builtin(Float64))]), + ), + ], + layout: Layout::Struct(&[ + ("x".into(), Builtin(Int64)), + ("y".into(), Builtin(Float64)), + ]), + } + }, + ) + } + #[test] fn bool_literal() { let arena = Bump::new(); From a037173cdbad9911d3cb3689cb57eb3488d39842 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 11 Mar 2020 23:00:44 +0100 Subject: [PATCH 04/31] complete the Content hashing --- compiler/types/src/pretty_print.rs | 23 ++++++++ compiler/types/src/subs.rs | 85 ++++++++++++++++++++++++++++-- compiler/types/src/types.rs | 4 +- 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/compiler/types/src/pretty_print.rs b/compiler/types/src/pretty_print.rs index cf5e8a3588..f576a5ca50 100644 --- a/compiler/types/src/pretty_print.rs +++ b/compiler/types/src/pretty_print.rs @@ -525,6 +525,29 @@ pub fn chase_ext_tag_union( } } +pub fn chase_ext_record( + subs: &Subs, + var: Variable, + fields: &mut MutMap, +) -> Option { + use crate::subs::Content::*; + use crate::subs::FlatType::*; + + match subs.get_without_compacting(var).content { + Structure(Record(sub_fields, sub_ext)) => { + fields.extend(sub_fields.into_iter()); + + chase_ext_record(subs, sub_ext, fields) + } + + Structure(EmptyRecord) => None, + + Alias(_, _, var) => chase_ext_record(subs, var, fields), + + content => Some(content), + } +} + fn write_boolean(env: &Env, boolean: Bool, subs: &mut Subs, buf: &mut String, parens: Parens) { match boolean.simplify(subs) { Err(atom) => write_boolean_atom(env, atom, subs, buf, parens), diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 44cfa0ea84..bad0b24414 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -567,7 +567,10 @@ impl ContentHash { T: std::hash::Hasher, { match content { - Content::Alias(_, _, actual) => Self::from_var_help(*actual, subs, hasher), + Content::Alias(_, _, actual) => { + // ensure an alias has the same hash as just the body of the alias + Self::from_var_help(*actual, subs, hasher) + } Content::Structure(flat_type) => { hasher.write_u8(0x10); Self::from_flat_type_help(flat_type, subs, hasher) @@ -598,9 +601,49 @@ impl ContentHash { Self::from_var_help(*ret, subs, hasher); } - FlatType::TagUnion(tags, ext) => { + FlatType::Apply(symbol, arguments) => { hasher.write_u8(1); + symbol.hash(hasher); + + for var in arguments { + Self::from_var_help(*var, subs, hasher); + } + } + + FlatType::EmptyRecord => { + hasher.write_u8(2); + } + + FlatType::Record(fields, ext) => { + hasher.write_u8(3); + + // We have to sort by the key, so this clone seems to be required + let mut fields = fields.clone(); + + match crate::pretty_print::chase_ext_record(subs, *ext, &mut fields) { + Some(_) => panic!("Record with non-empty ext var"), + None => { + let mut fields_vec = Vec::with_capacity(fields.len()); + fields_vec.extend(fields.into_iter()); + + fields_vec.sort(); + + for (name, argument) in fields_vec { + name.hash(hasher); + Self::from_var_help(argument, subs, hasher); + } + } + } + } + + FlatType::EmptyTagUnion => { + hasher.write_u8(4); + } + + FlatType::TagUnion(tags, ext) => { + hasher.write_u8(5); + // We have to sort by the key, so this clone seems to be required let mut tag_vec = Vec::with_capacity(tags.len()); tag_vec.extend(tags.clone().into_iter()); @@ -620,7 +663,43 @@ impl ContentHash { } } - _ => todo!(), + FlatType::RecursiveTagUnion(_rec, tags, ext) => { + // TODO should the rec_var be hashed in? + hasher.write_u8(6); + + // We have to sort by the key, so this clone seems to be required + let mut tag_vec = Vec::with_capacity(tags.len()); + tag_vec.extend(tags.clone().into_iter()); + + match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { + Some(_) => panic!("Tag union with non-empty ext var"), + None => { + tag_vec.sort(); + for (name, arguments) in tag_vec { + name.hash(hasher); + + for var in arguments { + Self::from_var_help(var, subs, hasher); + } + } + } + } + } + + FlatType::Boolean(boolean) => { + hasher.write_u8(7); + match boolean.simplify(subs) { + Ok(_variables) => hasher.write_u8(1), + Err(crate::boolean_algebra::Atom::One) => hasher.write_u8(1), + Err(crate::boolean_algebra::Atom::Zero) => hasher.write_u8(0), + Err(crate::boolean_algebra::Atom::Variable(_)) => unreachable!(), + } + } + + FlatType::Erroneous(_problem) => { + hasher.write_u8(8); + //TODO hash the problem? + } } } } diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 172d0117f0..2ff8f257ca 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -728,14 +728,14 @@ pub fn name_type_var(letters_used: u32, taken: &mut MutSet) -> (Lower } pub fn gather_fields( - subs: &mut Subs, + subs: &Subs, fields: MutMap, var: Variable, ) -> RecordStructure { use crate::subs::Content::*; use crate::subs::FlatType::*; - match subs.get(var).content { + match subs.get_without_compacting(var).content { Structure(Record(sub_fields, sub_ext)) => { gather_fields(subs, union(fields, &sub_fields), sub_ext) } From 2d0649fa665c4f763d25cb22628cfb6096011299 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 11 Mar 2020 23:13:32 +0100 Subject: [PATCH 05/31] attempt fix for gen tests --- compiler/gen/tests/test_gen.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index 464f680ddd..fb09a4243f 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -85,13 +85,15 @@ mod test_gen { // Declare all the Procs, then insert them into scope so their bodies // can look up their Funcs in scope later when calling each other by value. - for (name, opt_proc) in procs.iter() { - if let Some(proc) = opt_proc { - let (func_id, sig) = declare_proc(&env, &mut module, name.clone(), proc); + for (_name, partial_proc) in procs.iter() { + for ( _content_hash, opt_proc ) in partial_proc.specializations.iter() { + if let (name, Some(proc)) = opt_proc { + let (func_id, sig) = declare_proc(&env, &mut module, name.clone(), &proc); - declared.push((proc.clone(), sig.clone(), func_id)); + declared.push((proc.clone(), sig.clone(), func_id)); - scope.insert(name.clone(), ScopeEntry::Func { func_id, sig }); + scope.insert(name.clone(), ScopeEntry::Func { func_id, sig }); + } } } @@ -234,12 +236,14 @@ mod test_gen { // Add all the Proc headers to the module. // We have to do this in a separate pass first, // because their bodies may reference each other. - for (symbol, opt_proc) in procs.clone().into_iter() { - if let Some(proc) = opt_proc { + for (_symbol, partial_proc) in procs.clone().into_iter() { + for ( _content_hash, opt_proc ) in partial_proc.specializations { + if let (symbol, Some(proc)) = opt_proc { let (fn_val, arg_basic_types) = build_proc_header(&env, symbol, &proc); headers.push((proc, fn_val, arg_basic_types)); } + } } // Build each proc using its header info. @@ -369,12 +373,15 @@ mod test_gen { // Add all the Proc headers to the module. // We have to do this in a separate pass first, // because their bodies may reference each other. - for (symbol, opt_proc) in procs.clone().into_iter() { - if let Some(proc) = opt_proc { + for (_symbol, partial_proc) in procs.clone().into_iter() { + for ( _content_hash, opt_proc ) in partial_proc.specializations { + if let (symbol, Some(proc)) = opt_proc { let (fn_val, arg_basic_types) = build_proc_header(&env, symbol, &proc); headers.push((proc, fn_val, arg_basic_types)); } + } + } // Build each proc using its header info. From bb9c9d423a31f41570e039753cafad70e7b1dbab Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 00:40:10 +0100 Subject: [PATCH 06/31] make Procs a struct --- compiler/gen/tests/test_gen.rs | 34 ++++++------- compiler/mono/src/expr.rs | 83 ++++++++++++++++++++++++++------ compiler/mono/tests/test_mono.rs | 3 +- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index fb09a4243f..92a52f6c0a 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -24,13 +24,13 @@ mod test_gen { use inkwell::passes::PassManager; use inkwell::types::BasicType; use inkwell::OptimizationLevel; - use roc_collections::all::{ImMap, MutMap}; + use roc_collections::all::ImMap; use roc_gen::crane::build::{declare_proc, define_proc_body, ScopeEntry}; use roc_gen::crane::convert::type_from_layout; use roc_gen::crane::imports::define_malloc; use roc_gen::llvm::build::{build_proc, build_proc_header}; use roc_gen::llvm::convert::basic_type_from_layout; - use roc_mono::expr::Expr; + use roc_mono::expr::{Expr, Procs}; use roc_mono::layout::Layout; use roc_types::subs::Subs; use std::ffi::{CStr, CString}; @@ -65,7 +65,7 @@ mod test_gen { let main_ret_type = type_from_layout(cfg, &layout); // Compile and add all the Procs before adding main - let mut procs = MutMap::default(); + let mut procs = Procs::default(); let mut env = roc_gen::crane::build::Env { arena: &arena, interns, @@ -85,16 +85,14 @@ mod test_gen { // Declare all the Procs, then insert them into scope so their bodies // can look up their Funcs in scope later when calling each other by value. - for (_name, partial_proc) in procs.iter() { - for ( _content_hash, opt_proc ) in partial_proc.specializations.iter() { - if let (name, Some(proc)) = opt_proc { - let (func_id, sig) = declare_proc(&env, &mut module, name.clone(), &proc); + for (name, opt_proc) in procs.as_map().into_iter() { + if let Some(proc) = opt_proc { + let (func_id, sig) = declare_proc(&env, &mut module, name, &proc); declared.push((proc.clone(), sig.clone(), func_id)); scope.insert(name.clone(), ScopeEntry::Func { func_id, sig }); } - } } for (proc, sig, fn_id) in declared { @@ -222,7 +220,7 @@ mod test_gen { module: arena.alloc(module), pointer_bytes }; - let mut procs = MutMap::default(); + let mut procs = Procs::default(); let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap(); // Populate Procs and get the low-level Expr from the canonical Expr @@ -236,14 +234,12 @@ mod test_gen { // Add all the Proc headers to the module. // We have to do this in a separate pass first, // because their bodies may reference each other. - for (_symbol, partial_proc) in procs.clone().into_iter() { - for ( _content_hash, opt_proc ) in partial_proc.specializations { - if let (symbol, Some(proc)) = opt_proc { + for (symbol, opt_proc) in procs.as_map().into_iter() { + if let Some(proc) = opt_proc { let (fn_val, arg_basic_types) = build_proc_header(&env, symbol, &proc); headers.push((proc, fn_val, arg_basic_types)); } - } } // Build each proc using its header info. @@ -275,7 +271,7 @@ mod test_gen { &ImMap::default(), main_fn, &main_body, - &mut MutMap::default(), + &mut Procs::default(), ); builder.build_return(Some(&ret)); @@ -359,7 +355,7 @@ mod test_gen { module: arena.alloc(module), pointer_bytes }; - let mut procs = MutMap::default(); + let mut procs = Procs::default(); let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap(); // Populate Procs and get the low-level Expr from the canonical Expr @@ -373,14 +369,12 @@ mod test_gen { // Add all the Proc headers to the module. // We have to do this in a separate pass first, // because their bodies may reference each other. - for (_symbol, partial_proc) in procs.clone().into_iter() { - for ( _content_hash, opt_proc ) in partial_proc.specializations { - if let (symbol, Some(proc)) = opt_proc { + for (symbol, opt_proc) in procs.as_map().into_iter() { + if let Some(proc) = opt_proc { let (fn_val, arg_basic_types) = build_proc_header(&env, symbol, &proc); headers.push((proc, fn_val, arg_basic_types)); } - } } @@ -413,7 +407,7 @@ mod test_gen { &ImMap::default(), main_fn, &main_body, - &mut MutMap::default(), + &mut Procs::default(), ); builder.build_return(Some(&ret)); diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 0554575e0e..93e687a823 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -3,13 +3,66 @@ use bumpalo::collections::Vec; use bumpalo::Bump; use roc_can; use roc_can::pattern::Pattern; -use roc_collections::all::MutMap; +use roc_collections::all::{MutMap, MutSet}; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::{IdentIds, ModuleId, Symbol}; use roc_region::all::Located; use roc_types::subs::{Content, ContentHash, FlatType, Subs, Variable}; -pub type Procs<'a> = MutMap>; +#[derive(Clone, Debug, PartialEq, Default)] +pub struct Procs<'a> { + user_defined: MutMap>, + builtin: MutSet, +} + +impl<'a> Procs<'a> { + fn insert_user_defined(&mut self, symbol: Symbol, partial_proc: PartialProc<'a>) { + self.user_defined.insert(symbol, partial_proc); + } + + fn insert_specialization( + &mut self, + symbol: Symbol, + hash: ContentHash, + spec_name: Symbol, + proc: Option>, + ) { + self.user_defined + .get_mut(&symbol) + .map(|partial_proc| partial_proc.specializations.insert(hash, (spec_name, proc))); + } + + fn get_user_defined(&self, symbol: Symbol) -> Option<&PartialProc<'a>> { + self.user_defined.get(&symbol) + } + + pub fn len(&self) -> usize { + self.user_defined + .values() + .map(|v| v.specializations.len()) + .sum() + } + + fn insert_builtin(&mut self, symbol: Symbol) { + self.builtin.insert(symbol); + } + + pub fn as_map(&self) -> MutMap>> { + let mut result = MutMap::default(); + + for partial_proc in self.user_defined.values() { + for (_, (symbol, opt_proc)) in partial_proc.specializations.clone().into_iter() { + result.insert(symbol, opt_proc); + } + } + + for symbol in self.builtin.iter() { + result.insert(*symbol, None); + } + + result + } +} #[derive(Clone, Debug, PartialEq)] pub struct PartialProc<'a> { @@ -268,7 +321,7 @@ fn from_can<'a>( let symbol = name.unwrap_or_else(|| gen_closure_name(procs, &mut env.ident_ids, env.home)); - procs.insert( + procs.insert_user_defined( symbol, PartialProc { annotation, @@ -854,7 +907,7 @@ fn call_by_name<'a>( // TODO this seems very wrong! // something with mutable and immutable borrow of procs let procs_clone = procs.clone(); - let op_partial_proc = procs_clone.get(&proc_name); + let op_partial_proc = procs_clone.get_user_defined(proc_name); let specialized_proc_name = if let Some(partial_proc) = op_partial_proc { let content_hash = ContentHash::from_var(fn_var, env.subs); @@ -868,10 +921,12 @@ fn call_by_name<'a>( // register proc, so specialization doesn't loop infinitely // for recursive definitions - let mut temp = partial_proc.clone(); - temp.specializations - .insert(content_hash, (spec_proc_name, None)); - procs.insert(proc_name, temp); + // let mut temp = partial_proc.clone(); + // temp.specializations + // .insert(content_hash, (spec_proc_name, None)); + // procs.insert_user_defined(proc_name, temp); + + procs.insert_specialization(proc_name, content_hash, spec_proc_name, None); let proc = specialize_proc_body( env, @@ -883,21 +938,19 @@ fn call_by_name<'a>( partial_proc, ); - // we must be careful here, the specialization could have added different - // specializations of proc_name, so we must get the partial_proc again! - procs.get_mut(&proc_name).map(|partial_proc| { - partial_proc - .specializations - .insert(content_hash, (spec_proc_name, proc)) - }); + procs.insert_specialization(proc_name, content_hash, spec_proc_name, proc); spec_proc_name } } else { // This happens for built-in symbols (they are never defined as a Closure) + procs.insert_builtin(proc_name); proc_name }; + dbg!(proc_name); + dbg!(specialized_proc_name); + // generate actual call let mut args = Vec::with_capacity_in(loc_args.len(), env.arena); diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index b3f4f1fd0e..10cf290372 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -17,6 +17,7 @@ mod test_mono { use roc_module::ident::TagName::*; use roc_module::symbol::{Interns, Symbol}; use roc_mono::expr::Expr::{self, *}; + use roc_mono::expr::Procs; use roc_mono::layout::{Builtin, Layout}; use roc_types::subs::Subs; @@ -45,7 +46,7 @@ mod test_mono { let (_content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); // Compile and add all the Procs before adding main - let mut procs = MutMap::default(); + let mut procs = Procs::default(); let mut ident_ids = interns.all_ident_ids.remove(&home).unwrap(); // assume 64-bit pointers From 74b58db47786b419368897bd9af1d3aa167777a6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 19:45:12 -0400 Subject: [PATCH 07/31] Use Procs::default() --- compiler/mono/tests/test_opt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/mono/tests/test_opt.rs b/compiler/mono/tests/test_opt.rs index a7c6cdebc1..483c022eb2 100644 --- a/compiler/mono/tests/test_opt.rs +++ b/compiler/mono/tests/test_opt.rs @@ -13,9 +13,9 @@ mod helpers; mod test_opt { use crate::helpers::{infer_expr, uniq_expr}; use bumpalo::Bump; - use roc_collections::all::MutMap; use roc_module::symbol::Symbol; use roc_mono::expr::Expr::{self, *}; + use roc_mono::expr::Procs; use roc_mono::layout::{Builtin, Layout}; // HELPERS @@ -28,7 +28,7 @@ mod test_opt { let (_content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); // Compile and add all the Procs before adding main - let mut procs = MutMap::default(); + let mut procs = Procs::default(); let mut ident_ids = interns.all_ident_ids.remove(&home).unwrap(); // assume 64-bit pointers From 649575fab8f48f7e77d71be8e566216c219380b9 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 19:48:19 -0400 Subject: [PATCH 08/31] Improve some error messages --- compiler/gen/src/crane/build.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index cb45836e22..126b5193d1 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -120,9 +120,7 @@ pub fn build_expr<'a, B: Backend>( let fn_id = match scope.get(name) { Some(ScopeEntry::Func{ func_id, .. }) => *func_id, other => panic!( - "FunctionPointer could not find function named {:?} in scope; instead, found {:?} in scope {:?}", - name, other, scope - ), + "FunctionPointer could not find function named {:?} declared in scope (and it was not special-cased in crane::build as a builtin); instead, found {:?} in scope {:?}", name, other, scope), }; let func_ref = module.declare_func_in_func(fn_id, &mut builder.func); @@ -665,12 +663,9 @@ fn call_by_name<'a, B: Backend>( } _ => { let fn_id = match scope.get(&symbol) { - Some(ScopeEntry::Func{ func_id, .. }) => *func_id, - other => panic!( - "CallByName could not find function named {:?} in scope; instead, found {:?} in scope {:?}", - symbol, other, scope - ), - }; + Some(ScopeEntry::Func { func_id, .. }) => *func_id, + other => panic!("CallByName could not find function named {:?} declared in scope (and it was not special-cased in crane::build as a builtin); instead, found {:?} in scope {:?}", symbol, other, scope), + }; let local_func = module.declare_func_in_func(fn_id, &mut builder.func); let mut arg_vals = Vec::with_capacity_in(args.len(), env.arena); From 523282e7bc2fd87ddd45476a133b69c9fde334fb Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 20:50:32 -0400 Subject: [PATCH 09/31] Implement Int.#add --- compiler/gen/src/crane/build.rs | 2 +- compiler/gen/src/llvm/build.rs | 4 ++-- compiler/module/src/symbol.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index 126b5193d1..f7ccfab05a 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -545,7 +545,7 @@ fn call_by_name<'a, B: Backend>( procs: &Procs<'a>, ) -> Value { match symbol { - Symbol::NUM_ADD => { + Symbol::INT_ADD | Symbol::NUM_ADD => { debug_assert!(args.len() == 2); let a = build_arg(&args[0], env, scope, module, builder, procs); let b = build_arg(&args[1], env, scope, module, builder, procs); diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 6dbfc17981..c2e294abf9 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -536,13 +536,13 @@ fn call_with_args<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, ) -> BasicValueEnum<'ctx> { match symbol { - Symbol::NUM_ADD => { + Symbol::INT_ADD | Symbol::NUM_ADD => { debug_assert!(args.len() == 2); let int_val = env.builder.build_int_add( args[0].into_int_value(), args[1].into_int_value(), - "ADD_I64", + "add_i64", ); BasicValueEnum::IntValue(int_val) diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index 4ab921501f..9c39b8f745 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -575,7 +575,7 @@ define_builtins! { 4 INT_MOD: "mod" 5 INT_HIGHEST: "highest" 6 INT_LOWEST: "lowest" - 7 INT_ADD: "add" + 7 INT_ADD: "#add" } 3 FLOAT: "Float" => { 0 FLOAT_FLOAT: "Float" imported // the Float.Float type alias From 21e4eb505a49bff84e67044acce6f38b84ec33f6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 20:50:52 -0400 Subject: [PATCH 10/31] Implement and test Float.#add --- compiler/gen/src/crane/build.rs | 7 +++++++ compiler/gen/src/llvm/build.rs | 11 +++++++++++ compiler/gen/tests/test_gen.rs | 13 +++++++++++++ compiler/module/src/symbol.rs | 2 +- 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index f7ccfab05a..56ad0110e0 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -552,6 +552,13 @@ fn call_by_name<'a, B: Backend>( builder.ins().iadd(a, b) } + Symbol::FLOAT_ADD => { + debug_assert!(args.len() == 2); + let a = build_arg(&args[0], env, scope, module, builder, procs); + let b = build_arg(&args[1], env, scope, module, builder, procs); + + builder.ins().fadd(a, b) + } Symbol::NUM_SUB => { debug_assert!(args.len() == 2); let a = build_arg(&args[0], env, scope, module, builder, procs); diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index c2e294abf9..df44bdf4f2 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -547,6 +547,17 @@ fn call_with_args<'a, 'ctx, 'env>( BasicValueEnum::IntValue(int_val) } + Symbol::FLOAT_ADD => { + debug_assert!(args.len() == 2); + + let float_val = env.builder.build_float_add( + args[0].into_float_value(), + args[1].into_float_value(), + "add_f64", + ); + + BasicValueEnum::FloatValue(float_val) + } Symbol::NUM_SUB => { debug_assert!(args.len() == 2); diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index 92a52f6c0a..f51bf606c4 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -800,6 +800,19 @@ mod test_gen { ); } + #[test] + fn gen_add_f64() { + assert_evals_to!( + indoc!( + r#" + 1.1 + 2.4 + 3 + "# + ), + 6.5, + f64 + ); + } + #[test] fn gen_add_i64() { assert_evals_to!( diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index 9c39b8f745..15650d2320 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -586,7 +586,7 @@ define_builtins! { 5 FLOAT_SQRT: "sqrt" 6 FLOAT_HIGHEST: "highest" 7 FLOAT_LOWEST: "lowest" - 8 FLOAT_ADD: "add" + 8 FLOAT_ADD: "#add" } 4 BOOL: "Bool" => { 0 BOOL_BOOL: "Bool" imported // the Bool.Bool type alias From a0c4e9179214d0f5886b17757bd52b61549d1a4d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 20:53:06 -0400 Subject: [PATCH 11/31] Rename List.set_in_place to List.#setInPlace --- compiler/module/src/symbol.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index 15650d2320..ddd243e9b1 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -608,7 +608,7 @@ define_builtins! { 2 LIST_ISEMPTY: "isEmpty" 3 LIST_GET: "get" 4 LIST_SET: "set" - 5 LIST_SET_IN_PLACE: "set_in_place" + 5 LIST_SET_IN_PLACE: "#setInPlace" 6 LIST_PUSH: "push" 7 LIST_MAP: "map" 8 LIST_LENGTH: "length" From 762b2c7b10dd4af11a1bb3eb084fe14b643be215 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 21:03:58 -0400 Subject: [PATCH 12/31] use IntOrFloat::* --- compiler/mono/src/expr.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 93e687a823..0aa83a4f90 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -335,13 +335,15 @@ fn from_can<'a>( } Call(boxed, loc_args, _) => { + use IntOrFloat::*; + let (fn_var, loc_expr, ret_var) = *boxed; let specialize_builtin_functions = { |symbol, subs: &Subs| match symbol { Symbol::NUM_ADD => match to_int_or_float(subs, ret_var) { - IntOrFloat::FloatType => Symbol::FLOAT_ADD, - IntOrFloat::IntType => Symbol::INT_ADD, + FloatType => Symbol::FLOAT_ADD, + IntType => Symbol::INT_ADD, }, _ => symbol, } From 9fcfa90bff7fd4fd2d2a814338c3ce27ff23d47f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 21:05:41 -0400 Subject: [PATCH 13/31] Change capitalization --- compiler/gen/src/llvm/build.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index df44bdf4f2..c9fedd2fa8 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -564,7 +564,7 @@ fn call_with_args<'a, 'ctx, 'env>( let int_val = env.builder.build_int_sub( args[0].into_int_value(), args[1].into_int_value(), - "SUB_I64", + "sub_I64", ); BasicValueEnum::IntValue(int_val) @@ -575,7 +575,7 @@ fn call_with_args<'a, 'ctx, 'env>( let int_val = env.builder.build_int_mul( args[0].into_int_value(), args[1].into_int_value(), - "MUL_I64", + "mul_i64", ); BasicValueEnum::IntValue(int_val) @@ -585,7 +585,7 @@ fn call_with_args<'a, 'ctx, 'env>( let int_val = env .builder - .build_int_neg(args[0].into_int_value(), "NEGATE_I64"); + .build_int_neg(args[0].into_int_value(), "negate_i64"); BasicValueEnum::IntValue(int_val) } @@ -598,7 +598,7 @@ fn call_with_args<'a, 'ctx, 'env>( let builder = env.builder; let elem_bytes = 8; // TODO Look this up instead of hardcoding it! let elem_size = env.context.i64_type().const_int(elem_bytes, false); - let offset = builder.build_int_mul(elem_index, elem_size, "MUL_OFFSET"); + let offset = builder.build_int_mul(elem_index, elem_size, "mul_offset"); let elem_ptr = unsafe { builder.build_gep(list_ptr, &[offset], "elem") }; From df78068e81724460f5ea8ff6b0090e74a4662b4c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 21:10:41 -0400 Subject: [PATCH 14/31] Implement int and float subtraction --- compiler/gen/src/crane/build.rs | 9 ++++++++- compiler/gen/src/llvm/build.rs | 13 ++++++++++++- compiler/gen/tests/test_gen.rs | 13 +++++++++++++ compiler/module/src/symbol.rs | 2 ++ compiler/mono/src/expr.rs | 4 ++++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index 56ad0110e0..b46c0d80b4 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -559,13 +559,20 @@ fn call_by_name<'a, B: Backend>( builder.ins().fadd(a, b) } - Symbol::NUM_SUB => { + Symbol::INT_SUB | Symbol::NUM_SUB => { debug_assert!(args.len() == 2); let a = build_arg(&args[0], env, scope, module, builder, procs); let b = build_arg(&args[1], env, scope, module, builder, procs); builder.ins().isub(a, b) } + Symbol::FLOAT_SUB => { + debug_assert!(args.len() == 2); + let a = build_arg(&args[0], env, scope, module, builder, procs); + let b = build_arg(&args[1], env, scope, module, builder, procs); + + builder.ins().fsub(a, b) + } Symbol::NUM_MUL => { debug_assert!(args.len() == 2); let a = build_arg(&args[0], env, scope, module, builder, procs); diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index c9fedd2fa8..e530f4baef 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -558,7 +558,7 @@ fn call_with_args<'a, 'ctx, 'env>( BasicValueEnum::FloatValue(float_val) } - Symbol::NUM_SUB => { + Symbol::INT_SUB | Symbol::NUM_SUB => { debug_assert!(args.len() == 2); let int_val = env.builder.build_int_sub( @@ -569,6 +569,17 @@ fn call_with_args<'a, 'ctx, 'env>( BasicValueEnum::IntValue(int_val) } + Symbol::FLOAT_SUB => { + debug_assert!(args.len() == 2); + + let float_val = env.builder.build_float_sub( + args[0].into_float_value(), + args[1].into_float_value(), + "sub_f64", + ); + + BasicValueEnum::FloatValue(float_val) + } Symbol::NUM_MUL => { debug_assert!(args.len() == 2); diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index f51bf606c4..fe67f1eb42 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -826,6 +826,19 @@ mod test_gen { ); } + #[test] + fn gen_sub_f64() { + assert_evals_to!( + indoc!( + r#" + 1.5 - 2.4 - 3 + "# + ), + -3.9, + f64 + ); + } + #[test] fn gen_sub_i64() { assert_evals_to!( diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index ddd243e9b1..bee661cef2 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -576,6 +576,7 @@ define_builtins! { 5 INT_HIGHEST: "highest" 6 INT_LOWEST: "lowest" 7 INT_ADD: "#add" + 8 INT_SUB: "#sub" } 3 FLOAT: "Float" => { 0 FLOAT_FLOAT: "Float" imported // the Float.Float type alias @@ -587,6 +588,7 @@ define_builtins! { 6 FLOAT_HIGHEST: "highest" 7 FLOAT_LOWEST: "lowest" 8 FLOAT_ADD: "#add" + 9 FLOAT_SUB: "#sub" } 4 BOOL: "Bool" => { 0 BOOL_BOOL: "Bool" imported // the Bool.Bool type alias diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 0aa83a4f90..3d7fd87568 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -345,6 +345,10 @@ fn from_can<'a>( FloatType => Symbol::FLOAT_ADD, IntType => Symbol::INT_ADD, }, + Symbol::NUM_SUB => match to_int_or_float(subs, ret_var) { + FloatType => Symbol::FLOAT_SUB, + IntType => Symbol::INT_SUB, + }, _ => symbol, } }; From 2ad70d44a235e50f07e03c1d26525ec4037910ad Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 21:15:30 -0400 Subject: [PATCH 15/31] Rename Attr module to #Attr, drop #Attr.@Attr --- compiler/module/src/symbol.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index bee661cef2..5b3683a188 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -547,11 +547,13 @@ macro_rules! define_builtins { }; } +// NOTE: Some of these builtins have a # at the beginning of their names. +// This is because they are for compiler use only, and should not cause +// namespace conflicts with userspace! define_builtins! { - 0 ATTR: "Attr" => { + 0 ATTR: "#Attr" => { 0 UNDERSCORE: "_" // the _ used in pattern matches. This is Symbol 0. - 1 ATTR_ATTR: "Attr" // the Attr.Attr type alias, used in uniqueness types - 2 ATTR_AT_ATTR: "@Attr" // the Attr.@Attr private tag + 1 ATTR_ATTR: "Attr" // the #Attr.Attr type alias, used in uniqueness types. } 1 NUM: "Num" => { 0 NUM_NUM: "Num" imported // the Num.Num type alias From cf5e3f92a53486132f1f97d4431bf6211254a8dd Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 21:36:27 -0400 Subject: [PATCH 16/31] Have mono::Env store &mut Subs --- compiler/gen/tests/test_gen.rs | 12 ++++++------ compiler/mono/src/expr.rs | 16 +++++++--------- compiler/mono/tests/test_mono.rs | 8 ++++---- compiler/mono/tests/test_opt.rs | 4 ++-- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index fe67f1eb42..1809593a57 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -46,7 +46,7 @@ mod test_gen { let CanExprOut { loc_expr, var_store, var, constraint, home, interns, .. } = can_expr($src); let subs = Subs::new(var_store.into()); let mut unify_problems = Vec::new(); - let (content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); + let (content, mut subs) = infer_expr(subs, &mut unify_problems, &constraint, var); let shared_builder = settings::builder(); let shared_flags = settings::Flags::new(shared_builder); let mut module: Module = @@ -75,7 +75,7 @@ mod test_gen { let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap(); // Populate Procs and Subs, and get the low-level Expr from the canonical Expr - let mono_expr = Expr::new(&arena, &subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); + let mono_expr = Expr::new(&arena, &mut subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); // Put this module's ident_ids back in the interns env.interns.all_ident_ids.insert(home, ident_ids); @@ -174,7 +174,7 @@ mod test_gen { let CanExprOut { loc_expr, var_store, var, constraint, home, interns, .. } = can_expr($src); let subs = Subs::new(var_store.into()); let mut unify_problems = Vec::new(); - let (content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); + let (content, mut subs) = infer_expr(subs, &mut unify_problems, &constraint, var); let context = Context::create(); let module = context.create_module("app"); @@ -224,7 +224,7 @@ mod test_gen { let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap(); // Populate Procs and get the low-level Expr from the canonical Expr - let main_body = Expr::new(&arena, &subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); + let main_body = Expr::new(&arena, &mut subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); // Put this module's ident_ids back in the interns, so we can use them in Env. env.interns.all_ident_ids.insert(home, ident_ids); @@ -309,7 +309,7 @@ mod test_gen { let (loc_expr, _output, _problems, subs, var, constraint, home, interns) = uniq_expr($src); let mut unify_problems = Vec::new(); - let (content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); + let (content, mut subs) = infer_expr(subs, &mut unify_problems, &constraint, var); let context = Context::create(); let module = context.create_module("app"); @@ -359,7 +359,7 @@ mod test_gen { let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap(); // Populate Procs and get the low-level Expr from the canonical Expr - let main_body = Expr::new(&arena, &subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); + let main_body = Expr::new(&arena, &mut subs, loc_expr.value, &mut procs, home, &mut ident_ids, POINTER_SIZE); // Put this module's ident_ids back in the interns, so we can use them in Env. env.interns.all_ident_ids.insert(home, ident_ids); diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 3d7fd87568..d3ec6eee68 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -82,7 +82,7 @@ pub struct Proc<'a> { struct Env<'a, 'i> { pub arena: &'a Bump, - pub subs: &'a Subs, + pub subs: &'a mut Subs, pub home: ModuleId, pub ident_ids: &'i mut IdentIds, pub pointer_size: u32, @@ -175,7 +175,7 @@ pub enum Expr<'a> { impl<'a> Expr<'a> { pub fn new( arena: &'a Bump, - subs: &'a Subs, + subs: &'a mut Subs, can_expr: roc_can::expr::Expr, procs: &mut Procs<'a>, home: ModuleId, @@ -430,7 +430,6 @@ fn from_can<'a>( } => from_can_when(env, cond_var, expr_var, *loc_cond, branches, procs), Record(ext_var, fields) => { - let subs = env.subs; let arena = env.arena; let mut field_bodies = Vec::with_capacity_in(fields.len(), arena); @@ -440,7 +439,7 @@ fn from_can<'a>( field_bodies.push((label, expr)); } - let struct_layout = match Layout::from_var(arena, ext_var, subs, env.pointer_size) { + let struct_layout = match Layout::from_var(arena, ext_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { // Invalid field! @@ -494,10 +493,9 @@ fn from_can<'a>( field, .. } => { - let subs = env.subs; let arena = env.arena; - let struct_layout = match Layout::from_var(arena, ext_var, subs, env.pointer_size) { + let struct_layout = match Layout::from_var(arena, ext_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { // Invalid field! @@ -505,7 +503,8 @@ fn from_can<'a>( } }; - let field_layout = match Layout::from_var(arena, field_var, subs, env.pointer_size) { + let field_layout = match Layout::from_var(arena, field_var, env.subs, env.pointer_size) + { Ok(layout) => layout, Err(()) => { // Invalid field! @@ -524,9 +523,8 @@ fn from_can<'a>( elem_var, loc_elems, } => { - let subs = env.subs; let arena = env.arena; - let elem_layout = match Layout::from_var(arena, elem_var, subs, env.pointer_size) { + let elem_layout = match Layout::from_var(arena, elem_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { panic!("TODO gracefully handle List with invalid element layout"); diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 10cf290372..65ae5ea18e 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -43,7 +43,7 @@ mod test_mono { } = can_expr(src); let subs = Subs::new(var_store.into()); let mut unify_problems = Vec::new(); - let (_content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); + let (_content, mut subs) = infer_expr(subs, &mut unify_problems, &constraint, var); // Compile and add all the Procs before adding main let mut procs = Procs::default(); @@ -55,7 +55,7 @@ mod test_mono { // Populate Procs and Subs, and get the low-level Expr from the canonical Expr let mono_expr = Expr::new( &arena, - &subs, + &mut subs, loc_expr.value, &mut procs, home, @@ -128,8 +128,8 @@ mod test_mono { fn specialize_closure() { compiles_to( r#" - f = \x -> x + 5 - + f = \x -> x + 5 + { x: f 0x4, y: f 3.14 } "#, { diff --git a/compiler/mono/tests/test_opt.rs b/compiler/mono/tests/test_opt.rs index 483c022eb2..7acdd776cb 100644 --- a/compiler/mono/tests/test_opt.rs +++ b/compiler/mono/tests/test_opt.rs @@ -25,7 +25,7 @@ mod test_opt { let (loc_expr, _, _problems, subs, var, constraint, home, mut interns) = uniq_expr(src); let mut unify_problems = Vec::new(); - let (_content, subs) = infer_expr(subs, &mut unify_problems, &constraint, var); + let (_content, mut subs) = infer_expr(subs, &mut unify_problems, &constraint, var); // Compile and add all the Procs before adding main let mut procs = Procs::default(); @@ -37,7 +37,7 @@ mod test_opt { // Populate Procs and Subs, and get the low-level Expr from the canonical Expr let mono_expr = Expr::new( &arena, - &subs, + &mut subs, loc_expr.value, &mut procs, home, From 3dbaac210a253c1751c7437dec1a4a59a39ea572 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 02:38:33 +0100 Subject: [PATCH 17/31] add snapshot functions to Subs --- compiler/types/src/subs.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index bad0b24414..56b8b3a09a 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -6,7 +6,7 @@ use roc_module::symbol::Symbol; use std::fmt; use std::iter::{once, Iterator}; use std::sync::atomic::{AtomicU32, Ordering}; -use ven_ena::unify::{InPlace, UnificationTable, UnifyKey}; +use ven_ena::unify::{InPlace, Snapshot, UnificationTable, UnifyKey}; #[derive(Clone, Copy, Hash, PartialEq, Eq)] pub struct Mark(i32); @@ -402,6 +402,14 @@ impl Subs { pub fn is_empty(&self) -> bool { self.utable.is_empty() } + + pub fn snapshot(&mut self) -> Snapshot> { + self.utable.snapshot() + } + + pub fn rollback_to(&mut self, snapshot: Snapshot>) { + self.utable.rollback_to(snapshot) + } } #[inline(always)] From c9644e4ee7d35d002b422cc1a28e5aeff6546050 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 02:42:30 +0100 Subject: [PATCH 18/31] use snapshots to remove clone on Subs --- compiler/mono/src/expr.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index d3ec6eee68..e5eb771568 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -977,21 +977,12 @@ fn specialize_proc_body<'a>( loc_args: &[(Variable, Located)], partial_proc: &PartialProc<'a>, ) -> Option> { - let mut subs: Subs = env.subs.clone(); - roc_unify::unify::unify(&mut subs, partial_proc.annotation, fn_var); - - // Swap in copied subs, specialize the body, put old subs back - let specialized_body = { - let mut env = Env { - subs: env.arena.alloc(subs), - arena: env.arena, - home: env.home, - ident_ids: &mut env.ident_ids, - pointer_size: env.pointer_size, - }; - - from_can(&mut env, partial_proc.body.clone(), procs, None) - }; + // unify the called function with the specialized signature, then specialize the function body + let snapshot = env.subs.snapshot(); + roc_unify::unify::unify(env.subs, partial_proc.annotation, fn_var); + let specialized_body = from_can(env, partial_proc.body.clone(), procs, None); + // reset subs, so we don't get type errors when specializing for a different signature + env.subs.rollback_to(snapshot); let mut proc_args = Vec::with_capacity_in(loc_args.len(), &env.arena); From 9db7d2229a16f59be63b5a0124b6cac61959bc66 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 11 Mar 2020 22:18:44 -0400 Subject: [PATCH 19/31] Remove a .clone() on procs --- compiler/mono/src/expr.rs | 78 +++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index e5eb771568..d664bbe63d 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -908,50 +908,63 @@ fn call_by_name<'a>( ) -> Expr<'a> { // create specialized procedure to call - // TODO this seems very wrong! - // something with mutable and immutable borrow of procs - let procs_clone = procs.clone(); - let op_partial_proc = procs_clone.get_user_defined(proc_name); + // If we need to specialize the body, this will get populated with the info + // we need to do that. This is defined outside the procs.get_user_defined(...) call + // because if we tried to specialize the body inside that match, we would + // get a borrow checker error about trying to borrow `procs` as mutable + // while there is still an active immutable borrow. + let opt_specialize_body: Option<(ContentHash, Variable, roc_can::expr::Expr)>; - let specialized_proc_name = if let Some(partial_proc) = op_partial_proc { + let specialized_proc_name = if let Some(partial_proc) = procs.get_user_defined(proc_name) { let content_hash = ContentHash::from_var(fn_var, env.subs); if let Some(specialization) = partial_proc.specializations.get(&content_hash) { + opt_specialize_body = None; + // a specialization with this type hash already exists, use its symbol specialization.0 } else { + opt_specialize_body = Some(( + content_hash, + partial_proc.annotation, + partial_proc.body.clone(), + )); + // generate a symbol for this specialization - let spec_proc_name = gen_closure_name(procs, &mut env.ident_ids, env.home); - - // register proc, so specialization doesn't loop infinitely - // for recursive definitions - // let mut temp = partial_proc.clone(); - // temp.specializations - // .insert(content_hash, (spec_proc_name, None)); - // procs.insert_user_defined(proc_name, temp); - - procs.insert_specialization(proc_name, content_hash, spec_proc_name, None); - - let proc = specialize_proc_body( - env, - procs, - fn_var, - ret_var, - spec_proc_name, - &loc_args, - partial_proc, - ); - - procs.insert_specialization(proc_name, content_hash, spec_proc_name, proc); - - spec_proc_name + gen_closure_name(procs, &mut env.ident_ids, env.home) } } else { + opt_specialize_body = None; + // This happens for built-in symbols (they are never defined as a Closure) procs.insert_builtin(proc_name); proc_name }; + if let Some((content_hash, annotation, body)) = opt_specialize_body { + // register proc, so specialization doesn't loop infinitely + // for recursive definitions + // let mut temp = partial_proc.clone(); + // temp.specializations + // .insert(content_hash, (spec_proc_name, None)); + // procs.insert_user_defined(proc_name, temp); + + procs.insert_specialization(proc_name, content_hash, specialized_proc_name, None); + + let proc = specialize_proc_body( + env, + procs, + fn_var, + ret_var, + specialized_proc_name, + &loc_args, + annotation, + body, + ); + + procs.insert_specialization(proc_name, content_hash, specialized_proc_name, proc); + } + dbg!(proc_name); dbg!(specialized_proc_name); @@ -975,12 +988,13 @@ fn specialize_proc_body<'a>( ret_var: Variable, proc_name: Symbol, loc_args: &[(Variable, Located)], - partial_proc: &PartialProc<'a>, + annotation: Variable, + body: roc_can::expr::Expr, ) -> Option> { // unify the called function with the specialized signature, then specialize the function body let snapshot = env.subs.snapshot(); - roc_unify::unify::unify(env.subs, partial_proc.annotation, fn_var); - let specialized_body = from_can(env, partial_proc.body.clone(), procs, None); + roc_unify::unify::unify(env.subs, annotation, fn_var); + let specialized_body = from_can(env, body, procs, None); // reset subs, so we don't get type errors when specializing for a different signature env.subs.rollback_to(snapshot); From db362f6df096195dbb84b6504ee3837489f02b8f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:31:53 -0400 Subject: [PATCH 20/31] Reproduce named identity function bug --- compiler/gen/tests/test_gen.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index 1809593a57..2bfb722823 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -787,6 +787,21 @@ mod test_gen { ); } + #[test] + fn apply_identity_() { + assert_evals_to!( + indoc!( + r#" + identity = \a -> a + + identity 5 + "# + ), + 5, + i64 + ); + } + #[test] fn apply_unnamed_fn() { assert_evals_to!( From 173ba925ff192610733cf7188358715341e4b352 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:39:28 -0400 Subject: [PATCH 21/31] Formatting --- compiler/gen/tests/test_gen.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/gen/tests/test_gen.rs b/compiler/gen/tests/test_gen.rs index 2bfb722823..06103dd529 100644 --- a/compiler/gen/tests/test_gen.rs +++ b/compiler/gen/tests/test_gen.rs @@ -86,13 +86,13 @@ mod test_gen { // Declare all the Procs, then insert them into scope so their bodies // can look up their Funcs in scope later when calling each other by value. for (name, opt_proc) in procs.as_map().into_iter() { - if let Some(proc) = opt_proc { - let (func_id, sig) = declare_proc(&env, &mut module, name, &proc); + if let Some(proc) = opt_proc { + let (func_id, sig) = declare_proc(&env, &mut module, name, &proc); - declared.push((proc.clone(), sig.clone(), func_id)); + declared.push((proc.clone(), sig.clone(), func_id)); - scope.insert(name.clone(), ScopeEntry::Func { func_id, sig }); - } + scope.insert(name.clone(), ScopeEntry::Func { func_id, sig }); + } } for (proc, sig, fn_id) in declared { From 9b68fbe3c974a25ebd87458b71b9ba9569f04098 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:39:53 -0400 Subject: [PATCH 22/31] Add PRETTY_PRINT_DEBUG_SYMBOLS --- compiler/module/src/symbol.rs | 42 ++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index 5b3683a188..cce62d3652 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -9,6 +9,14 @@ use std::{fmt, u32}; #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct Symbol(u64); +// When this is `true` (which it normally should be), Symbol's Debug::fmt implementation +// attempts to pretty print debug symbols using interns recorded using +// register_debug_idents calls (which should be made in debug mode). +// Set it to false if you want to see the raw ModuleId and IdentId ints, +// but please set it back to true before checking in the result! +#[cfg(debug_assertions)] +const PRETTY_PRINT_DEBUG_SYMBOLS: bool = true; + /// In Debug builds only, Symbol has a name() method that lets /// you look up its name in a global intern table. This table is /// behind a mutex, so it is neither populated nor available in release builds. @@ -101,26 +109,30 @@ impl Symbol { impl fmt::Debug for Symbol { #[cfg(debug_assertions)] fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let module_id = self.module_id(); - let ident_id = self.ident_id(); + if PRETTY_PRINT_DEBUG_SYMBOLS { + let module_id = self.module_id(); + let ident_id = self.ident_id(); - match DEBUG_IDENT_IDS_BY_MODULE_ID.lock() { - Ok(names) => match &names.get(&module_id.0) { - Some(ident_ids) => match ident_ids.get_name(ident_id) { - Some(ident_str) => write!(f, "`{:?}.{}`", module_id, ident_str), + match DEBUG_IDENT_IDS_BY_MODULE_ID.lock() { + Ok(names) => match &names.get(&module_id.0) { + Some(ident_ids) => match ident_ids.get_name(ident_id) { + Some(ident_str) => write!(f, "`{:?}.{}`", module_id, ident_str), + None => fallback_debug_fmt(*self, f), + }, None => fallback_debug_fmt(*self, f), }, - None => fallback_debug_fmt(*self, f), - }, - Err(err) => { - // Print and return Err rather than panicking, because this - // might be used in a panic error message, and if we panick - // while we're already panicking it'll kill the process - // without printing any of the errors! - println!("DEBUG INFO: Failed to acquire lock for Debug reading from DEBUG_IDENT_IDS_BY_MODULE_ID, presumably because a thread panicked: {:?}", err); + Err(err) => { + // Print and return Err rather than panicking, because this + // might be used in a panic error message, and if we panick + // while we're already panicking it'll kill the process + // without printing any of the errors! + println!("DEBUG INFO: Failed to acquire lock for Debug reading from DEBUG_IDENT_IDS_BY_MODULE_ID, presumably because a thread panicked: {:?}", err); - fallback_debug_fmt(*self, f) + fallback_debug_fmt(*self, f) + } } + } else { + fallback_debug_fmt(*self, f) } } From 494a8574bfc49c19a7d5d4019d5a0bbe4bcf0970 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:40:07 -0400 Subject: [PATCH 23/31] Drop obsolete add_closure function --- compiler/mono/src/expr.rs | 51 --------------------------------------- 1 file changed, 51 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index d664bbe63d..7dd13f6955 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -330,7 +330,6 @@ fn from_can<'a>( }, ); - // add_closure(env, symbol, loc_body.value, ret_var, &loc_args, procs) Expr::FunctionPointer(symbol) } @@ -546,56 +545,6 @@ fn from_can<'a>( } } -/* -fn add_closure<'a>( - env: &mut Env<'a, '_>, - symbol: Symbol, - can_body: roc_can::expr::Expr, - ret_var: Variable, - loc_args: &[(Variable, Located)], - procs: &mut Procs<'a>, -) -> Expr<'a> { - let subs = &env.subs; - let arena = env.arena; - let mut proc_args = Vec::with_capacity_in(loc_args.len(), arena); - - for (arg_var, loc_arg) in loc_args.iter() { - let layout = match Layout::from_var(arena, *arg_var, subs, env.pointer_size) { - Ok(layout) => layout, - Err(()) => { - // Invalid closure! - procs.insert(symbol, None); - - return Expr::FunctionPointer(symbol); - } - }; - - let arg_name: Symbol = match &loc_arg.value { - Pattern::Identifier(symbol) => *symbol, - _ => { - panic!("TODO determine arg_name for pattern {:?}", loc_arg.value); - } - }; - - proc_args.push((layout, arg_name)); - } - - let ret_layout = Layout::from_var(arena, ret_var, subs, env.pointer_size) - .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)); - - let proc = Proc { - args: proc_args.into_bump_slice(), - body: from_can(env, can_body, procs, None), - closes_over: Layout::Struct(&[]), - ret_layout, - }; - - procs.insert(symbol, Some(proc)); - - Expr::FunctionPointer(symbol) -} -*/ - fn store_pattern<'a>( env: &mut Env<'a, '_>, can_pat: Pattern, From 723ef8e6d087cb432973d0ecdc814d38b4494093 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:40:16 -0400 Subject: [PATCH 24/31] Add a missing register_debug_idents call --- compiler/mono/src/expr.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 7dd13f6955..284310d153 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -594,6 +594,8 @@ fn store_pattern<'a>( fn gen_closure_name(procs: &Procs<'_>, ident_ids: &mut IdentIds, home: ModuleId) -> Symbol { let ident_id = ident_ids.add(format!("_{}", procs.len()).into()); + home.register_debug_idents(&ident_ids); + Symbol::new(home, ident_id) } From f74471012ca9719266b4180667975816cfcae9ca Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 12 Mar 2020 00:53:24 -0400 Subject: [PATCH 25/31] Improve an error message --- compiler/gen/src/crane/build.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index b46c0d80b4..fcec1ba984 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -159,7 +159,10 @@ pub fn build_expr<'a, B: Backend>( Some(ScopeEntry::Func { .. }) => { panic!("TODO I don't yet know how to return fn pointers") } - None => panic!("Could not find a var for {:?} in scope {:?}", name, scope), + None => panic!( + "Could not resolve lookup for {:?} because no ScopeEntry was found for {:?} in scope {:?}", + name, name, scope + ), }, Struct { layout, fields } => { let cfg = env.cfg; From 9761aabe65173f070220375d83475cc29dd5c8b1 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 12:58:03 +0100 Subject: [PATCH 26/31] add is_empty to Procs --- compiler/mono/src/expr.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 284310d153..9c1cb5aea1 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -43,6 +43,10 @@ impl<'a> Procs<'a> { .sum() } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + fn insert_builtin(&mut self, symbol: Symbol) { self.builtin.insert(symbol); } From cfb3952fbfe26abc11afd161dace0e1780a207fc Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 13:20:26 +0100 Subject: [PATCH 27/31] put patterns into PartialProc So Load(symbol) finds a defined value --- compiler/mono/src/expr.rs | 40 ++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 9c1cb5aea1..60cd959372 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -71,6 +71,7 @@ impl<'a> Procs<'a> { #[derive(Clone, Debug, PartialEq)] pub struct PartialProc<'a> { pub annotation: Variable, + pub patterns: std::vec::Vec<(Variable, Located)>, pub body: roc_can::expr::Expr, pub specializations: MutMap>)>, } @@ -320,7 +321,7 @@ fn from_can<'a>( Expr::Store(stored.into_bump_slice(), arena.alloc(ret)) } - Closure(annotation, _, _, _loc_args, boxed_body) => { + Closure(annotation, _, _, loc_args, boxed_body) => { let (loc_body, _ret_var) = *boxed_body; let symbol = name.unwrap_or_else(|| gen_closure_name(procs, &mut env.ident_ids, env.home)); @@ -329,6 +330,7 @@ fn from_can<'a>( symbol, PartialProc { annotation, + patterns: loc_args, body: loc_body.value, specializations: MutMap::default(), }, @@ -868,7 +870,13 @@ fn call_by_name<'a>( // because if we tried to specialize the body inside that match, we would // get a borrow checker error about trying to borrow `procs` as mutable // while there is still an active immutable borrow. - let opt_specialize_body: Option<(ContentHash, Variable, roc_can::expr::Expr)>; + #[allow(clippy::type_complexity)] + let opt_specialize_body: Option<( + ContentHash, + Variable, + roc_can::expr::Expr, + std::vec::Vec<(Variable, Located)>, + )>; let specialized_proc_name = if let Some(partial_proc) = procs.get_user_defined(proc_name) { let content_hash = ContentHash::from_var(fn_var, env.subs); @@ -883,6 +891,7 @@ fn call_by_name<'a>( content_hash, partial_proc.annotation, partial_proc.body.clone(), + partial_proc.patterns.clone(), )); // generate a symbol for this specialization @@ -896,7 +905,7 @@ fn call_by_name<'a>( proc_name }; - if let Some((content_hash, annotation, body)) = opt_specialize_body { + if let Some((content_hash, annotation, body, loc_patterns)) = opt_specialize_body { // register proc, so specialization doesn't loop infinitely // for recursive definitions // let mut temp = partial_proc.clone(); @@ -913,6 +922,7 @@ fn call_by_name<'a>( ret_var, specialized_proc_name, &loc_args, + &loc_patterns, annotation, body, ); @@ -920,9 +930,6 @@ fn call_by_name<'a>( procs.insert_specialization(proc_name, content_hash, specialized_proc_name, proc); } - dbg!(proc_name); - dbg!(specialized_proc_name); - // generate actual call let mut args = Vec::with_capacity_in(loc_args.len(), env.arena); @@ -936,6 +943,7 @@ fn call_by_name<'a>( Expr::CallByName(specialized_proc_name, args.into_bump_slice()) } +#[allow(clippy::too_many_arguments)] fn specialize_proc_body<'a>( env: &mut Env<'a, '_>, procs: &mut Procs<'a>, @@ -943,6 +951,7 @@ fn specialize_proc_body<'a>( ret_var: Variable, proc_name: Symbol, loc_args: &[(Variable, Located)], + loc_patterns: &[(Variable, Located)], annotation: Variable, body: roc_can::expr::Expr, ) -> Option> { @@ -955,7 +964,7 @@ fn specialize_proc_body<'a>( let mut proc_args = Vec::with_capacity_in(loc_args.len(), &env.arena); - for (arg_var, _loc_arg) in loc_args.iter() { + for ((arg_var, _), (_, loc_pattern)) in loc_args.iter().zip(loc_patterns.iter()) { let layout = match Layout::from_var(&env.arena, *arg_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { @@ -966,14 +975,15 @@ fn specialize_proc_body<'a>( // TODO FIXME what is the idea here? arguments don't map to identifiers one-to-one // e.g. underscore and record patterns - let arg_name = proc_name; - - // let arg_name: Symbol = match &loc_arg.value { - // Pattern::Identifier(symbol) => *symbol, - // _ => { - // panic!("TODO determine arg_name for pattern {:?}", loc_arg.value); - // } - // }; + let arg_name: Symbol = match &loc_pattern.value { + Pattern::Identifier(symbol) => *symbol, + _ => { + panic!( + "TODO determine arg_name for pattern {:?}", + loc_pattern.value + ); + } + }; proc_args.push((layout, arg_name)); } From de40cf62f3ac203450e7e0138d9311e7baa6246f Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 14:03:26 +0100 Subject: [PATCH 28/31] special-case anonymous functions --- compiler/mono/src/expr.rs | 91 +++++++++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 60cd959372..558c60b893 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -12,6 +12,7 @@ use roc_types::subs::{Content, ContentHash, FlatType, Subs, Variable}; #[derive(Clone, Debug, PartialEq, Default)] pub struct Procs<'a> { user_defined: MutMap>, + anonymous: MutMap>>, builtin: MutSet, } @@ -20,6 +21,10 @@ impl<'a> Procs<'a> { self.user_defined.insert(symbol, partial_proc); } + fn insert_anonymous(&mut self, symbol: Symbol, proc: Option>) { + self.anonymous.insert(symbol, proc); + } + fn insert_specialization( &mut self, symbol: Symbol, @@ -37,10 +42,14 @@ impl<'a> Procs<'a> { } pub fn len(&self) -> usize { - self.user_defined + let anonymous: usize = self.anonymous.len(); + let user_defined: usize = self + .user_defined .values() .map(|v| v.specializations.len()) - .sum() + .sum(); + + anonymous + user_defined } pub fn is_empty(&self) -> bool { @@ -60,6 +69,10 @@ impl<'a> Procs<'a> { } } + for (symbol, proc) in self.anonymous.clone().into_iter() { + result.insert(symbol, proc); + } + for symbol in self.builtin.iter() { result.insert(*symbol, None); } @@ -71,7 +84,7 @@ impl<'a> Procs<'a> { #[derive(Clone, Debug, PartialEq)] pub struct PartialProc<'a> { pub annotation: Variable, - pub patterns: std::vec::Vec<(Variable, Located)>, + pub patterns: std::vec::Vec>, pub body: roc_can::expr::Expr, pub specializations: MutMap>)>, } @@ -322,19 +335,46 @@ fn from_can<'a>( } Closure(annotation, _, _, loc_args, boxed_body) => { - let (loc_body, _ret_var) = *boxed_body; - let symbol = - name.unwrap_or_else(|| gen_closure_name(procs, &mut env.ident_ids, env.home)); + let (loc_body, ret_var) = *boxed_body; - procs.insert_user_defined( - symbol, - PartialProc { - annotation, - patterns: loc_args, - body: loc_body.value, - specializations: MutMap::default(), - }, - ); + let (arg_vars, patterns): (std::vec::Vec<_>, std::vec::Vec<_>) = + loc_args.into_iter().unzip(); + + let symbol = match name { + Some(symbol) => { + // a named closure + procs.insert_user_defined( + symbol, + PartialProc { + annotation, + patterns, + body: loc_body.value, + specializations: MutMap::default(), + }, + ); + symbol + } + None => { + // an anonymous closure. These will always be specialized already + // by the surrounding context + let symbol = gen_closure_name(procs, &mut env.ident_ids, env.home); + let opt_proc = specialize_proc_body( + env, + procs, + annotation, + ret_var, + symbol, + &arg_vars, + &patterns, + annotation, + loc_body.value, + ); + + procs.insert_anonymous(symbol, opt_proc); + + symbol + } + }; Expr::FunctionPointer(symbol) } @@ -875,7 +915,7 @@ fn call_by_name<'a>( ContentHash, Variable, roc_can::expr::Expr, - std::vec::Vec<(Variable, Located)>, + std::vec::Vec>, )>; let specialized_proc_name = if let Some(partial_proc) = procs.get_user_defined(proc_name) { @@ -907,21 +947,17 @@ fn call_by_name<'a>( if let Some((content_hash, annotation, body, loc_patterns)) = opt_specialize_body { // register proc, so specialization doesn't loop infinitely - // for recursive definitions - // let mut temp = partial_proc.clone(); - // temp.specializations - // .insert(content_hash, (spec_proc_name, None)); - // procs.insert_user_defined(proc_name, temp); - procs.insert_specialization(proc_name, content_hash, specialized_proc_name, None); + let arg_vars = loc_args.iter().map(|v| v.0).collect::>(); + let proc = specialize_proc_body( env, procs, fn_var, ret_var, specialized_proc_name, - &loc_args, + &arg_vars, &loc_patterns, annotation, body, @@ -950,21 +986,22 @@ fn specialize_proc_body<'a>( fn_var: Variable, ret_var: Variable, proc_name: Symbol, - loc_args: &[(Variable, Located)], - loc_patterns: &[(Variable, Located)], + loc_args: &[Variable], + loc_patterns: &[Located], annotation: Variable, body: roc_can::expr::Expr, ) -> Option> { // unify the called function with the specialized signature, then specialize the function body let snapshot = env.subs.snapshot(); - roc_unify::unify::unify(env.subs, annotation, fn_var); + let unified = roc_unify::unify::unify(env.subs, annotation, fn_var); + debug_assert!(unified.mismatches.is_empty()); let specialized_body = from_can(env, body, procs, None); // reset subs, so we don't get type errors when specializing for a different signature env.subs.rollback_to(snapshot); let mut proc_args = Vec::with_capacity_in(loc_args.len(), &env.arena); - for ((arg_var, _), (_, loc_pattern)) in loc_args.iter().zip(loc_patterns.iter()) { + for (arg_var, loc_pattern) in loc_args.iter().zip(loc_patterns.iter()) { let layout = match Layout::from_var(&env.arena, *arg_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { From f372e4d108c9efc12ec01c9dd4f4329425b09777 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 16:55:45 +0100 Subject: [PATCH 29/31] move fresh symbol generation into Env --- compiler/mono/src/expr.rs | 27 ++++++---- compiler/mono/tests/test_mono.rs | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 10 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 558c60b893..1197e9344d 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -104,6 +104,20 @@ struct Env<'a, 'i> { pub home: ModuleId, pub ident_ids: &'i mut IdentIds, pub pointer_size: u32, + symbol_counter: usize, +} + +impl<'a, 'i> Env<'a, 'i> { + pub fn fresh_symbol(&mut self) -> Symbol { + let ident_id = self + .ident_ids + .add(format!("_{}", self.symbol_counter).into()); + self.symbol_counter += 1; + + self.home.register_debug_idents(&self.ident_ids); + + Symbol::new(self.home, ident_id) + } } #[derive(Clone, Debug, PartialEq)] @@ -206,6 +220,7 @@ impl<'a> Expr<'a> { home, ident_ids, pointer_size, + symbol_counter: 0, }; from_can(&mut env, can_expr, procs, None) @@ -357,7 +372,7 @@ fn from_can<'a>( None => { // an anonymous closure. These will always be specialized already // by the surrounding context - let symbol = gen_closure_name(procs, &mut env.ident_ids, env.home); + let symbol = env.fresh_symbol(); let opt_proc = specialize_proc_body( env, procs, @@ -637,14 +652,6 @@ fn store_pattern<'a>( } } -fn gen_closure_name(procs: &Procs<'_>, ident_ids: &mut IdentIds, home: ModuleId) -> Symbol { - let ident_id = ident_ids.add(format!("_{}", procs.len()).into()); - - home.register_debug_idents(&ident_ids); - - Symbol::new(home, ident_id) -} - fn from_can_when<'a>( env: &mut Env<'a, '_>, cond_var: Variable, @@ -935,7 +942,7 @@ fn call_by_name<'a>( )); // generate a symbol for this specialization - gen_closure_name(procs, &mut env.ident_ids, env.home) + env.fresh_symbol() } } else { opt_specialize_body = None; diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 65ae5ea18e..4e29588d34 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -160,6 +160,92 @@ mod test_mono { ) } + #[test] + fn polymorphic_identity() { + compiles_to( + r#" + id = \x -> x + + id { x: id 0x4 } + "#, + { + use self::Builtin::*; + use Layout::Builtin; + let home = test_home(); + + let gen_symbol_3 = Interns::from_index(home, 3); + let gen_symbol_4 = Interns::from_index(home, 4); + + CallByName( + gen_symbol_3, + &[( + Struct { + fields: &[( + "x".into(), + CallByName(gen_symbol_4, &[(Int(4), Builtin(Int64))]), + )], + layout: Layout::Struct(&[("x".into(), Builtin(Int64))]), + }, + Layout::Struct(&[("x".into(), Builtin(Int64))]), + )], + ) + }, + ) + } + + // needs LetRec to be converted to mono + // #[test] + // fn polymorphic_recursive() { + // compiles_to( + // r#" + // f = \x -> + // when x < 10 is + // True -> f (x + 1) + // False -> x + // + // { x: f 0x4, y: f 3.14 } + // "#, + // { + // use self::Builtin::*; + // use Layout::Builtin; + // let home = test_home(); + // + // let gen_symbol_3 = Interns::from_index(home, 3); + // let gen_symbol_4 = Interns::from_index(home, 4); + // + // Float(3.4) + // + // }, + // ) + // } + + // needs layout for non-empty tag union + // #[test] + // fn is_nil() { + // let arena = Bump::new(); + // + // compiles_to_with_interns( + // r#" + // LinkedList a : [ Cons a (LinkedList a), Nil ] + // + // isNil : LinkedList a -> Bool + // isNil = \list -> + // when list is + // Nil -> True + // Cons _ _ -> False + // + // listInt : LinkedList Int + // listInt = Nil + // + // isNil listInt + // "#, + // |interns| { + // let home = test_home(); + // let var_is_nil = interns.symbol(home, "isNil".into()); + // }, + // ); + // } + #[test] fn bool_literal() { let arena = Bump::new(); From 4da01c720e92b5f6097140824e16ec6fd00d5440 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 21:32:58 +0100 Subject: [PATCH 30/31] monomorphize the ext_var of records/tag unions --- compiler/mono/src/expr.rs | 120 ++++++++++++++++---- compiler/mono/tests/test_mono.rs | 20 ++++ compiler/region/src/all.rs | 5 + compiler/types/src/pretty_print.rs | 16 +-- compiler/types/src/subs.rs | 174 +++++++++++++++++++++-------- 5 files changed, 258 insertions(+), 77 deletions(-) diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 1197e9344d..0fd6237211 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -84,7 +84,7 @@ impl<'a> Procs<'a> { #[derive(Clone, Debug, PartialEq)] pub struct PartialProc<'a> { pub annotation: Variable, - pub patterns: std::vec::Vec>, + pub patterns: Vec<'a, Symbol>, pub body: roc_can::expr::Expr, pub specializations: MutMap>)>, } @@ -284,6 +284,79 @@ fn to_int_or_float(subs: &Subs, var: Variable) -> IntOrFloat { } } +fn patterns_to_when<'a>( + env: &mut Env<'a, '_>, + patterns: std::vec::Vec<(Variable, Located)>, + body_var: Variable, + mut body: Located, +) -> ( + Vec<'a, Variable>, + Vec<'a, Symbol>, + Located, +) { + let mut arg_vars = Vec::with_capacity_in(patterns.len(), env.arena); + let mut symbols = Vec::with_capacity_in(patterns.len(), env.arena); + + for (pattern_var, pattern) in patterns.into_iter().rev() { + let (new_symbol, new_body) = pattern_to_when(env, pattern_var, pattern, body_var, body); + body = new_body; + symbols.push(new_symbol); + arg_vars.push(pattern_var); + } + + (arg_vars, symbols, body) +} + +/// turn irrefutable patterns into when. For example +/// +/// foo = \{ x } -> body +/// +/// Assuming the above program typechecks, the pattern match cannot fail +/// (it is irrefutable). It becomes +/// +/// foo = \r -> +/// when r is +/// { x } -> body +/// +/// conversion of one-pattern when expressions will do the most optimal thing +fn pattern_to_when<'a>( + env: &mut Env<'a, '_>, + pattern_var: Variable, + pattern: Located, + body_var: Variable, + body: Located, +) -> (Symbol, Located) { + use roc_can::expr::Expr::*; + use roc_can::pattern::Pattern::*; + + match &pattern.value { + Identifier(symbol) => (*symbol, body), + Underscore => { + // for underscore we generate a dummy Symbol + (env.fresh_symbol(), body) + } + + AppliedTag(_, _, _) | RecordDestructure(_, _) | Shadowed(_, _) | UnsupportedPattern(_) => { + let symbol = env.fresh_symbol(); + + let wrapped_body = When { + cond_var: pattern_var, + expr_var: body_var, + loc_cond: Box::new(Located::at_zero(Var(symbol))), + branches: vec![(pattern, body)], + }; + + (symbol, Located::at_zero(wrapped_body)) + } + + // These patters are refutable, and thus should never occur outside a `when` expression + IntLiteral(_) | NumLiteral(_,_) | FloatLiteral(_) | StrLiteral(_) => { + unreachable!("refutable pattern {:?} where irrefutable pattern is expected. This should never happen!", pattern.value) + } + + } +} + fn from_can<'a>( env: &mut Env<'a, '_>, can_expr: roc_can::expr::Expr, @@ -352,8 +425,16 @@ fn from_can<'a>( Closure(annotation, _, _, loc_args, boxed_body) => { let (loc_body, ret_var) = *boxed_body; - let (arg_vars, patterns): (std::vec::Vec<_>, std::vec::Vec<_>) = - loc_args.into_iter().unzip(); + // turn record/tag patterns into a when expression, e.g. + // + // foo = \{ x } -> body + // + // becomes + // + // foo = \r -> when r is { x } -> body + // + // conversion of one-pattern when expressions will do the most optimal thing + let (arg_vars, arg_symbols, body) = patterns_to_when(env, loc_args, ret_var, loc_body); let symbol = match name { Some(symbol) => { @@ -362,8 +443,8 @@ fn from_can<'a>( symbol, PartialProc { annotation, - patterns, - body: loc_body.value, + patterns: arg_symbols, + body: body.value, specializations: MutMap::default(), }, ); @@ -373,6 +454,11 @@ fn from_can<'a>( // an anonymous closure. These will always be specialized already // by the surrounding context let symbol = env.fresh_symbol(); + + // Has the side-effect of monomorphizing record types + // turning the ext_var into EmptyRecord or EmptyTagUnion + let _ = ContentHash::from_var(annotation, env.subs); + let opt_proc = specialize_proc_body( env, procs, @@ -380,9 +466,9 @@ fn from_can<'a>( ret_var, symbol, &arg_vars, - &patterns, + &arg_symbols, annotation, - loc_body.value, + body.value, ); procs.insert_anonymous(symbol, opt_proc); @@ -922,7 +1008,7 @@ fn call_by_name<'a>( ContentHash, Variable, roc_can::expr::Expr, - std::vec::Vec>, + Vec<'a, Symbol>, )>; let specialized_proc_name = if let Some(partial_proc) = procs.get_user_defined(proc_name) { @@ -994,7 +1080,7 @@ fn specialize_proc_body<'a>( ret_var: Variable, proc_name: Symbol, loc_args: &[Variable], - loc_patterns: &[Located], + pattern_symbols: &[Symbol], annotation: Variable, body: roc_can::expr::Expr, ) -> Option> { @@ -1008,7 +1094,7 @@ fn specialize_proc_body<'a>( let mut proc_args = Vec::with_capacity_in(loc_args.len(), &env.arena); - for (arg_var, loc_pattern) in loc_args.iter().zip(loc_patterns.iter()) { + for (arg_var, arg_name) in loc_args.iter().zip(pattern_symbols.iter()) { let layout = match Layout::from_var(&env.arena, *arg_var, env.subs, env.pointer_size) { Ok(layout) => layout, Err(()) => { @@ -1017,19 +1103,7 @@ fn specialize_proc_body<'a>( } }; - // TODO FIXME what is the idea here? arguments don't map to identifiers one-to-one - // e.g. underscore and record patterns - let arg_name: Symbol = match &loc_pattern.value { - Pattern::Identifier(symbol) => *symbol, - _ => { - panic!( - "TODO determine arg_name for pattern {:?}", - loc_pattern.value - ); - } - }; - - proc_args.push((layout, arg_name)); + proc_args.push((layout, *arg_name)); } let ret_layout = Layout::from_var(&env.arena, ret_var, env.subs, env.pointer_size) diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 4e29588d34..1d7103dd3e 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -160,6 +160,26 @@ mod test_mono { ) } + // #[test] + // fn record_pattern() { + // compiles_to( + // r#" + // \{ x } -> x + 0x5 + // "#, + // { Float(3.45) }, + // ) + // } + // + // #[test] + // fn tag_pattern() { + // compiles_to( + // r#" + // \Foo x -> x + 0x5 + // "#, + // { Float(3.45) }, + // ) + // } + #[test] fn polymorphic_identity() { compiles_to( diff --git a/compiler/region/src/all.rs b/compiler/region/src/all.rs index 772d0e0afc..5c9410bc0c 100644 --- a/compiler/region/src/all.rs +++ b/compiler/region/src/all.rs @@ -89,6 +89,11 @@ impl Located { pub fn at(region: Region, value: T) -> Located { Located { value, region } } + + pub fn at_zero(value: T) -> Located { + let region = Region::zero(); + Located { value, region } + } } impl Located { diff --git a/compiler/types/src/pretty_print.rs b/compiler/types/src/pretty_print.rs index f576a5ca50..0108b35618 100644 --- a/compiler/types/src/pretty_print.rs +++ b/compiler/types/src/pretty_print.rs @@ -436,7 +436,7 @@ fn write_flat_type( buf.push_str(" ]"); - if let Some(content) = ext_content { + if let Err(content) = ext_content { // This is an open tag union, so print the variable // right after the ']' // @@ -483,7 +483,7 @@ fn write_flat_type( buf.push_str(" ]"); - if let Some(content) = ext_content { + if let Err(content) = ext_content { // This is an open tag union, so print the variable // right after the ']' // @@ -508,10 +508,10 @@ pub fn chase_ext_tag_union( subs: &Subs, var: Variable, fields: &mut Vec<(TagName, Vec)>, -) -> Option { +) -> Result<(), Content> { use FlatType::*; match subs.get_without_compacting(var).content { - Content::Structure(EmptyTagUnion) => None, + Content::Structure(EmptyTagUnion) => Ok(()), Content::Structure(TagUnion(tags, ext_var)) | Content::Structure(RecursiveTagUnion(_, tags, ext_var)) => { for (label, vars) in tags { @@ -521,7 +521,7 @@ pub fn chase_ext_tag_union( chase_ext_tag_union(subs, ext_var, fields) } - content => Some(content), + content => Err(content), } } @@ -529,7 +529,7 @@ pub fn chase_ext_record( subs: &Subs, var: Variable, fields: &mut MutMap, -) -> Option { +) -> Result<(), Content> { use crate::subs::Content::*; use crate::subs::FlatType::*; @@ -540,11 +540,11 @@ pub fn chase_ext_record( chase_ext_record(subs, sub_ext, fields) } - Structure(EmptyRecord) => None, + Structure(EmptyRecord) => Ok(()), Alias(_, _, var) => chase_ext_record(subs, var, fields), - content => Some(content), + content => Err(content), } } diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 56b8b3a09a..3e8b42e3dc 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -554,7 +554,7 @@ pub enum FlatType { pub struct ContentHash(u64); impl ContentHash { - pub fn from_var(var: Variable, subs: &Subs) -> Self { + pub fn from_var(var: Variable, subs: &mut Subs) -> Self { use std::hash::Hasher; let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -563,14 +563,14 @@ impl ContentHash { ContentHash(hasher.finish()) } - pub fn from_var_help(var: Variable, subs: &Subs, hasher: &mut T) + pub fn from_var_help(var: Variable, subs: &mut Subs, hasher: &mut T) where T: std::hash::Hasher, { - Self::from_content_help(&subs.get_without_compacting(var).content, subs, hasher) + Self::from_content_help(var, &subs.get_without_compacting(var).content, subs, hasher) } - pub fn from_content_help(content: &Content, subs: &Subs, hasher: &mut T) + pub fn from_content_help(var: Variable, content: &Content, subs: &mut Subs, hasher: &mut T) where T: std::hash::Hasher, { @@ -581,7 +581,7 @@ impl ContentHash { } Content::Structure(flat_type) => { hasher.write_u8(0x10); - Self::from_flat_type_help(flat_type, subs, hasher) + Self::from_flat_type_help(var, flat_type, subs, hasher) } Content::FlexVar(_) | Content::RigidVar(_) => { hasher.write_u8(0x11); @@ -592,8 +592,12 @@ impl ContentHash { } } - pub fn from_flat_type_help(flat_type: &FlatType, subs: &Subs, hasher: &mut T) - where + pub fn from_flat_type_help( + flat_type_var: Variable, + flat_type: &FlatType, + subs: &mut Subs, + hasher: &mut T, + ) where T: std::hash::Hasher, { use std::hash::Hash; @@ -623,26 +627,50 @@ impl ContentHash { hasher.write_u8(2); } - FlatType::Record(fields, ext) => { + FlatType::Record(record_fields, ext) => { hasher.write_u8(3); - // We have to sort by the key, so this clone seems to be required - let mut fields = fields.clone(); + // NOTE: This function will modify the subs, putting all fields from the ext_var + // into the record itself, then setting the ext_var to EMPTY_RECORD - match crate::pretty_print::chase_ext_record(subs, *ext, &mut fields) { - Some(_) => panic!("Record with non-empty ext var"), - None => { - let mut fields_vec = Vec::with_capacity(fields.len()); - fields_vec.extend(fields.into_iter()); + let mut fields = Vec::with_capacity(record_fields.len()); - fields_vec.sort(); - - for (name, argument) in fields_vec { - name.hash(hasher); - Self::from_var_help(argument, subs, hasher); + let mut extracted_fields_from_ext = false; + if *ext != Variable::EMPTY_RECORD { + let mut fields_map = MutMap::default(); + match crate::pretty_print::chase_ext_record(subs, *ext, &mut fields_map) { + Err(Content::FlexVar(_)) | Ok(()) => { + if !fields_map.is_empty() { + extracted_fields_from_ext = true; + fields.extend(fields_map.into_iter()); + } } + Err(content) => panic!("Record with unexpected ext_var: {:?}", content), } } + + fields.extend(record_fields.clone().into_iter()); + fields.sort(); + + for (name, argument) in &fields { + name.hash(hasher); + Self::from_var_help(*argument, subs, hasher); + } + + if *ext != Variable::EMPTY_RECORD { + // unify ext with empty record + let desc = subs.get(Variable::EMPTY_RECORD); + subs.union(Variable::EMPTY_RECORD, *ext, desc); + } + + if extracted_fields_from_ext { + let fields_map = fields.into_iter().collect(); + + subs.set_content( + flat_type_var, + Content::Structure(FlatType::Record(fields_map, Variable::EMPTY_RECORD)), + ); + } } FlatType::EmptyTagUnion => { @@ -652,46 +680,100 @@ impl ContentHash { FlatType::TagUnion(tags, ext) => { hasher.write_u8(5); - // We have to sort by the key, so this clone seems to be required + // NOTE: This function will modify the subs, putting all tags from the ext_var + // into the tag union itself, then setting the ext_var to EMPTY_TAG_UNION + let mut tag_vec = Vec::with_capacity(tags.len()); - tag_vec.extend(tags.clone().into_iter()); - match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { - Some(_) => panic!("Tag union with non-empty ext var"), - None => { - tag_vec.sort(); - for (name, arguments) in tag_vec { - name.hash(hasher); - - for var in arguments { - Self::from_var_help(var, subs, hasher); - } + let mut extracted_fields_from_ext = false; + if *ext != Variable::EMPTY_TAG_UNION { + match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { + Err(Content::FlexVar(_)) | Ok(()) => { + extracted_fields_from_ext = !tag_vec.is_empty(); } + Err(content) => panic!("TagUnion with unexpected ext_var: {:?}", content), } } + + tag_vec.extend(tags.clone().into_iter()); + tag_vec.sort(); + for (name, arguments) in &tag_vec { + name.hash(hasher); + + for var in arguments { + Self::from_var_help(*var, subs, hasher); + } + } + + if *ext != Variable::EMPTY_TAG_UNION { + // unify ext with empty record + let desc = subs.get(Variable::EMPTY_TAG_UNION); + subs.union(Variable::EMPTY_TAG_UNION, *ext, desc); + } + + if extracted_fields_from_ext { + let fields_map = tag_vec.into_iter().collect(); + + subs.set_content( + flat_type_var, + Content::Structure(FlatType::TagUnion( + fields_map, + Variable::EMPTY_TAG_UNION, + )), + ); + } } - FlatType::RecursiveTagUnion(_rec, tags, ext) => { - // TODO should the rec_var be hashed in? + FlatType::RecursiveTagUnion(rec, tags, ext) => { + // NOTE: rec is not hashed in. If all the tags and their arguments are the same, + // then the recursive tag unions are the same hasher.write_u8(6); - // We have to sort by the key, so this clone seems to be required + // NOTE: This function will modify the subs, putting all tags from the ext_var + // into the tag union itself, then setting the ext_var to EMPTY_TAG_UNION + let mut tag_vec = Vec::with_capacity(tags.len()); - tag_vec.extend(tags.clone().into_iter()); - match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { - Some(_) => panic!("Tag union with non-empty ext var"), - None => { - tag_vec.sort(); - for (name, arguments) in tag_vec { - name.hash(hasher); - - for var in arguments { - Self::from_var_help(var, subs, hasher); - } + let mut extracted_fields_from_ext = false; + if *ext != Variable::EMPTY_TAG_UNION { + match crate::pretty_print::chase_ext_tag_union(subs, *ext, &mut tag_vec) { + Err(Content::FlexVar(_)) | Ok(()) => { + extracted_fields_from_ext = !tag_vec.is_empty(); + } + Err(content) => { + panic!("RecursiveTagUnion with unexpected ext_var: {:?}", content) } } } + + tag_vec.extend(tags.clone().into_iter()); + tag_vec.sort(); + for (name, arguments) in &tag_vec { + name.hash(hasher); + + for var in arguments { + Self::from_var_help(*var, subs, hasher); + } + } + + if *ext != Variable::EMPTY_TAG_UNION { + // unify ext with empty record + let desc = subs.get(Variable::EMPTY_TAG_UNION); + subs.union(Variable::EMPTY_TAG_UNION, *ext, desc); + } + + if extracted_fields_from_ext { + let fields_map = tag_vec.into_iter().collect(); + + subs.set_content( + flat_type_var, + Content::Structure(FlatType::RecursiveTagUnion( + *rec, + fields_map, + Variable::EMPTY_TAG_UNION, + )), + ); + } } FlatType::Boolean(boolean) => { From f02193b962fa1b0f5785d9e22c87c69bb0f75548 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 12 Mar 2020 22:56:58 +0100 Subject: [PATCH 31/31] fix new clippy warnings --- compiler/can/src/def.rs | 2 +- compiler/load/src/file.rs | 7 +++++-- compiler/parse/src/expr.rs | 7 +++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/compiler/can/src/def.rs b/compiler/can/src/def.rs index 13a0431cfc..ae462339cc 100644 --- a/compiler/can/src/def.rs +++ b/compiler/can/src/def.rs @@ -1326,7 +1326,7 @@ fn to_pending_def<'a>( } } - Err(_err) => panic!("TODO gracefully handle shadowing of type alias"), + Err(err) => panic!("TODO gracefully handle shadowing of type alias {:?}", err), } } diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 15e1051029..33aa4097dc 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -1048,8 +1048,11 @@ fn parse_and_constrain( (module, ident_ids, constraint, problems) } - Err(_runtime_error) => { - panic!("TODO gracefully handle module canonicalization error"); + Err(runtime_error) => { + panic!( + "TODO gracefully handle module canonicalization error {:?}", + runtime_error + ); } }; diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index dfaf6b1434..289a8e71d0 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -1310,8 +1310,11 @@ pub fn ident_etc<'a>(min_indent: u16) -> impl Parser<'a, Expr<'a>> { region: loc_arg.region, }); } - Err(_malformed) => { - panic!("TODO early return malformed pattern"); + Err(malformed) => { + panic!( + "TODO early return malformed pattern {:?}", + malformed + ); } } }