From 760de2f32854e7a70ec2b8b9b59ba006c5955cee Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 21 Dec 2019 19:47:11 -0500 Subject: [PATCH 1/2] Rename Rank::outermost to Rank::toplevel --- src/solve.rs | 10 +++++----- src/subs.rs | 10 ++-------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/solve.rs b/src/solve.rs index 3bff8b2f11..3b99fb8260 100644 --- a/src/solve.rs +++ b/src/solve.rs @@ -75,7 +75,7 @@ pub fn run( vars_by_symbol: vars_by_symbol.clone(), mark: Mark::none().next(), }; - let rank = Rank::outermost(); + let rank = Rank::toplevel(); solve( vars_by_symbol, @@ -593,7 +593,7 @@ fn adjust_rank_content( Structure(flat_type) => { match flat_type { Apply { args, .. } => { - let mut rank = Rank::outermost(); + let mut rank = Rank::toplevel(); for var in args { rank = rank.max(adjust_rank(subs, young_mark, visit_mark, group_rank, var)); @@ -614,7 +614,7 @@ fn adjust_rank_content( EmptyRecord => { // from elm-compiler: THEORY: an empty record never needs to get generalized - Rank::outermost() + Rank::toplevel() } Record(fields, ext_var) => { @@ -631,9 +631,9 @@ fn adjust_rank_content( } Alias(_, _, args, _) => { - let mut rank = Rank::outermost(); + let mut rank = Rank::toplevel(); - // from elm-compiler: THEORY: anything in the real_var would be Rank::outermost() + // from elm-compiler: THEORY: anything in the real_var would be Rank::toplevel() for (_, var) in args { rank = rank.max(adjust_rank(subs, young_mark, visit_mark, group_rank, var)); } diff --git a/src/subs.rs b/src/subs.rs index 2e3bc161d0..6f0f99e651 100644 --- a/src/subs.rs +++ b/src/subs.rs @@ -279,7 +279,7 @@ impl Rank { Rank(0) } - pub fn outermost() -> Self { + pub fn toplevel() -> Self { Rank(1) } @@ -300,13 +300,7 @@ impl fmt::Display for Rank { impl fmt::Debug for Rank { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self == &Rank::none() { - write!(f, "none") - } else if self == &Rank::outermost() { - write!(f, "outermost") - } else { - write!(f, "Rank({})", self.0) - } + self.0.fmt(f) } } From 302b03cb20cd24b61e505b7aae83ddfa81b60872 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 22 Dec 2019 10:44:29 -0800 Subject: [PATCH 2/2] Make defs use "let rec" style; drop Variable --- src/can/def.rs | 39 +++++++++++++++++--------------------- src/can/expr.rs | 2 +- src/solve.rs | 2 +- src/types.rs | 6 +++++- src/uniqueness/mod.rs | 2 +- tests/test_canonicalize.rs | 14 +++++++------- 6 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/can/def.rs b/src/can/def.rs index 5005e2585c..4041e73b39 100644 --- a/src/can/def.rs +++ b/src/can/def.rs @@ -918,37 +918,32 @@ pub fn can_defs_with_return<'a>( output.rigids = output.rigids.union(found_rigids); - // Rigid constraint for the def expr as a whole + // Rigid constraint for the def expr as a whole. + // This is a "LetRec" constraint; it supports recursion. + // (The only advantage of "Let" over "LetRec" is if you want to + // shadow things, and Roc disallows shadowing anyway.) let constraint = Let(Box::new(LetConstraint { rigid_vars: rigid_info.vars, flex_vars: Vec::new(), def_types: rigid_info.def_types, - defs_constraint: - // Flex constraint - Let(Box::new(LetConstraint { + defs_constraint: True, + ret_constraint: Let(Box::new(LetConstraint { + rigid_vars: Vec::new(), + flex_vars: flex_info.vars, + def_types: flex_info.def_types.clone(), + defs_constraint: Let(Box::new(LetConstraint { + flex_vars: Vec::new(), rigid_vars: Vec::new(), - flex_vars: flex_info.vars, - def_types: flex_info.def_types.clone(), - defs_constraint: - // Final flex constraints - Let(Box::new(LetConstraint { - rigid_vars: Vec::new(), - flex_vars: Vec::new(), - def_types: flex_info.def_types, - defs_constraint: True, - ret_constraint: And(flex_info.constraints) - })), - ret_constraint: And(vec![And(rigid_info.constraints), ret_con]) + def_types: flex_info.def_types, + defs_constraint: True, + ret_constraint: And(flex_info.constraints), })), - ret_constraint: True, + ret_constraint: And(vec![And(rigid_info.constraints), ret_con]), + })), })); match can_defs { - Ok(defs) => ( - Defs(var_store.fresh(), defs, Box::new(ret_expr)), - output, - constraint, - ), + Ok(defs) => (Defs(defs, Box::new(ret_expr)), output, constraint), Err(err) => (RuntimeError(err), output, constraint), } } diff --git a/src/can/expr.rs b/src/can/expr.rs index c738a98246..bf7755865c 100644 --- a/src/can/expr.rs +++ b/src/can/expr.rs @@ -74,7 +74,7 @@ pub enum Expr { Box>, Vec<(Located, Located)>, ), - Defs(Variable, Vec, Box>), + Defs(Vec, Box>), /// This is *only* for calling functions, not for tag application. /// The Tag variant contains any applied values inside it. diff --git a/src/solve.rs b/src/solve.rs index 3b99fb8260..ceff06b545 100644 --- a/src/solve.rs +++ b/src/solve.rs @@ -565,7 +565,7 @@ fn adjust_rank( } else if mark == visit_mark { desc.rank } else { - let min_rank = desc.rank.min(group_rank); + let min_rank = group_rank.min(desc.rank); // TODO from elm-compiler: how can min_rank ever be group_rank? desc.rank = min_rank; diff --git a/src/types.rs b/src/types.rs index 0cdf933631..30b07b343c 100644 --- a/src/types.rs +++ b/src/types.rs @@ -45,6 +45,8 @@ impl fmt::Debug for Type { match self { Type::EmptyRec => write!(f, "{{}}"), Type::Function(args, ret) => { + write!(f, "Fn(")?; + for (index, arg) in args.iter().enumerate() { if index > 0 { ", ".fmt(f)?; @@ -55,7 +57,9 @@ impl fmt::Debug for Type { write!(f, " -> ")?; - ret.fmt(f) + ret.fmt(f)?; + + write!(f, ")") } Type::Variable(var) => write!(f, "<{:?}>", var), diff --git a/src/uniqueness/mod.rs b/src/uniqueness/mod.rs index c8e7aa4f66..c8773d620e 100644 --- a/src/uniqueness/mod.rs +++ b/src/uniqueness/mod.rs @@ -385,7 +385,7 @@ pub fn canonicalize_expr( ) } - Defs(_, defs, loc_ret) => { + Defs(defs, loc_ret) => { // The body expression gets a new scope for canonicalization, // so clone it. diff --git a/tests/test_canonicalize.rs b/tests/test_canonicalize.rs index 1539251f3e..e19f94704b 100644 --- a/tests/test_canonicalize.rs +++ b/tests/test_canonicalize.rs @@ -232,7 +232,7 @@ mod test_canonicalize { fn get_closure(expr: &Expr, i: usize) -> roc::can::expr::Recursive { match expr { - Defs(_, assignments, _) => match &assignments.get(i).map(|def| &def.expr.value) { + Defs(assignments, _) => match &assignments.get(i).map(|def| &def.expr.value) { Some(Closure(_, recursion, _, _)) => recursion.clone(), Some(other @ _) => { panic!("assignment at {} is not a closure, but a {:?}", i, other) @@ -248,17 +248,17 @@ mod test_canonicalize { with_larger_debug_stack(|| { let src = indoc!( r#" - g = \x -> + g = \x -> case x when 0 -> 0 _ -> g (x - 1) - h = \x -> + h = \x -> case x when 0 -> 0 _ -> g (x - 1) - p = \x -> + p = \x -> case x when 0 -> 0 1 -> g (x - 1) @@ -326,7 +326,7 @@ mod test_canonicalize { // TODO when a case witn no branches parses, remove the pattern wildcard here let src = indoc!( r#" - q = \x -> + q = \x -> case q x when _ -> 0 @@ -347,12 +347,12 @@ mod test_canonicalize { // TODO when a case with no branches parses, remove the pattern wildcard here let src = indoc!( r#" - q = \x -> + q = \x -> case x when 0 -> 0 _ -> p (x - 1) - p = \x -> + p = \x -> case x when 0 -> 0 _ -> q (x - 1)