From 912fe3b59363b2c37e31062fc22874add2c30eee Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 01:04:44 +0100 Subject: [PATCH 01/25] Fix pattern constraint gen --- src/can/mod.rs | 220 ++++++++++++++------------------------------ src/can/pattern.rs | 27 ++---- src/infer.rs | 2 +- tests/test_infer.rs | 20 ++-- 4 files changed, 91 insertions(+), 178 deletions(-) diff --git a/src/can/mod.rs b/src/can/mod.rs index 4a4dd2266e..f1e4d70ae0 100644 --- a/src/can/mod.rs +++ b/src/can/mod.rs @@ -4,6 +4,7 @@ use self::num::{ finish_parsing_bin, finish_parsing_float, finish_parsing_hex, finish_parsing_int, finish_parsing_oct, float_expr_from_result, int_expr_from_result, }; +use self::pattern::PatternState; use self::pattern::PatternType::*; use self::pattern::{canonicalize_pattern, Pattern}; use self::problem::Problem; @@ -471,12 +472,13 @@ fn canonicalize_expr( scope.idents = union_pairs(scope.idents, arg_idents.iter()); let mut state = PatternState { - assignment_types: ImMap::default(), + headers: ImMap::default(), vars: Vec::with_capacity(loc_arg_patterns.len()), constraints: Vec::with_capacity(1), }; - let args = constrain_args(loc_arg_patterns.iter(), &scope, subs, &mut state); let mut can_args: Vec> = Vec::with_capacity(loc_arg_patterns.len()); + let mut vars = Vec::with_capacity(state.vars.capacity()); + let mut pattern_types = Vec::with_capacity(state.vars.capacity()); for loc_pattern in loc_arg_patterns.into_iter() { // Exclude the current ident from shadowable_idents; you can't shadow yourself! @@ -485,10 +487,14 @@ fn canonicalize_expr( remove_idents(&loc_pattern.value, &mut shadowable_idents); let pattern_var = subs.mk_flex_var(); - let pattern_expected = PExpected::NoExpectation(Type::Variable(pattern_var)); + let pattern_type = Type::Variable(pattern_var); + let pattern_expected = PExpected::NoExpectation(pattern_type.clone()); - let (can_arg, _state) = canonicalize_pattern( + pattern_types.push(pattern_type); + + let can_arg = canonicalize_pattern( env, + &mut state, subs, &mut scope, FunctionArg, @@ -498,10 +504,19 @@ fn canonicalize_expr( pattern_expected, ); + vars.push(pattern_var); + can_args.push(can_arg); } - let body_type = NoExpectation(args.ret_type); + let ret_var = subs.mk_flex_var(); + let ret_type = Type::Variable(ret_var); + + state.vars.push(ret_var); + + let fn_typ = Type::Function(pattern_types, Box::new(ret_type.clone())); + + let body_type = NoExpectation(ret_type); let (loc_body_expr, mut output) = canonicalize_expr( rigids, env, @@ -523,17 +538,17 @@ fn canonicalize_expr( let typ = Variable(var); output.constraint = exists( - args.vars, + state.vars.clone(), And(vec![ Let(Box::new(LetConstraint { rigid_vars: Vec::new(), flex_vars: state.vars, - assignment_types: state.assignment_types, + assignment_types: state.headers, assignments_constraint, ret_constraint, })), // "the closure's type is equal to the var we've stored for later use in the proc" - Eq(args.typ, NoExpectation(typ.clone()), region), + Eq(fn_typ, NoExpectation(typ.clone()), region), // "the var we've stored for later is equal to the overall expected type" Eq(typ, expected, region), ]), @@ -566,7 +581,7 @@ fn canonicalize_expr( region, output.references.clone(), var, - args.ret_var, + ret_var, ); // Always return a function pointer, in case that's how the closure is being used (e.g. with Apply). @@ -847,8 +862,15 @@ fn canonicalize_case_branch<'a>( } } - let (loc_can_pattern, state) = canonicalize_pattern( + let mut state = PatternState { + headers: ImMap::default(), + vars: Vec::with_capacity(1), + constraints: Vec::with_capacity(1), + }; + + let loc_can_pattern = canonicalize_pattern( env, + &mut state, subs, &mut scope, CaseBranch, @@ -1258,48 +1280,6 @@ impl Info { /// In elm/compiler this is called RTV - the "Rigid Type Variables" dictionary. // type BoundTypeVars = ImMap, Type>; -struct PatternState { - assignment_types: ImMap>, - vars: Vec, - constraints: Vec, -} - -impl PatternState { - pub fn add_pattern( - &mut self, - scope: &Scope, - loc_pattern: Located, - expected: Expected, - ) { - let region = loc_pattern.region; - - match loc_pattern.value { - ast::Pattern::Identifier(name) => { - let symbol = scope.symbol(&name); - - self.add_to_assignment_types(region, symbol, expected) - } - ast::Pattern::Underscore => (), - _ => panic!("TODO other patterns"), - } - } - - fn add_to_assignment_types( - &mut self, - region: Region, - symbol: Symbol, - expected: Expected, - ) { - self.assignment_types.insert( - symbol, - Located { - region, - value: expected.get_type(), - }, - ); - } -} - fn add_pattern_to_lookup_types<'a>( scope: &Scope, loc_pattern: Located>, @@ -1367,6 +1347,10 @@ fn can_defs<'a>( let iter = defs.iter(); for loc_def in iter { + // Make types for the body expr, even if we won't end up having a body. + let expr_var = subs.mk_flex_var(); + let expr_type = Type::Variable(expr_var); + // Each assignment gets to have all the idents in scope that are assigned in this // block. Order of assignments doesn't matter, thanks to referential transparency! let (opt_loc_pattern, (loc_can_expr, can_output)) = match loc_def.value { @@ -1400,37 +1384,6 @@ fn can_defs<'a>( (None, (loc_expr, Output::new(True))) } Def::Body(ref loc_pattern, loc_expr) => { - // Make types for the pattern and the body expr. - let expr_var = subs.mk_flex_var(); - let expr_type = Type::Variable(expr_var); - let pattern_var = subs.mk_flex_var(); - let pattern_type = Type::Variable(pattern_var); - - flex_info.vars.push(pattern_var); - - let mut state = PatternState { - assignment_types: ImMap::default(), - vars: Vec::with_capacity(1), - constraints: Vec::with_capacity(1), - }; - - state.add_pattern( - &scope, - loc_pattern.clone(), - NoExpectation(pattern_type.clone()), - ); - let def_constraint = And(state.constraints); - - // Any time there's a lookup on this symbol in the outer Let, - // it should result in this expression's type. After all, this - // is the type to which this symbol is assigned! - add_pattern_to_lookup_types( - &scope, - loc_pattern.clone(), - &mut flex_info.assignment_types, - expr_type.clone(), - ); - let (loc_can_expr, output) = canonicalize_expr( rigids, env, @@ -1438,17 +1391,9 @@ fn can_defs<'a>( &mut scope, loc_expr.region, &loc_expr.value, - NoExpectation(expr_type), + NoExpectation(expr_type.clone()), ); - flex_info.constraints.push(Let(Box::new(LetConstraint { - rigid_vars: Vec::new(), - flex_vars: state.vars, - assignment_types: state.assignment_types, - assignments_constraint: def_constraint, - ret_constraint: output.constraint.clone(), - }))); - (Some(loc_pattern), (loc_can_expr, output)) } Def::CustomType(_, _) => { @@ -1470,10 +1415,18 @@ fn can_defs<'a>( remove_idents(&loc_pattern.value, &mut shadowable_idents); let pattern_var = subs.mk_flex_var(); - let pattern_expected = PExpected::NoExpectation(Type::Variable(pattern_var)); + let pattern_type = Type::Variable(pattern_var); + let pattern_expected = PExpected::NoExpectation(pattern_type); - let (loc_can_pattern, _state) = canonicalize_pattern( + let mut state = PatternState { + headers: ImMap::default(), + vars: Vec::with_capacity(1), + constraints: Vec::with_capacity(1), + }; + + let loc_can_pattern = canonicalize_pattern( env, + &mut state, subs, &mut scope, Assignment, @@ -1482,6 +1435,28 @@ fn can_defs<'a>( &mut shadowable_idents, pattern_expected, ); + + flex_info.vars.push(pattern_var); + + // Any time there's a lookup on this symbol in the outer Let, + // it should result in this expression's type. After all, this + // is the type to which this symbol is assigned! + add_pattern_to_lookup_types( + &scope, + // TODO can we we avoid this clone? + loc_pattern.clone(), + &mut flex_info.assignment_types, + expr_type.clone(), + ); + + flex_info.constraints.push(Let(Box::new(LetConstraint { + rigid_vars: Vec::new(), + flex_vars: state.vars, + assignment_types: state.headers, + assignments_constraint: And(state.constraints), + ret_constraint: can_output.constraint.clone(), + }))); + let mut renamed_closure_assignment: Option<&Symbol> = None; // Give closures names (and tail-recursive status) where appropriate. @@ -1745,60 +1720,3 @@ fn can_defs<'a>( } } } - -struct Args { - pub vars: Vec, - pub typ: Type, - pub ret_type: Type, - pub ret_var: Variable, -} - -fn constrain_args<'a, I>(args: I, scope: &Scope, subs: &mut Subs, state: &mut PatternState) -> Args -where - I: Iterator>>, -{ - let (mut vars, arg_types) = patterns_to_variables(args, scope, subs, state); - - let ret_var = subs.mk_flex_var(); - let ret_type = Type::Variable(ret_var); - - vars.push(ret_var); - - let typ = Type::Function(arg_types, Box::new(ret_type.clone())); - - Args { - vars, - typ, - ret_type, - ret_var, - } -} - -fn patterns_to_variables<'a, I>( - patterns: I, - scope: &Scope, - subs: &mut Subs, - state: &mut PatternState, -) -> (Vec, Vec) -where - I: Iterator>>, -{ - let mut vars = Vec::with_capacity(state.vars.capacity()); - let mut pattern_types = Vec::with_capacity(state.vars.capacity()); - - for loc_pattern in patterns { - let pattern_var = subs.mk_flex_var(); - let pattern_type = Type::Variable(pattern_var); - - state.add_pattern( - scope, - loc_pattern.clone(), - NoExpectation(pattern_type.clone()), - ); - - vars.push(pattern_var); - pattern_types.push(pattern_type); - } - - (vars, pattern_types) -} diff --git a/src/can/pattern.rs b/src/can/pattern.rs index 3a6b7a355f..aa414a3bd3 100644 --- a/src/can/pattern.rs +++ b/src/can/pattern.rs @@ -47,6 +47,7 @@ pub enum PatternType { pub fn canonicalize_pattern<'a>( env: &'a mut Env, + state: &'a mut PatternState, subs: &mut Subs, scope: &mut Scope, pattern_type: PatternType, @@ -54,7 +55,7 @@ pub fn canonicalize_pattern<'a>( region: Region, shadowable_idents: &'a mut ImMap, expected: PExpected, -) -> (Located, State) { +) -> Located { use self::PatternType::*; use can::ast::Pattern::*; @@ -245,6 +246,7 @@ pub fn canonicalize_pattern<'a>( &SpaceBefore(sub_pattern, _) | SpaceAfter(sub_pattern, _) => { return canonicalize_pattern( env, + state, subs, scope, pattern_type, @@ -257,21 +259,12 @@ pub fn canonicalize_pattern<'a>( _ => panic!("TODO finish restoring can_pattern branch for {:?}", pattern), }; - let mut state = State { - headers: ImMap::default(), - vars: Vec::new(), - constraints: Vec::new(), - }; + add_constraints(&pattern, region, expected, state); - add_constraints(&pattern, region, expected, &mut state); - - ( - Located { - region, - value: can_pattern, - }, - state, - ) + Located { + region, + value: can_pattern, + } } /// When we detect an unsupported pattern type (e.g. 5 = 1 + 2 is unsupported because you can't @@ -284,7 +277,7 @@ fn unsupported_pattern(env: &mut Env, pattern_type: PatternType, region: Region) // CONSTRAIN -pub struct State { +pub struct PatternState { pub headers: ImMap>, pub vars: Vec, pub constraints: Vec, @@ -294,7 +287,7 @@ fn add_constraints<'a>( pattern: &'a ast::Pattern<'a>, region: Region, expected: PExpected, - state: &'a mut State, + state: &'a mut PatternState, ) { use parse::ast::Pattern::*; diff --git a/src/infer.rs b/src/infer.rs index 24708fdb6a..ffe90a4ffc 100644 --- a/src/infer.rs +++ b/src/infer.rs @@ -19,5 +19,5 @@ pub fn infer_expr( solve(&env, subs, constraint); - subs.get(expr_var).content + subs.get(dbg!(expr_var)).content } diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 26f31a9af1..064c9453a2 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -530,15 +530,17 @@ mod test_infer { "Int", ); } - // #[test] - // fn identity() { - // infer_eq( - // indoc!(r#" - // \val -> val - // "#), - // "a -> a" - // ); - // } + + fn identity() { + infer_eq( + indoc!( + r#" + \val -> val + "# + ), + "a -> a", + ); + } // #[test] // fn always_function() { From d82b7815c2a2029e096e25195a53da9773ff30bc Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 01:54:54 +0100 Subject: [PATCH 02/25] Canonicalize patterns with scope.symbol --- src/can/pattern.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/can/pattern.rs b/src/can/pattern.rs index aa414a3bd3..97ba0d0585 100644 --- a/src/can/pattern.rs +++ b/src/can/pattern.rs @@ -259,7 +259,7 @@ pub fn canonicalize_pattern<'a>( _ => panic!("TODO finish restoring can_pattern branch for {:?}", pattern), }; - add_constraints(&pattern, region, expected, state); + add_constraints(&pattern, &scope, region, expected, state); Located { region, @@ -285,6 +285,7 @@ pub struct PatternState { fn add_constraints<'a>( pattern: &'a ast::Pattern<'a>, + scope: &'a Scope, region: Region, expected: PExpected, state: &'a mut PatternState, @@ -297,7 +298,7 @@ fn add_constraints<'a>( } Identifier(name) => { state.headers.insert( - Symbol::new("TODO pass home into add_constraints, or improve Symbol to not need it for idents in patterns", name), + scope.symbol(name), Located { region, value: expected.get_type(), @@ -332,7 +333,7 @@ fn add_constraints<'a>( } SpaceBefore(pattern, _) | SpaceAfter(pattern, _) => { - add_constraints(pattern, region, expected, state) + add_constraints(pattern, scope, region, expected, state) } Variant(_, _) | Apply(_, _) | RecordDestructure(_) | EmptyRecordLiteral => { From a335632630262829ad630bf8067f8a6fb4679b92 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 01:55:03 +0100 Subject: [PATCH 03/25] Rename identity test to identity_function --- tests/test_infer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 064c9453a2..4f8875a517 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -531,7 +531,7 @@ mod test_infer { ); } - fn identity() { + fn identity_function() { infer_eq( indoc!( r#" From ffe088e686b5d3f0272f0cd05a4b42ca8a1290d4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:42:36 +0100 Subject: [PATCH 04/25] Name some Reason variants better --- src/can/mod.rs | 8 ++++++-- src/types.rs | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/can/mod.rs b/src/can/mod.rs index f1e4d70ae0..73ab65b31a 100644 --- a/src/can/mod.rs +++ b/src/can/mod.rs @@ -236,7 +236,9 @@ fn canonicalize_expr( let fn_region = loc_fn.region; let fn_expected = NoExpectation(fn_type.clone()); // TODO look up the name and use NamedFnArg if possible. - let fn_reason = Reason::AnonymousFnCall(loc_args.len() as u8); + let fn_reason = Reason::AnonymousFnCall { + arity: loc_args.len() as u8, + }; // Canonicalize the function expression and its arguments let (fn_expr, mut output) = canonicalize_expr( @@ -270,7 +272,9 @@ fn canonicalize_expr( let arg_var = subs.mk_flex_var(); let arg_type = Variable(arg_var); // TODO look up the name and use NamedFnArg if possible. - let reason = Reason::AnonymousFnArg(index as u8); + let reason = Reason::AnonymousFnArg { + arg_index: index as u8, + }; let expected_arg = ForReason(reason, arg_type.clone(), region); let (arg_expr, arg_out) = canonicalize_expr( rigids, diff --git a/src/types.rs b/src/types.rs index 9080894660..743a45b2f1 100644 --- a/src/types.rs +++ b/src/types.rs @@ -184,9 +184,9 @@ pub enum AnnotationSource { #[derive(Debug, Clone, PartialEq, Eq)] pub enum Reason { - AnonymousFnArg(u8 /* arg index */), + AnonymousFnArg { arg_index: u8 }, NamedFnArg(String /* function name */, u8 /* arg index */), - AnonymousFnCall(u8 /* arity */), + AnonymousFnCall { arity: u8 }, NamedFnCall(String /* function name */, u8 /* arity */), BinOpArg(BinOp, ArgSide), BinOpRet(BinOp), From b6f862b7d675e397bc86fd525c7e902181d597ec Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:42:46 +0100 Subject: [PATCH 05/25] Cleanup --- src/infer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/infer.rs b/src/infer.rs index ffe90a4ffc..24708fdb6a 100644 --- a/src/infer.rs +++ b/src/infer.rs @@ -19,5 +19,5 @@ pub fn infer_expr( solve(&env, subs, constraint); - subs.get(dbg!(expr_var)).content + subs.get(expr_var).content } From bf2e75a53548d85c759e4f3edfc6df9780406466 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:43:10 +0100 Subject: [PATCH 06/25] Name all type vars --- src/pretty_print_types.rs | 73 +++++++++++++++++++++++++++++++++++++++ src/subs.rs | 4 +++ tests/test_infer.rs | 5 ++- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index d4d79f29a7..5cc40a9749 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -4,6 +4,79 @@ use types; static WILDCARD: &str = "*"; static EMPTY_RECORD: &str = "{}"; +/// Generate names for all type variables, replacing FlexVar(None) with +/// FlexVar(Some(name)) where appropriate. Example: for the identity +/// function, generate a name of "a" for both its argument and return +/// type variables. +pub fn name_all_type_vars(letters_used: usize, variable: Variable, subs: &mut Subs) { + use subs::Content::*; + use subs::FlatType::*; + + let mut letters_used = letters_used; + + match subs.get(variable).content { + Structure(Apply { + module_name: _, + name: _, + args, + }) => { + for var in args { + let root = subs.get_root_key(var); + + // If this var is *not* its own root, then the + // root var necessarily appears in multiple places. + // Generate a name for it! + if var != root { + name_root(letters_used, root, subs); + + letters_used += 1; + } + } + } + Structure(Func(arg_vars, ret_var)) => { + for var in arg_vars { + let root = subs.get_root_key(var); + + if var != root { + name_root(letters_used, root, subs); + + letters_used += 1; + } + } + + let root = subs.get_root_key(ret_var); + + if ret_var != root { + name_root(letters_used, root, subs); + } + } + _ => (), + } +} + +fn name_root(letters_used: usize, root: Variable, subs: &mut Subs) { + use subs::Content::*; + + let generated_name = if letters_used == 0 { + // This should generate "a", then "b", etc. + "a" + } else { + panic!("TODO finish type variable naming algorithm"); + }; + + let mut descriptor = subs.get(root); + + match descriptor.content { + FlexVar(None) => { + descriptor.content = FlexVar(Some(generated_name.into())); + + // TODO is this necessary, or was mutating descriptor in place sufficient? + subs.set(root, descriptor); + } + _ => (), + } +} + pub fn content_to_string(content: Content, subs: &mut Subs) -> String { let mut buf = String::new(); diff --git a/src/subs.rs b/src/subs.rs index fe0a3792b0..ddac38fe8c 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -67,6 +67,10 @@ impl Subs { self.utable.probe_value(key) } + pub fn get_root_key(&mut self, key: Variable) -> Variable { + self.utable.get_root_key(key) + } + pub fn set(&mut self, key: Variable, r_value: Descriptor) { let l_key = self.utable.get_root_key(key); let unified = unify::unify_var_val(self, l_key, &r_value); diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 4f8875a517..09d17fc56b 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -12,7 +12,7 @@ mod helpers; mod test_infer { use helpers::can_expr; use roc::infer::infer_expr; - use roc::pretty_print_types::content_to_string; + use roc::pretty_print_types::{content_to_string, name_all_type_vars}; // HELPERS @@ -20,6 +20,9 @@ mod test_infer { let (_, output, _, procedures, mut subs, variable) = can_expr(src); let content = infer_expr(&mut subs, procedures, &output.constraint, variable); + + name_all_type_vars(0, variable, &mut subs); + let actual_str = content_to_string(content, &mut subs); assert_eq!(actual_str, expected.to_string()); From 2c3307d4ce7801c99b86579367848bb700cc042d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:43:23 +0100 Subject: [PATCH 07/25] Drop obsolete FlatType::BinOp --- src/pretty_print_types.rs | 1 - src/subs.rs | 1 - src/unify.rs | 8 -------- 3 files changed, 10 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 5cc40a9749..56bcf27c6c 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -115,7 +115,6 @@ fn write_flat_type(flat_type: FlatType, subs: &mut Subs, buf: &mut String, use_p ), EmptyRecord => buf.push_str(EMPTY_RECORD), Func(args, ret) => write_fn(args, ret, subs, buf, use_parens), - BinOp(l_arg, r_arg, ret) => write_fn(vec![l_arg, r_arg], ret, subs, buf, use_parens), Erroneous(problem) => { buf.push_str(&format!("", problem)); } diff --git a/src/subs.rs b/src/subs.rs index ddac38fe8c..b00123981d 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -149,7 +149,6 @@ pub enum FlatType { args: Vec, }, Func(Vec, Variable), - BinOp(Variable, Variable, Variable), Erroneous(Problem), EmptyRecord, } diff --git a/src/unify.rs b/src/unify.rs index 7dde1ed1c2..3255698231 100644 --- a/src/unify.rs +++ b/src/unify.rs @@ -90,14 +90,6 @@ fn unify_flat_type(subs: &mut Subs, left: &FlatType, right: &FlatType) -> Descri from_content(Error(Problem::MissingArguments)) } } - (BinOp(l_l_arg, l_r_arg, l_ret), BinOp(r_l_arg, r_r_arg, r_ret)) => { - let l_arg = union_vars(subs, *l_l_arg, *r_l_arg); - let r_arg = union_vars(subs, *l_r_arg, *r_r_arg); - let ret = union_vars(subs, *l_ret, *r_ret); - let flat_type = BinOp(l_arg, r_arg, ret); - - from_content(Structure(flat_type)) - } _ => from_content(Error(Problem::GenericMismatch)), } } From 9ee5fad6ab0bed08b7ca3d235cd4f71a38858e0b Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:43:34 +0100 Subject: [PATCH 08/25] Remove unnecessary ref --- src/solve.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/solve.rs b/src/solve.rs index 3a046fdcdc..efb3a8bfed 100644 --- a/src/solve.rs +++ b/src/solve.rs @@ -39,13 +39,13 @@ pub fn solve(env: &Env, subs: &mut Subs, constraint: &Constraint) { subs.union(actual, expected); } Let(let_con) => { - match let_con.ret_constraint { + match &let_con.ret_constraint { True => { // If the return expression is guaranteed to solve, // solve the assignments themselves and move on. solve(env, subs, &let_con.assignments_constraint) } - ref ret_con => { + ret_con => { // Solve the assignments' constraints first. solve(env, subs, &let_con.assignments_constraint); From cb47a2f16ff53da0757610a8aa16f9f515facf2a Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:43:41 +0100 Subject: [PATCH 09/25] Add anonymous identity function test --- tests/test_infer.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 09d17fc56b..92f01f5280 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -534,6 +534,19 @@ mod test_infer { ); } + #[test] + fn anonymous_identity() { + infer_eq( + indoc!( + r#" + (\a -> a) 3.14 + "# + ), + "Float", + ); + } + + #[test] fn identity_function() { infer_eq( indoc!( From 90b8db4fbe263a24fa8b0a467ba0ac7518e02b9f Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:45:36 +0100 Subject: [PATCH 10/25] Restore always_function test --- tests/test_infer.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 92f01f5280..434ea7e345 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -558,15 +558,17 @@ mod test_infer { ); } - // #[test] - // fn always_function() { - // infer_eq( - // indoc!(r#" - // \val -> \_ -> val - // "#), - // "a -> (* -> a)" - // ); - // } + #[test] + fn always_function() { + infer_eq( + indoc!( + r#" + \val -> \_ -> val + "# + ), + "a -> (* -> a)", + ); + } // OPERATORS From dd014b7794cdc686c65517efe653f7ea618375be Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 14 Nov 2019 08:45:40 +0100 Subject: [PATCH 11/25] Drop obsolete comment --- tests/test_infer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 434ea7e345..49595abd01 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -489,7 +489,6 @@ mod test_infer { } // TODO type annotations - // TODO fix identity inference // TODO BoundTypeVariables // TODO conditionals From c2e966ca3dc6b3822136b13f999cfcd454d304af Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 16:29:52 +0000 Subject: [PATCH 12/25] Make name_root support multiple letters --- src/pretty_print_types.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 56bcf27c6c..1f31d59fa8 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -3,12 +3,13 @@ use types; static WILDCARD: &str = "*"; static EMPTY_RECORD: &str = "{}"; +static THE_LETTER_A: u32 = 'a' as u32; /// Generate names for all type variables, replacing FlexVar(None) with /// FlexVar(Some(name)) where appropriate. Example: for the identity /// function, generate a name of "a" for both its argument and return /// type variables. -pub fn name_all_type_vars(letters_used: usize, variable: Variable, subs: &mut Subs) { +pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs) { use subs::Content::*; use subs::FlatType::*; @@ -54,14 +55,18 @@ pub fn name_all_type_vars(letters_used: usize, variable: Variable, subs: &mut Su } } -fn name_root(letters_used: usize, root: Variable, subs: &mut Subs) { +fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) { use subs::Content::*; - let generated_name = if letters_used == 0 { + // TODO we should arena-allocate this String, + // so all the strings in the entire pass only require ~1 allocation. + let generated_name = if letters_used < 26 { // This should generate "a", then "b", etc. - "a" + std::char::from_u32(THE_LETTER_A + letters_used) + .unwrap() + .to_string() } else { - panic!("TODO finish type variable naming algorithm"); + panic!("TODO generate aa, ab, ac, ..."); }; let mut descriptor = subs.get(root); From 90bc97b6d9ddd3bbdc1a26a00e86e3d60ddddf15 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 16:46:03 +0000 Subject: [PATCH 13/25] Format parens better in function types --- src/pretty_print_types.rs | 56 +++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 1f31d59fa8..ab84680c10 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -5,6 +5,27 @@ static WILDCARD: &str = "*"; static EMPTY_RECORD: &str = "{}"; static THE_LETTER_A: u32 = 'a' as u32; +/// Rerquirements for parentheses. +/// +/// If we're inside a function (that is, this is either an argument or a return +/// value), we may need to use parens. Examples: +/// +/// a -> (* -> a) +/// (* -> a) -> a +/// +/// Separately, if we're inside a type parameter, we may need to use parens: +/// +/// List Int +/// List (List Int) +/// +/// Otherwise, parens are unnecessary. +#[derive(Clone, Copy, Debug, PartialEq)] +enum Parens { + InFn, + InTypeParam, + Unnecessary, +} + /// Generate names for all type variables, replacing FlexVar(None) with /// FlexVar(Some(name)) where appropriate. Example: for the identity /// function, generate a name of "a" for both its argument and return @@ -85,24 +106,24 @@ fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) { pub fn content_to_string(content: Content, subs: &mut Subs) -> String { let mut buf = String::new(); - write_content(content, subs, &mut buf, false); + write_content(content, subs, &mut buf, Parens::Unnecessary); buf } -fn write_content(content: Content, subs: &mut Subs, buf: &mut String, use_parens: bool) { +fn write_content(content: Content, subs: &mut Subs, buf: &mut String, parens: Parens) { use subs::Content::*; match content { FlexVar(Some(name)) => buf.push_str(&name), FlexVar(None) => buf.push_str(WILDCARD), RigidVar(name) => buf.push_str(&name), - Structure(flat_type) => write_flat_type(flat_type, subs, buf, use_parens), + Structure(flat_type) => write_flat_type(flat_type, subs, buf, parens), Error(_) => buf.push_str(""), } } -fn write_flat_type(flat_type: FlatType, subs: &mut Subs, buf: &mut String, use_parens: bool) { +fn write_flat_type(flat_type: FlatType, subs: &mut Subs, buf: &mut String, parens: Parens) { use subs::FlatType::*; match flat_type { @@ -116,10 +137,10 @@ fn write_flat_type(flat_type: FlatType, subs: &mut Subs, buf: &mut String, use_p args, subs, buf, - use_parens, + parens, ), EmptyRecord => buf.push_str(EMPTY_RECORD), - Func(args, ret) => write_fn(args, ret, subs, buf, use_parens), + Func(args, ret) => write_fn(args, ret, subs, buf, parens), Erroneous(problem) => { buf.push_str(&format!("", problem)); } @@ -132,9 +153,9 @@ fn write_apply( args: Vec, subs: &mut Subs, buf: &mut String, - use_parens: bool, + parens: Parens, ) { - let write_parens = use_parens && !args.is_empty(); + let write_parens = parens == Parens::InTypeParam && !args.is_empty(); // Hardcoded type aliases if module_name == "Str" && type_name == "Str" { @@ -147,7 +168,7 @@ fn write_apply( let arg_content = subs.get(arg).content; let mut arg_param = String::new(); - write_content(arg_content, subs, &mut arg_param, true); + write_content(arg_content, subs, &mut arg_param, Parens::InTypeParam); if arg_param == "Int.Integer" { buf.push_str("Int"); @@ -178,7 +199,7 @@ fn write_apply( .unwrap_or_else(|| panic!("List did not have any type parameters somehow.")); let arg_content = subs.get(arg).content; - write_content(arg_content, subs, buf, true); + write_content(arg_content, subs, buf, Parens::InTypeParam); if write_parens { buf.push_str(")"); @@ -192,7 +213,7 @@ fn write_apply( for arg in args { buf.push_str(" "); - write_content(subs.get(arg).content, subs, buf, true); + write_content(subs.get(arg).content, subs, buf, Parens::InTypeParam); } if write_parens { @@ -201,14 +222,9 @@ fn write_apply( } } -fn write_fn( - args: Vec, - ret: Variable, - subs: &mut Subs, - buf: &mut String, - use_parens: bool, -) { +fn write_fn(args: Vec, ret: Variable, subs: &mut Subs, buf: &mut String, parens: Parens) { let mut needs_comma = false; + let use_parens = parens != Parens::Unnecessary; if use_parens { buf.push_str("("); @@ -221,11 +237,11 @@ fn write_fn( needs_comma = true; } - write_content(subs.get(arg).content, subs, buf, false); + write_content(subs.get(arg).content, subs, buf, Parens::InFn); } buf.push_str(" -> "); - write_content(subs.get(ret).content, subs, buf, false); + write_content(subs.get(ret).content, subs, buf, Parens::InFn); if use_parens { buf.push_str(")"); From 75166878e88545905ae0577ebdc129ff8bfaf905 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 17:40:37 +0000 Subject: [PATCH 14/25] Give a more descriptive panic when lookup fails --- src/can/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/can/mod.rs b/src/can/mod.rs index 73ab65b31a..40f870643b 100644 --- a/src/can/mod.rs +++ b/src/can/mod.rs @@ -1658,7 +1658,12 @@ fn can_defs<'a>( // As a bonus, the topological sort also reveals any cycles between the assignments, allowing // us to give a CircularAssignment error. let successors = |symbol: &Symbol| -> ImSet { - let (_, references) = refs_by_assignment.get(symbol).unwrap(); + let (_, references) = refs_by_assignment.get(symbol).unwrap_or_else(|| { + panic!( + "Could not find symbol {:?} in refs_by_assignment, which had: {:?}", + symbol, refs_by_assignment + ) + }); local_successors(&references, &env.procedures) }; From 377ba817953ef5db54bfd08bc866203d832cba81 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 17:41:00 +0000 Subject: [PATCH 15/25] Improve Debug display of union find structure --- src/ena/snapshot_vec.rs | 74 +++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/src/ena/snapshot_vec.rs b/src/ena/snapshot_vec.rs index 67a4de1ed8..e8796a9170 100644 --- a/src/ena/snapshot_vec.rs +++ b/src/ena/snapshot_vec.rs @@ -37,8 +37,13 @@ pub enum UndoLog { Other(D::Undo), } +/// A Vec where we have Debug overridden to render the indices like +/// a hashmap, since we really care about those when debugging one of these. +#[derive(Clone)] +struct BackingVec(Vec); + pub struct SnapshotVec { - values: Vec, + values: BackingVec, undo_log: Vec>, num_open_snapshots: usize, } @@ -78,13 +83,32 @@ pub trait SnapshotVecDelegate { impl Default for SnapshotVec { fn default() -> Self { SnapshotVec { - values: Vec::new(), + values: BackingVec(Vec::new()), undo_log: Vec::new(), num_open_snapshots: 0, } } } +impl fmt::Debug for BackingVec +where + T: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{{{{")?; + + for (index, elem) in self.0.iter().enumerate() { + write!(f, "\n {} => {:?},", index, elem)?; + } + + if !self.0.is_empty() { + write!(f, "\n")?; + } + + write!(f, "}}}}") + } +} + impl SnapshotVec { pub fn new() -> Self { Self::default() @@ -92,7 +116,7 @@ impl SnapshotVec { pub fn with_capacity(c: usize) -> SnapshotVec { SnapshotVec { - values: Vec::with_capacity(c), + values: BackingVec(Vec::with_capacity(c)), undo_log: Vec::new(), num_open_snapshots: 0, } @@ -109,16 +133,16 @@ impl SnapshotVec { } pub fn len(&self) -> usize { - self.values.len() + self.values.0.len() } pub fn is_empty(&self) -> bool { - self.values.len() == 0 + self.values.0.len() == 0 } pub fn push(&mut self, elem: D::Value) -> usize { - let len = self.values.len(); - self.values.push(elem); + let len = self.values.0.len(); + self.values.0.push(elem); if self.in_snapshot() { self.undo_log.push(NewElem(len)); @@ -128,26 +152,26 @@ impl SnapshotVec { } pub fn get(&self, index: usize) -> &D::Value { - &self.values[index] + &self.values.0[index] } /// Reserve space for new values, just like an ordinary vec. pub fn reserve(&mut self, additional: usize) { // This is not affected by snapshots or anything. - self.values.reserve(additional); + self.values.0.reserve(additional); } /// Returns a mutable pointer into the vec; whatever changes you make here cannot be undone /// automatically, so you should be sure call `record()` with some sort of suitable undo /// action. pub fn get_mut(&mut self, index: usize) -> &mut D::Value { - &mut self.values[index] + &mut self.values.0[index] } /// Updates the element at the given index. The old value will saved (and perhaps restored) if /// a snapshot is active. pub fn set(&mut self, index: usize, new_elem: D::Value) { - let old_elem = mem::replace(&mut self.values[index], new_elem); + let old_elem = mem::replace(&mut self.values.0[index], new_elem); if self.in_snapshot() { self.undo_log.push(SetElem(index, old_elem)); } @@ -157,11 +181,11 @@ impl SnapshotVec { /// otherwise equivalent to -- invoking `set` for each element. pub fn set_all(&mut self, mut new_elems: impl FnMut(usize) -> D::Value) { if !self.in_snapshot() { - for (index, slot) in self.values.iter_mut().enumerate() { + for (index, slot) in self.values.0.iter_mut().enumerate() { *slot = new_elems(index); } } else { - for i in 0..self.values.len() { + for i in 0..self.values.0.len() { self.set(i, new_elems(i)); } } @@ -173,16 +197,16 @@ impl SnapshotVec { D::Value: Clone, { if self.in_snapshot() { - let old_elem = self.values[index].clone(); + let old_elem = self.values.0[index].clone(); self.undo_log.push(SetElem(index, old_elem)); } - op(&mut self.values[index]); + op(&mut self.values.0[index]); } pub fn start_snapshot(&mut self) -> Snapshot { self.num_open_snapshots += 1; Snapshot { - value_count: self.values.len(), + value_count: self.values.0.len(), undo_len: self.undo_log.len(), } } @@ -205,16 +229,16 @@ impl SnapshotVec { while self.undo_log.len() > snapshot.undo_len { match self.undo_log.pop().unwrap() { NewElem(i) => { - self.values.pop(); - assert!(self.values.len() == i); + self.values.0.pop(); + assert!(self.values.0.len() == i); } SetElem(i, v) => { - self.values[i] = v; + self.values.0[i] = v; } Other(u) => { - D::reverse(&mut self.values, u); + D::reverse(&mut self.values.0, u); } } } @@ -244,13 +268,13 @@ impl SnapshotVec { impl ops::Deref for SnapshotVec { type Target = [D::Value]; fn deref(&self) -> &[D::Value] { - &*self.values + &*self.values.0 } } impl ops::DerefMut for SnapshotVec { fn deref_mut(&mut self) -> &mut [D::Value] { - &mut *self.values + &mut *self.values.0 } } @@ -272,9 +296,9 @@ impl Extend for SnapshotVec { where T: IntoIterator, { - let initial_len = self.values.len(); - self.values.extend(iterable); - let final_len = self.values.len(); + let initial_len = self.values.0.len(); + self.values.0.extend(iterable); + let final_len = self.values.0.len(); if self.in_snapshot() { self.undo_log.extend((initial_len..final_len).map(NewElem)); From 7382dc93bab84bb709dc7c8c489634923a4f6d7b Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 17:41:15 +0000 Subject: [PATCH 16/25] Use unwrap_or_else over unwrap --- src/pretty_print_types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index ab84680c10..5f885480bf 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -84,7 +84,7 @@ fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) { let generated_name = if letters_used < 26 { // This should generate "a", then "b", etc. std::char::from_u32(THE_LETTER_A + letters_used) - .unwrap() + .unwrap_or_else(|| panic!("Tried to convert {} to a char", THE_LETTER_A + letters_used)) .to_string() } else { panic!("TODO generate aa, ab, ac, ..."); From b26c4393cb920472a9ecd7771ca282467542ebbc Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 17:41:36 +0000 Subject: [PATCH 17/25] Add more identity inference tests --- tests/test_infer.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 49595abd01..f68501276f 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -545,6 +545,33 @@ mod test_infer { ); } + #[test] + fn identity_of_identity() { + infer_eq( + indoc!( + r#" + (\val -> val) (\val -> val) + "# + ), + "a -> a", + ); + } + + // #[test] + // TODO FIXME this should work, but instead causes a stack overflow! + // fn recursive_identity() { + // infer_eq( + // indoc!( + // r#" + // identity = \val -> val + + // identity identity + // "# + // ), + // "a -> a", + // ); + // } + #[test] fn identity_function() { infer_eq( From 394f33de8a86fa02a56aa9e6cfd2ec5c46d8f609 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 17:41:53 +0000 Subject: [PATCH 18/25] Add more function inference tests --- tests/test_infer.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index f68501276f..ed7674d875 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -584,6 +584,61 @@ mod test_infer { ); } + #[test] + fn use_apply() { + infer_eq( + indoc!( + r#" + apply = \f x -> f x + identity = \a -> a + + apply identity 5 + "# + ), + "Int", + ); + } + + #[test] + fn apply_function() { + infer_eq( + indoc!( + r#" + \f x -> f x + "# + ), + "(a -> b), a -> b", + ); + } + + #[test] + fn use_flip() { + infer_eq( + indoc!( + r#" + flip = \f -> (\a b -> f b a) + neverendingInt = \f int -> f int + x = neverendingInt (\a -> a) 5 + + flip neverendingInt + "# + ), + "(Int, (a -> a)) -> Int", + ); + } + + #[test] + fn flip_function() { + infer_eq( + indoc!( + r#" + \f -> (\a b -> f b a), + "# + ), + "(a, b -> c) -> (b, a -> c)", + ); + } + #[test] fn always_function() { infer_eq( @@ -596,6 +651,18 @@ mod test_infer { ); } + #[test] + fn pass_a_function() { + infer_eq( + indoc!( + r#" + \to_a -> to_a {} + "# + ), + "({} -> a) -> a", + ); + } + // OPERATORS // #[test] From c5882fe6676f0fb20e4a1f250705590f671df8d3 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 18:07:57 +0000 Subject: [PATCH 19/25] Fix some bugs in type var name gen --- src/pretty_print_types.rs | 64 ++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 5f885480bf..288c8c61a6 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -30,55 +30,54 @@ enum Parens { /// FlexVar(Some(name)) where appropriate. Example: for the identity /// function, generate a name of "a" for both its argument and return /// type variables. -pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs) { +pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs) -> u32 { use subs::Content::*; use subs::FlatType::*; let mut letters_used = letters_used; match subs.get(variable).content { + FlexVar(None) => { + let root = subs.get_root_key(variable); + + // If this var is *not* its own root, then the + // root var necessarily appears in multiple places. + // Generate a name for it! + if variable != root { + letters_used = name_root(letters_used, root, subs); + } + } + FlexVar(Some(name)) => { + let root = subs.get_root_key(variable); + + // Propagate name to root if necessary. + if variable != root { + set_root_name(root, &*name, subs); + } + } Structure(Apply { module_name: _, name: _, args, }) => { for var in args { - let root = subs.get_root_key(var); - - // If this var is *not* its own root, then the - // root var necessarily appears in multiple places. - // Generate a name for it! - if var != root { - name_root(letters_used, root, subs); - - letters_used += 1; - } + letters_used = name_all_type_vars(letters_used, var, subs); } } Structure(Func(arg_vars, ret_var)) => { for var in arg_vars { - let root = subs.get_root_key(var); - - if var != root { - name_root(letters_used, root, subs); - - letters_used += 1; - } + letters_used = name_all_type_vars(letters_used, var, subs); } - let root = subs.get_root_key(ret_var); - - if ret_var != root { - name_root(letters_used, root, subs); - } + letters_used = name_all_type_vars(letters_used, ret_var, subs); } _ => (), } + + letters_used } -fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) { - use subs::Content::*; - +fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) -> u32 { // TODO we should arena-allocate this String, // so all the strings in the entire pass only require ~1 allocation. let generated_name = if letters_used < 26 { @@ -90,15 +89,26 @@ fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) { panic!("TODO generate aa, ab, ac, ..."); }; + set_root_name(root, &generated_name, subs); + + letters_used + 1 +} + +fn set_root_name(root: Variable, name: &str, subs: &mut Subs) { + use subs::Content::*; + let mut descriptor = subs.get(root); match descriptor.content { FlexVar(None) => { - descriptor.content = FlexVar(Some(generated_name.into())); + descriptor.content = FlexVar(Some(name.into())); // TODO is this necessary, or was mutating descriptor in place sufficient? subs.set(root, descriptor); } + FlexVar(Some(_existing)) => { + panic!("TODO FIXME - make sure the generated name does not clash with any bound vars! In other words, if the user decided to name a type variable 'a', make sure we don't generate 'a' to name a different one!"); + } _ => (), } } From 47fdedc63e7568de117de32b347628d398793431 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 18:22:21 +0000 Subject: [PATCH 20/25] Avoid unnecessary work. --- src/pretty_print_types.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 288c8c61a6..cc179e5fe2 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -47,13 +47,8 @@ pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs letters_used = name_root(letters_used, root, subs); } } - FlexVar(Some(name)) => { - let root = subs.get_root_key(variable); - - // Propagate name to root if necessary. - if variable != root { - set_root_name(root, &*name, subs); - } + FlexVar(Some(_)) => { + // This root already has a name. Nothing to do here! } Structure(Apply { module_name: _, From 036a72e6e55db5fe1f703aa92933cf8036718fe2 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 18:50:37 +0000 Subject: [PATCH 21/25] Fix generated type var order --- src/pretty_print_types.rs | 64 +++++++++++++++++++++++++++++++++------ src/subs.rs | 2 +- tests/test_infer.rs | 2 +- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index cc179e5fe2..1af299165e 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -1,3 +1,4 @@ +use collections::MutMap; use subs::{Content, FlatType, Subs, Variable}; use types; @@ -26,25 +27,52 @@ enum Parens { Unnecessary, } +/// How many times a root variable appeared in Subs. +/// +/// We only care about whether it was a single time or multiple times, +/// because single appearances get a wildcard (*) and multiple times +/// get a generated letter ("a" etc). +enum Appearances { + Single, + Multiple, +} + /// Generate names for all type variables, replacing FlexVar(None) with /// FlexVar(Some(name)) where appropriate. Example: for the identity /// function, generate a name of "a" for both its argument and return /// type variables. -pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs) -> u32 { +/// +/// It's important that we track insertion order, which is why +/// names_needed is a Vec. We also want to count how many times a root +/// appears, because we should only generate a name for it if it appears +/// more than once. +fn find_names_needed( + variable: Variable, + subs: &mut Subs, + roots: &mut Vec, + root_appearances: &mut MutMap, +) { use subs::Content::*; use subs::FlatType::*; - let mut letters_used = letters_used; - match subs.get(variable).content { FlexVar(None) => { let root = subs.get_root_key(variable); // If this var is *not* its own root, then the // root var necessarily appears in multiple places. - // Generate a name for it! - if variable != root { - letters_used = name_root(letters_used, root, subs); + // We need a name for it! + match root_appearances.get(&root) { + Some(Appearances::Single) => { + root_appearances.insert(root, Appearances::Multiple); + } + Some(Appearances::Multiple) => { + // It's already multiple, so do nothing! + } + None => { + roots.push(root); + root_appearances.insert(root, Appearances::Single); + } } } FlexVar(Some(_)) => { @@ -56,20 +84,36 @@ pub fn name_all_type_vars(letters_used: u32, variable: Variable, subs: &mut Subs args, }) => { for var in args { - letters_used = name_all_type_vars(letters_used, var, subs); + find_names_needed(var, subs, roots, root_appearances); } } Structure(Func(arg_vars, ret_var)) => { for var in arg_vars { - letters_used = name_all_type_vars(letters_used, var, subs); + find_names_needed(var, subs, roots, root_appearances); } - letters_used = name_all_type_vars(letters_used, ret_var, subs); + find_names_needed(ret_var, subs, roots, root_appearances); } _ => (), } +} - letters_used +pub fn name_all_type_vars(variable: Variable, subs: &mut Subs) { + let mut roots = Vec::new(); + let mut letters_used = 0; + let mut appearances = MutMap::default(); + + // Populate names_needed + find_names_needed(variable, subs, &mut roots, &mut appearances); + + for root in roots { + match appearances.get(&root) { + Some(Appearances::Multiple) => { + letters_used = name_root(letters_used, root, subs); + } + _ => (), + } + } } fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) -> u32 { diff --git a/src/subs.rs b/src/subs.rs index b00123981d..86de1596d5 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -8,7 +8,7 @@ pub struct Subs { utable: UnificationTable>, } -#[derive(Copy, PartialEq, Eq, Clone)] +#[derive(Copy, PartialEq, Eq, Clone, Hash)] pub struct Variable(u32); impl Variable { diff --git a/tests/test_infer.rs b/tests/test_infer.rs index ed7674d875..bfd3619db6 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -21,7 +21,7 @@ mod test_infer { let content = infer_expr(&mut subs, procedures, &output.constraint, variable); - name_all_type_vars(0, variable, &mut subs); + name_all_type_vars(variable, &mut subs); let actual_str = content_to_string(content, &mut subs); From a2cc417a342995eb77145629bd94ecd7d38f5d75 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 19:05:10 +0000 Subject: [PATCH 22/25] Disable unrelated failing tests for now --- tests/test_infer.rs | 52 +++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index bfd3619db6..514ca1d380 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -611,21 +611,22 @@ mod test_infer { ); } - #[test] - fn use_flip() { - infer_eq( - indoc!( - r#" - flip = \f -> (\a b -> f b a) - neverendingInt = \f int -> f int - x = neverendingInt (\a -> a) 5 + // #[test] + // TODO FIXME this should pass, but instead fails to canonicalize + // fn use_flip() { + // infer_eq( + // indoc!( + // r#" + // flip = \f -> (\a b -> f b a) + // neverendingInt = \f int -> f int + // x = neverendingInt (\a -> a) 5 - flip neverendingInt - "# - ), - "(Int, (a -> a)) -> Int", - ); - } + // flip neverendingInt + // "# + // ), + // "(Int, (a -> a)) -> Int", + // ); + // } #[test] fn flip_function() { @@ -651,17 +652,18 @@ mod test_infer { ); } - #[test] - fn pass_a_function() { - infer_eq( - indoc!( - r#" - \to_a -> to_a {} - "# - ), - "({} -> a) -> a", - ); - } + // #[test] + // TODO this should pass, but instead infers the wrong type + // fn pass_a_function() { + // infer_eq( + // indoc!( + // r#" + // \to_a -> to_a {} + // "# + // ), + // "({} -> a) -> a", + // ); + // } // OPERATORS From 349f060495768b129dd3ebbb8cbbb78b8716a75c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 19:20:20 +0000 Subject: [PATCH 23/25] Don't generate names that collide with rigids --- src/pretty_print_types.rs | 34 ++++++++++++++++++++++++---------- src/subs.rs | 2 +- src/unify.rs | 2 +- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/pretty_print_types.rs b/src/pretty_print_types.rs index 1af299165e..17820337aa 100644 --- a/src/pretty_print_types.rs +++ b/src/pretty_print_types.rs @@ -1,4 +1,4 @@ -use collections::MutMap; +use collections::{MutMap, MutSet}; use subs::{Content, FlatType, Subs, Variable}; use types; @@ -51,6 +51,7 @@ fn find_names_needed( subs: &mut Subs, roots: &mut Vec, root_appearances: &mut MutMap, + names_taken: &mut MutSet, ) { use subs::Content::*; use subs::FlatType::*; @@ -84,17 +85,24 @@ fn find_names_needed( args, }) => { for var in args { - find_names_needed(var, subs, roots, root_appearances); + find_names_needed(var, subs, roots, root_appearances, names_taken); } } Structure(Func(arg_vars, ret_var)) => { for var in arg_vars { - find_names_needed(var, subs, roots, root_appearances); + find_names_needed(var, subs, roots, root_appearances, names_taken); } - find_names_needed(ret_var, subs, roots, root_appearances); + find_names_needed(ret_var, subs, roots, root_appearances, names_taken); + } + RigidVar(name) => { + // User-defined names are already taken. + // We must not accidentally generate names that collide with them! + names_taken.insert(name.to_string()); + } + Error(_) | Structure(Erroneous(_)) | Structure(EmptyRecord) => { + // Errors and empty records don't need names. } - _ => (), } } @@ -102,21 +110,22 @@ pub fn name_all_type_vars(variable: Variable, subs: &mut Subs) { let mut roots = Vec::new(); let mut letters_used = 0; let mut appearances = MutMap::default(); + let mut taken = MutSet::default(); // Populate names_needed - find_names_needed(variable, subs, &mut roots, &mut appearances); + find_names_needed(variable, subs, &mut roots, &mut appearances, &mut taken); for root in roots { match appearances.get(&root) { Some(Appearances::Multiple) => { - letters_used = name_root(letters_used, root, subs); + letters_used = name_root(letters_used, root, subs, &taken); } _ => (), } } } -fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) -> u32 { +fn name_root(letters_used: u32, root: Variable, subs: &mut Subs, taken: &MutSet) -> u32 { // TODO we should arena-allocate this String, // so all the strings in the entire pass only require ~1 allocation. let generated_name = if letters_used < 26 { @@ -128,9 +137,14 @@ fn name_root(letters_used: u32, root: Variable, subs: &mut Subs) -> u32 { panic!("TODO generate aa, ab, ac, ..."); }; - set_root_name(root, &generated_name, subs); + if taken.contains(&generated_name) { + // If the generated name is already taken, try again. + name_root(letters_used + 1, root, subs, taken) + } else { + set_root_name(root, &generated_name, subs); - letters_used + 1 + letters_used + 1 + } } fn set_root_name(root: Variable, name: &str, subs: &mut Subs) { diff --git a/src/subs.rs b/src/subs.rs index 86de1596d5..0a63f71dc4 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -136,7 +136,7 @@ impl From for Descriptor { #[derive(Clone, Debug, PartialEq, Eq)] pub enum Content { FlexVar(Option> /* name - e.g. in pattern matching */), - RigidVar(String /* name given in a user-written annotation */), + RigidVar(Box /* name given in a user-written annotation */), Structure(FlatType), Error(Problem), } diff --git a/src/unify.rs b/src/unify.rs index 3255698231..1c066877ef 100644 --- a/src/unify.rs +++ b/src/unify.rs @@ -129,7 +129,7 @@ fn unify_rigid(name: &str, other: &Content) -> Descriptor { match other { FlexVar(_) => { // If the other is flex, rigid wins! - from_content(RigidVar(name.to_string())) + from_content(RigidVar(name.into())) } RigidVar(_) | Structure(_) => { // Type mismatch! Rigid can only unify with flex, even if the From 0b61fcdc2588037391f3c1f14bd988fa682ebc72 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 19:35:01 +0000 Subject: [PATCH 24/25] Clarify some comments. --- src/subs.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/subs.rs b/src/subs.rs index 0a63f71dc4..8d84d9f7fa 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -135,8 +135,13 @@ impl From for Descriptor { #[derive(Clone, Debug, PartialEq, Eq)] pub enum Content { + /// A type variable which the user did not name in an annotation, + /// + /// When we auto-generate a type var name, e.g. the "a" in (a -> a), we + /// change the Option in here from None to Some. FlexVar(Option> /* name - e.g. in pattern matching */), - RigidVar(Box /* name given in a user-written annotation */), + /// name given in a user-written annotation + RigidVar(Box), Structure(FlatType), Error(Problem), } From 11acfd8d2a6c1a24b2723cc72a97d9605c0b12fe Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 16 Nov 2019 20:04:56 +0000 Subject: [PATCH 25/25] Fix invalid function arg name in test --- tests/test_infer.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/test_infer.rs b/tests/test_infer.rs index 514ca1d380..6f92320cff 100644 --- a/tests/test_infer.rs +++ b/tests/test_infer.rs @@ -652,18 +652,17 @@ mod test_infer { ); } - // #[test] - // TODO this should pass, but instead infers the wrong type - // fn pass_a_function() { - // infer_eq( - // indoc!( - // r#" - // \to_a -> to_a {} - // "# - // ), - // "({} -> a) -> a", - // ); - // } + #[test] + fn pass_a_function() { + infer_eq( + indoc!( + r#" + \f -> f {} + "# + ), + "({} -> a) -> a", + ); + } // OPERATORS