language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-10-02 08:11:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

Use DeclareRec for all recursive functions

Browse source 
Even ones that are only self-recursive. e.g. LinkedList.map.
This commit is contained in:
Folkert 2020-01-16 21:48:19 +01:00
parent f240d38947
commit 7f5998b9e6
2 changed files with 89 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

63
src/can/def.rs
View file

@ -229,7 +229,7 @@ pub fn sort_can_defs(
// recursive definitions.
// All successors that occur in the body of a symbol.
let mut all_successors = |symbol: &Symbol| -> ImSet<Symbol> {
let all_successors_without_self = |symbol: &Symbol| -> ImSet<Symbol> {
// This may not be in refs_by_symbol. For example, the `f` in `f x` here:
//
// f = \z -> z
@ -272,6 +272,49 @@ pub fn sort_can_defs(
}
};
// All successors that occur in the body of a symbol, including the symbol itself
// This is required to determine whether a symbol is recursive. Recursive symbols
// (that are not faulty) always need a DeclareRec, even if there is just one symbol in the
// group
let mut all_successors_with_self = |symbol: &Symbol| -> ImSet<Symbol> {
// This may not be in refs_by_symbol. For example, the `f` in `f x` here:
//
// f = \z -> z
//
// (\x ->
// a = f x
// x
// )
//
// It's not part of the current defs (the one with `a = f x`); rather,
// it's in the enclosing scope. It's still referenced though, so successors
// will receive it as an argument!
match refs_by_symbol.get(symbol) {
Some((_, references)) => {
// We can only sort the symbols at the current level. That is safe because
// symbols defined at higher levels cannot refer to symbols at lower levels.
// Therefore they can never form a cycle!
//
// In the above example, `f` cannot reference `a`, and in the closure
// a call to `f` cannot cycle back to `a`.
let mut loc_succ = local_successors(&references, &env.closures);
// if the current symbol is a closure, peek into its body
if let Some(References { locals, .. }) = env.closures.get(symbol) {
for s in locals.clone() {
loc_succ.insert(s);
}
}
// remove anything that is not defined in the current block
loc_succ.retain(|key| defined_symbols_set.contains(key));
loc_succ
}
None => ImSet::default(),
}
};
// If a symbol is a direct successor of itself, there is an invalid cycle.
// The difference with the function above is that this one does not look behind lambdas,
// but does consider direct self-recursion.
@ -295,15 +338,16 @@ pub fn sort_can_defs(
// TODO also do the same `addDirects` check elm/compiler does, so we can
// report an error if a recursive definition can't possibly terminate!
match topological_sort_into_groups(defined_symbols.as_slice(), all_successors) {
match topological_sort_into_groups(defined_symbols.as_slice(), all_successors_without_self) {
Ok(groups) => {
let mut declarations = Vec::new();
// groups are in reversed order
for group in groups.into_iter().rev() {
group_to_declaration(
group,
&env.closures,
&mut all_successors,
&mut all_successors_with_self,
&can_defs_by_symbol,
&mut declarations,
);
@ -320,7 +364,7 @@ pub fn sort_can_defs(
group_to_declaration(
group,
&env.closures,
&mut all_successors,
&mut all_successors_with_self,
&can_defs_by_symbol,
&mut declarations,
);
@ -337,7 +381,8 @@ pub fn sort_can_defs(
//
// foo = if b then foo else bar
for cycle in strongly_connected_components(&nodes_in_cycle, all_successors) {
for cycle in strongly_connected_components(&nodes_in_cycle, all_successors_without_self)
{
// check whether the cycle is faulty, which is when has a direct successor in the
// current cycle. This catches things like:
//
@ -386,7 +431,7 @@ pub fn sort_can_defs(
group_to_declaration(
cycle,
&env.closures,
&mut all_successors,
&mut all_successors_with_self,
&can_defs_by_symbol,
&mut declarations,
);
@ -442,9 +487,15 @@ fn group_to_declaration(
new_def.loc_expr.value = Closure(fn_var, name, recursion, args, body);
}
let is_recursive = successors(&symbol).contains(&symbol);
if !seen_pattern_regions.contains(&new_def.loc_pattern.region) {
if is_recursive {
declarations.push(DeclareRec(vec![new_def.clone()]));
} else {
declarations.push(Declare(new_def.clone()));
}
}
seen_pattern_regions.insert(new_def.loc_pattern.region);
}
} else {

31
tests/test_canonicalize.rs
View file

@ -262,13 +262,21 @@ mod test_canonicalize {
fn get_closure(expr: &Expr, i: usize) -> roc::can::expr::Recursive {
match expr {
LetRec(assignments, _, _) => match &assignments.get(i).map(|def| &def.loc_expr.value) {
LetRec(assignments, body, _) => {
match &assignments.get(i).map(|def| &def.loc_expr.value) {
Some(Closure(_, _, recursion, _, _)) => recursion.clone(),
Some(other @ _) => {
panic!("assignment at {} is not a closure, but a {:?}", i, other)
}
None => panic!("Looking for assignment at {} but the list is too short", i),
},
None => {
if i > 0 {
get_closure(&body.value, i - 1)
} else {
panic!("Looking for assignment at {} but the list is too short", i)
}
}
}
}
LetNonRec(def, body, _) => {
if i > 0 {
// recurse in the body (not the def!)
@ -308,22 +316,23 @@ mod test_canonicalize {
1 -> g (x - 1)
_ -> p (x - 1)
# variables must be (indirectly) referenced in the body for analysis to work
{ x: p, y: h }
# { x: p, y: h }
g
"#
);
let arena = Bump::new();
let (actual, _output, _problems, _var_store, _vars, _constraint) =
can_expr_with(&arena, "Blah", src, &ImMap::default());
let detected = get_closure(&actual.value, 0);
assert_eq!(detected, Recursive::TailRecursive);
let detected0 = get_closure(&actual.value, 0);
let detected1 = get_closure(&actual.value, 1);
let detected2 = get_closure(&actual.value, 2);
let detected = get_closure(&actual.value, 1);
assert_eq!(detected, Recursive::NotRecursive);
let detected = get_closure(&actual.value, 2);
assert_eq!(detected, Recursive::TailRecursive);
assert_eq!(detected0, Recursive::TailRecursive);
assert_eq!(detected1, Recursive::NotRecursive);
assert_eq!(detected2, Recursive::TailRecursive);
});
}