Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tail recursive hover and bug fix for correctly marking if else as tail recursive #6466

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions crates/compiler/can/src/copy.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
def::Def,
expr::{
ClosureData, Expr, Field, OpaqueWrapFunctionData, StructAccessorData, WhenBranchPattern,
ClosureData, Expr, Field, OpaqueWrapFunctionData, Recursive, StructAccessorData,
WhenBranchPattern,
},
pattern::{DestructType, ListPatterns, Pattern, RecordDestruct, TupleDestruct},
};
Expand Down Expand Up @@ -393,7 +394,7 @@ fn deep_copy_expr_help<C: CopyEnv>(env: &mut C, copied: &mut Vec<Variable>, expr
LetNonRec(Box::new(def), Box::new(body.map(|e| go_help!(e))))
}

Call(f, args, called_via) => {
Call(f, args, called_via, rec) => {
let (fn_var, fn_expr, clos_var, ret_var) = &**f;
Call(
Box::new((
Expand All @@ -406,6 +407,7 @@ fn deep_copy_expr_help<C: CopyEnv>(env: &mut C, copied: &mut Vec<Variable>, expr
.map(|(var, expr)| (sub!(*var), expr.map(|e| go_help!(e))))
.collect(),
*called_via,
*rec,
)
}
Crash { msg, ret_var } => Crash {
Expand Down
3 changes: 2 additions & 1 deletion crates/compiler/can/src/debug/pretty_print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ fn expr<'a>(c: &Ctx, p: EPrec, f: &'a Arena<'a>, e: &'a Expr) -> DocBuilder<'a,
.append(f.hardline())
.append(expr(c, Free, f, &body.value))
.group(),
Call(fun, args, _) => {
//TODO! add recursive to this
Call(fun, args, _, _) => {
let (_, fun, _, _) = &**fun;
maybe_paren!(
Free,
Expand Down
67 changes: 54 additions & 13 deletions crates/compiler/can/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::annotation::IntroducedVariables;
use crate::annotation::OwnedNamedOrAble;
use crate::derive;
use crate::env::Env;
use crate::expr::canonicalize_expr_with_tail;
use crate::expr::get_lookup_symbols;
use crate::expr::AnnotatedMark;
use crate::expr::ClosureData;
Expand Down Expand Up @@ -928,6 +929,7 @@ pub(crate) fn canonicalize_defs<'a>(
scope: &mut Scope,
loc_defs: &'a mut roc_parse::ast::Defs<'a>,
pattern_type: PatternType,
last_is_tail: bool,
) -> (CanDefs, Output, MutMap<Symbol, Region>) {
// Canonicalizing defs while detecting shadowing involves a multi-step process:
//
Expand Down Expand Up @@ -1021,6 +1023,7 @@ pub(crate) fn canonicalize_defs<'a>(
pattern_type,
aliases,
symbols_introduced,
last_is_tail,
)
}

Expand All @@ -1034,6 +1037,7 @@ fn canonicalize_value_defs<'a>(
pattern_type: PatternType,
mut aliases: VecMap<Symbol, Alias>,
mut symbols_introduced: MutMap<Symbol, Region>,
last_is_tail: bool,
) -> (CanDefs, Output, MutMap<Symbol, Region>) {
// Canonicalize all the patterns, record shadowing problems, and store
// the ast::Expr values in pending_exprs for further canonicalization
Expand Down Expand Up @@ -1099,6 +1103,7 @@ fn canonicalize_value_defs<'a>(
let mut def_ordering = DefOrdering::from_symbol_to_id(env.home, symbol_to_index, capacity);

for (def_id, pending_def) in pending_value_defs.into_iter().enumerate() {
let is_tail = last_is_tail && (def_id == capacity - 1);
let temp_output = canonicalize_pending_value_def(
env,
pending_def,
Expand All @@ -1107,6 +1112,7 @@ fn canonicalize_value_defs<'a>(
var_store,
pattern_type,
&mut aliases,
is_tail,
);

output = temp_output.output;
Expand Down Expand Up @@ -1997,7 +2003,7 @@ pub(crate) fn sort_can_defs(

(declarations, output)
}

//TODO:ELI Not sure if this is needed now that we have my new system
fn mark_def_recursive(mut def: Def) -> Def {
if let Closure(ClosureData {
recursive: recursive @ Recursive::NotRecursive,
Expand Down Expand Up @@ -2149,6 +2155,7 @@ fn canonicalize_pending_value_def<'a>(
var_store: &mut VarStore,
pattern_type: PatternType,
aliases: &mut VecMap<Symbol, Alias>,
is_tail: bool,
) -> DefOutput {
use PendingValueDef::*;

Expand Down Expand Up @@ -2288,6 +2295,7 @@ fn canonicalize_pending_value_def<'a>(
loc_can_pattern,
loc_expr,
Some(Loc::at(loc_ann.region, type_annotation)),
is_tail,
)
}
Body(loc_can_pattern, loc_expr) => {
Expand All @@ -2300,6 +2308,7 @@ fn canonicalize_pending_value_def<'a>(
loc_can_pattern,
loc_expr,
None,
is_tail,
)
}
};
Expand Down Expand Up @@ -2332,6 +2341,7 @@ fn canonicalize_pending_body<'a>(
loc_expr: &'a Loc<ast::Expr>,

opt_loc_annotation: Option<Loc<crate::annotation::Annotation>>,
is_tail: bool,
) -> DefOutput {
let mut loc_value = &loc_expr.value;

Expand Down Expand Up @@ -2369,13 +2379,7 @@ fn canonicalize_pending_body<'a>(
// reset the tailcallable_symbol
env.tailcallable_symbol = outer_tailcallable;

// The closure is self tail recursive iff it tail calls itself (by defined name).
let is_recursive = match can_output.tail_call {
Some(tail_symbol) if tail_symbol == *defined_symbol => Recursive::TailRecursive,
_ => Recursive::NotRecursive,
};

closure_data.recursive = is_recursive;
closure_data.recursive = can_output.rec_call;
closure_data.name = *defined_symbol;

let loc_can_expr = Loc::at(loc_expr.region, Expr::Closure(closure_data));
Expand Down Expand Up @@ -2421,8 +2425,14 @@ fn canonicalize_pending_body<'a>(
}

_ => {
let (loc_can_expr, can_output) =
canonicalize_expr(env, var_store, scope, loc_expr.region, &loc_expr.value);
let (loc_can_expr, can_output) = canonicalize_expr_with_tail(
env,
var_store,
scope,
loc_expr.region,
&loc_expr.value,
is_tail,
);

let def_references = DefReferences::Value(can_output.references.clone());
output.union(can_output);
Expand Down Expand Up @@ -2451,6 +2461,25 @@ fn canonicalize_pending_body<'a>(
def,
}
}
///checks if we immediately return the last def, for the purposes of being tail recursiive this means the def may as well not be there
fn check_if_last_def_is_return<'a>(
loc_defs: &'a Defs<'a>,
loc_ret: &'a Loc<ast::Expr<'a>>,
) -> bool {
match loc_defs.last() {
Some(Err(ast::ValueDef::Body(pat, _)))
| Some(Err(ast::ValueDef::AnnotatedBody {
body_pattern: pat, ..
})) => match pat.value {
ast::Pattern::Identifier(symb) => match loc_ret.value {
ast::Expr::Var { ident, .. } => symb == ident,
_ => false,
},
_ => false,
},
_ => false,
}
}

#[inline(always)]
pub fn can_defs_with_return<'a>(
Expand All @@ -2459,25 +2488,37 @@ pub fn can_defs_with_return<'a>(
scope: &mut Scope,
loc_defs: &'a mut Defs<'a>,
loc_ret: &'a Loc<ast::Expr<'a>>,
in_tail: bool,
) -> (Expr, Output) {
let last_is_tail = check_if_last_def_is_return(loc_defs, loc_ret);
let (unsorted, defs_output, symbols_introduced) = canonicalize_defs(
env,
Output::default(),
var_store,
scope,
loc_defs,
PatternType::DefExpr,
last_is_tail,
);

// The def as a whole is a tail call iff its return expression is a tail call.
// The def as a whole is a tail call if its return expression is a tail call.
// Use its output as a starting point because its tail_call already has the right answer!
let (ret_expr, mut output) =
canonicalize_expr(env, var_store, scope, loc_ret.region, &loc_ret.value);

let (ret_expr, mut output) = canonicalize_expr_with_tail(
env,
var_store,
scope,
loc_ret.region,
&loc_ret.value,
in_tail,
);

output
.introduced_variables
.union(&defs_output.introduced_variables);

output.rec_call.union_mut(defs_output.rec_call);

// Sort the defs with the output of the return expression - we'll use this to catch unused defs
// due only to recursion.
let (declarations, mut output) = sort_can_defs(env, var_store, unsorted, output);
Expand Down
70 changes: 60 additions & 10 deletions crates/compiler/can/src/effect_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,12 @@ fn build_effect_map(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(Expr::EmptyRecord))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

// `toEffect (thunk {})`
Expand All @@ -266,7 +271,12 @@ fn build_effect_map(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(force_thunk_call))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

let inner_closure_symbol = {
Expand Down Expand Up @@ -423,7 +433,12 @@ fn force_thunk(expr: Expr, thunk_var: Variable, var_store: &mut VarStore) -> Exp
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(Expr::EmptyRecord))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
}

fn build_effect_after(
Expand Down Expand Up @@ -462,7 +477,12 @@ fn build_effect_after(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(force_effect_call))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

// let @Effect thunk = toEffect (effect {}) in thunk {}
Expand Down Expand Up @@ -725,7 +745,12 @@ fn force_effect(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(Expr::EmptyRecord))];
let call = Expr::Call(Box::new(boxed), arguments, CalledVia::Space);
let call = Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
);

Loc::at_zero(call)
};
Expand Down Expand Up @@ -943,7 +968,12 @@ fn build_effect_forever_inner_body(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(Expr::EmptyRecord))];
let call = Expr::Call(Box::new(boxed), arguments, CalledVia::Space);
let call = Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
);

Loc::at_zero(call)
};
Expand All @@ -968,7 +998,12 @@ fn build_effect_forever_inner_body(

let effect_var = var_store.fresh();
let arguments = vec![(effect_var, Loc::at_zero(Expr::Var(effect, effect_var)))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

// ```
Expand Down Expand Up @@ -1226,7 +1261,12 @@ fn build_effect_loop_inner_body(

let state_var = var_store.fresh();
let arguments = vec![(state_var, Loc::at_zero(Expr::Var(state_symbol, state_var)))];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

Def {
Expand All @@ -1250,7 +1290,12 @@ fn build_effect_loop_inner_body(
);

let arguments = vec![(var_store.fresh(), Loc::at_zero(Expr::EmptyRecord))];
let call = Expr::Call(Box::new(boxed), arguments, CalledVia::Space);
let call = Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
);

Loc::at_zero(call)
};
Expand All @@ -1274,7 +1319,12 @@ fn build_effect_loop_inner_body(
),
(step_var, Loc::at_zero(Expr::Var(step_symbol, step_var))),
];
Expr::Call(Box::new(boxed), arguments, CalledVia::Space)
Expr::Call(
Box::new(boxed),
arguments,
CalledVia::Space,
Recursive::NotRecursive,
)
};

// ```
Expand Down
Loading