diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 42df5ee6fbb..a85f4d5d8c1 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -238,7 +238,7 @@ fn main() -> io::Result<()> { current_block = String::new(); } else if in_roc_block { current_block.push_str(&line); - current_block.push_str("\n"); + current_block.push('\n'); } } diff --git a/crates/compiler/can/src/builtins.rs b/crates/compiler/can/src/builtins.rs index 60f01975841..64e7d9d6697 100644 --- a/crates/compiler/can/src/builtins.rs +++ b/crates/compiler/can/src/builtins.rs @@ -446,6 +446,7 @@ fn defn_help( function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: ret_var, + early_returns: vec![], name: fn_name, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index 0dc2f10f7a3..d15f88280e8 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -97,6 +97,7 @@ impl Constraints { Category::List, Category::Str, Category::Character, + Category::Return, ]); pattern_categories.extend([ @@ -150,6 +151,7 @@ impl Constraints { pub const CATEGORY_LIST: Index = Index::new(11); pub const CATEGORY_STR: Index = Index::new(12); pub const CATEGORY_CHARACTER: Index = Index::new(13); + pub const CATEGORY_RETURN: Index = Index::new(14); pub const PCATEGORY_RECORD: Index = Index::new(0); pub const PCATEGORY_EMPTYRECORD: Index = Index::new(1); diff --git a/crates/compiler/can/src/copy.rs b/crates/compiler/can/src/copy.rs index e0784a34fd9..0f791e82733 100644 --- a/crates/compiler/can/src/copy.rs +++ b/crates/compiler/can/src/copy.rs @@ -456,6 +456,7 @@ fn deep_copy_expr_help(env: &mut C, copied: &mut Vec, expr function_type, closure_type, return_type, + early_returns, name, captured_symbols, recursive, @@ -465,6 +466,10 @@ fn deep_copy_expr_help(env: &mut C, copied: &mut Vec, expr function_type: sub!(*function_type), closure_type: sub!(*closure_type), return_type: sub!(*return_type), + early_returns: early_returns + .iter() + .map(|(var, region)| (sub!(*var), *region)) + .collect(), name: *name, captured_symbols: captured_symbols .iter() @@ -688,6 +693,14 @@ fn deep_copy_expr_help(env: &mut C, copied: &mut Vec, expr lookups_in_cond: lookups_in_cond.to_vec(), }, + Return { + return_value, + return_var, + } => Return { + return_value: Box::new(return_value.map(|e| go_help!(e))), + return_var: sub!(*return_var), + }, + Dbg { source_location, source, diff --git a/crates/compiler/can/src/debug/pretty_print.rs b/crates/compiler/can/src/debug/pretty_print.rs index b842d6076ef..1ab6dec4774 100644 --- a/crates/compiler/can/src/debug/pretty_print.rs +++ b/crates/compiler/can/src/debug/pretty_print.rs @@ -449,6 +449,7 @@ fn expr<'a>(c: &Ctx, p: EPrec, f: &'a Arena<'a>, e: &'a Expr) -> DocBuilder<'a, Dbg { .. } => todo!(), Expect { .. } => todo!(), ExpectFx { .. } => todo!(), + Return { .. } => todo!(), TypedHole(_) => todo!(), RuntimeError(_) => todo!(), } diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index c95b62bf8dc..31e9c1330f8 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -1681,7 +1681,7 @@ impl DefOrdering { } #[inline(always)] -pub(crate) fn sort_can_defs_new( +pub(crate) fn sort_top_level_can_defs( env: &mut Env<'_>, scope: &mut Scope, var_store: &mut VarStore, @@ -2354,6 +2354,7 @@ fn canonicalize_pending_value_def<'a>( function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: var_store.fresh(), + early_returns: scope.early_returns.clone(), name: symbol, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, @@ -2571,6 +2572,8 @@ fn canonicalize_pending_body<'a>( loc_value = value; } + let expr_var = var_store.fresh(); + // We treat closure definitions `foo = \a, b -> ...` differently from other body expressions, // because they need more bookkeeping (for tail calls, closure captures, etc.) // @@ -2602,9 +2605,15 @@ fn canonicalize_pending_body<'a>( 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, + let is_recursive = if !can_output.tail_calls.is_empty() + && can_output + .tail_calls + .iter() + .all(|tail_symbol| tail_symbol == defined_symbol) + { + Recursive::TailRecursive + } else { + Recursive::NotRecursive }; closure_data.recursive = is_recursive; @@ -2664,7 +2673,6 @@ fn canonicalize_pending_body<'a>( } }; - let expr_var = var_store.fresh(); let mut vars_by_symbol = SendMap::default(); pattern_to_vars_by_symbol(&mut vars_by_symbol, &loc_can_pattern.value, expr_var); diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index 4d06abf270e..c462af089b6 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -315,7 +315,7 @@ pub fn desugar_value_def_suffixed<'a>(arena: &'a Bump, value_def: ValueDef<'a>) } }, - // TODO support desugaring of Dbg, Expect, and ExpectFx + // TODO support desugaring of Dbg and ExpectFx Dbg { .. } | ExpectFx { .. } => value_def, ModuleImport { .. } | IngestedFileImport(_) => value_def, @@ -1008,6 +1008,7 @@ pub fn desugar_expr<'a>( Expect(condition, continuation) => { let desugared_condition = &*env.arena.alloc(desugar_expr(env, scope, condition)); let desugared_continuation = &*env.arena.alloc(desugar_expr(env, scope, continuation)); + env.arena.alloc(Loc { value: Expect(desugared_condition, desugared_continuation), region: loc_expr.region, @@ -1019,7 +1020,6 @@ pub fn desugar_expr<'a>( } DbgStmt(condition, continuation) => { let desugared_condition = &*env.arena.alloc(desugar_expr(env, scope, condition)); - let desugared_continuation = &*env.arena.alloc(desugar_expr(env, scope, continuation)); env.arena.alloc(Loc { @@ -1027,6 +1027,15 @@ pub fn desugar_expr<'a>( region: loc_expr.region, }) } + Return(return_value, after_return) => { + let desugared_return_value = &*env.arena.alloc(desugar_expr(env, scope, return_value)); + + env.arena.alloc(Loc { + // Do not desugar after_return since it isn't run anyway + value: Return(desugared_return_value, *after_return), + region: loc_expr.region, + }) + } // note this only exists after desugaring LowLevelDbg(_, _, _) => loc_expr, diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 9dcf964e46b..50cd40253a1 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -39,7 +39,7 @@ pub type PendingDerives = VecMap>)>; #[derive(Clone, Default, Debug)] pub struct Output { pub references: References, - pub tail_call: Option, + pub tail_calls: Vec, pub introduced_variables: IntroducedVariables, pub aliases: VecMap, pub non_closures: VecSet, @@ -50,8 +50,8 @@ impl Output { pub fn union(&mut self, other: Self) { self.references.union_mut(&other.references); - if let (None, Some(later)) = (self.tail_call, other.tail_call) { - self.tail_call = Some(later); + if self.tail_calls.is_empty() && !other.tail_calls.is_empty() { + self.tail_calls = other.tail_calls; } self.introduced_variables @@ -287,6 +287,11 @@ pub enum Expr { symbol: Symbol, }, + Return { + return_value: Box>, + return_var: Variable, + }, + /// Rendered as empty box in editor TypedHole(Variable), @@ -361,6 +366,7 @@ impl Expr { Self::Expect { .. } => Category::Expect, Self::ExpectFx { .. } => Category::Expect, Self::Crash { .. } => Category::Crash, + Self::Return { .. } => Category::Return, Self::Dbg { .. } => Category::Expect, @@ -401,6 +407,7 @@ pub struct ClosureData { pub function_type: Variable, pub closure_type: Variable, pub return_type: Variable, + pub early_returns: Vec<(Variable, Region)>, pub name: Symbol, pub captured_symbols: Vec<(Symbol, Variable)>, pub recursive: Recursive, @@ -477,6 +484,7 @@ impl StructAccessorData { function_type: function_var, closure_type: closure_var, return_type: field_var, + early_returns: vec![], name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -550,6 +558,7 @@ impl OpaqueWrapFunctionData { function_type: function_var, closure_type: closure_var, return_type: opaque_var, + early_returns: vec![], name: function_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -715,7 +724,6 @@ pub fn canonicalize_expr<'a>( let output = Output { references, - tail_call: None, ..Default::default() }; @@ -783,7 +791,6 @@ pub fn canonicalize_expr<'a>( let output = Output { references, - tail_call: None, ..Default::default() }; @@ -900,17 +907,19 @@ pub fn canonicalize_expr<'a>( output.union(fn_expr_output); // Default: We're not tail-calling a symbol (by name), we're tail-calling a function value. - output.tail_call = None; + output.tail_calls = vec![]; let expr = match fn_expr.value { Var(symbol, _) => { output.references.insert_call(symbol); // we're tail-calling a symbol by name, check if it's the tail-callable symbol - output.tail_call = match &env.tailcallable_symbol { - Some(tc_sym) if *tc_sym == symbol => Some(symbol), - Some(_) | None => None, - }; + if env + .tailcallable_symbol + .is_some_and(|tc_sym| tc_sym == symbol) + { + output.tail_calls.push(symbol); + } Call( Box::new(( @@ -1009,7 +1018,7 @@ pub fn canonicalize_expr<'a>( } ast::Expr::Defs(loc_defs, loc_ret) => { // The body expression gets a new scope for canonicalization, - scope.inner_scope(|inner_scope| { + scope.inner_def_scope(|inner_scope| { let defs: Defs = (*loc_defs).clone(); can_defs_with_return(env, var_store, inner_scope, env.arena.alloc(defs), loc_ret) }) @@ -1036,12 +1045,12 @@ pub fn canonicalize_expr<'a>( canonicalize_expr(env, var_store, scope, loc_cond.region, &loc_cond.value); // the condition can never be a tail-call - output.tail_call = None; + output.tail_calls = vec![]; let mut can_branches = Vec::with_capacity(branches.len()); for branch in branches.iter() { - let (can_when_branch, branch_references) = scope.inner_scope(|inner_scope| { + let (can_when_branch, branch_references) = scope.inner_def_scope(|inner_scope| { canonicalize_when_branch( env, var_store, @@ -1061,7 +1070,7 @@ pub fn canonicalize_expr<'a>( // if code gen mistakenly thinks this is a tail call just because its condition // happened to be one. (The condition gave us our initial output value.) if branches.is_empty() { - output.tail_call = None; + output.tail_calls = vec![]; } // Incorporate all three expressions into a combined Output value. @@ -1259,6 +1268,40 @@ pub fn canonicalize_expr<'a>( output, ) } + ast::Expr::Return(return_expr, after_return) => { + let mut output = Output::default(); + + if let Some(after_return) = after_return { + let region_with_return = + Region::span_across(&return_expr.region, &after_return.region); + + env.problem(Problem::StatementsAfterReturn { + region: region_with_return, + }); + } + + let (loc_return_expr, output1) = canonicalize_expr( + env, + var_store, + scope, + return_expr.region, + &return_expr.value, + ); + + output.union(output1); + + let return_var = var_store.fresh(); + + scope.early_returns.push((return_var, return_expr.region)); + + ( + Return { + return_value: Box::new(loc_return_expr), + return_var, + }, + output, + ) + } ast::Expr::If { if_thens, final_else: final_else_branch, @@ -1494,7 +1537,7 @@ pub fn canonicalize_closure<'a>( loc_body_expr: &'a Loc>, opt_def_name: Option, ) -> (ClosureData, Output) { - scope.inner_scope(|inner_scope| { + scope.inner_function_scope(|inner_scope| { canonicalize_closure_body( env, var_store, @@ -1609,6 +1652,17 @@ fn canonicalize_closure_body<'a>( } } + let mut final_expr = &loc_body_expr; + while let Expr::LetRec(_, inner, _) | Expr::LetNonRec(_, inner) = &final_expr.value { + final_expr = inner; + } + + if let Expr::Return { return_value, .. } = &final_expr.value { + env.problem(Problem::ReturnAtEndOfFunction { + region: return_value.region, + }); + } + // store the references of this function in the Env. This information is used // when we canonicalize a surrounding def (if it exists) env.closures.insert(symbol, output.references.clone()); @@ -1622,10 +1676,13 @@ fn canonicalize_closure_body<'a>( output.non_closures.insert(symbol); } + let return_type_var = var_store.fresh(); + let closure_data = ClosureData { function_type: var_store.fresh(), closure_type: var_store.fresh(), - return_type: var_store.fresh(), + return_type: return_type_var, + early_returns: scope.early_returns.clone(), name: symbol, captured_symbols, recursive: Recursive::NotRecursive, @@ -2028,7 +2085,8 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr { | other @ TypedHole { .. } | other @ ForeignCall { .. } | other @ OpaqueWrapFunction(_) - | other @ Crash { .. } => other, + | other @ Crash { .. } + | other @ Return { .. } => other, List { elem_var, @@ -2254,6 +2312,7 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr { function_type, closure_type, return_type, + early_returns, recursive, name, captured_symbols, @@ -2270,6 +2329,7 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr { function_type, closure_type, return_type, + early_returns, recursive, name, captured_symbols, @@ -2496,6 +2556,7 @@ pub fn is_valid_interpolation(expr: &ast::Expr<'_>) -> bool { ast::Expr::DbgStmt(_, _) | ast::Expr::LowLevelDbg(_, _, _) | ast::Expr::Expect(_, _) + | ast::Expr::Return(_, _) | ast::Expr::When(_, _) | ast::Expr::Backpassing(_, _, _) | ast::Expr::SpaceBefore(_, _) @@ -2796,6 +2857,7 @@ impl Declarations { pub fn new() -> Self { Self::with_capacity(0) } + pub fn with_capacity(capacity: usize) -> Self { Self { declarations: Vec::with_capacity(capacity), @@ -2842,6 +2904,7 @@ impl Declarations { let function_def = FunctionDef { closure_type: loc_closure_data.value.closure_type, return_type: loc_closure_data.value.return_type, + early_returns: loc_closure_data.value.early_returns, captured_symbols: loc_closure_data.value.captured_symbols, arguments: loc_closure_data.value.arguments, }; @@ -2893,6 +2956,7 @@ impl Declarations { let function_def = FunctionDef { closure_type: loc_closure_data.value.closure_type, return_type: loc_closure_data.value.return_type, + early_returns: loc_closure_data.value.early_returns, captured_symbols: loc_closure_data.value.captured_symbols, arguments: loc_closure_data.value.arguments, }; @@ -3073,6 +3137,7 @@ impl Declarations { let function_def = FunctionDef { closure_type: closure_data.closure_type, return_type: closure_data.return_type, + early_returns: closure_data.early_returns, captured_symbols: closure_data.captured_symbols, arguments: closure_data.arguments, }; @@ -3113,6 +3178,7 @@ impl Declarations { function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: var_store.fresh(), + early_returns: vec![], name: self.symbols[index].value, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -3125,6 +3191,7 @@ impl Declarations { let function_def = FunctionDef { closure_type: loc_closure_data.value.closure_type, return_type: loc_closure_data.value.return_type, + early_returns: loc_closure_data.value.early_returns, captured_symbols: loc_closure_data.value.captured_symbols, arguments: loc_closure_data.value.arguments, }; @@ -3259,6 +3326,7 @@ impl DeclarationTag { pub struct FunctionDef { pub closure_type: Variable, pub return_type: Variable, + pub early_returns: Vec<(Variable, Region)>, pub captured_symbols: Vec<(Symbol, Variable)>, pub arguments: Vec<(Variable, AnnotatedMark, Loc)>, } @@ -3403,6 +3471,9 @@ pub(crate) fn get_lookup_symbols(expr: &Expr) -> Vec { // Intentionally ignore the lookups in the nested `expect` condition itself, // because they couldn't possibly influence the outcome of this `expect`! } + Expr::Return { return_value, .. } => { + stack.push(&return_value.value); + } Expr::Crash { msg, .. } => stack.push(&msg.value), Expr::Num(_, _, _, _) | Expr::Float(_, _, _, _, _) diff --git a/crates/compiler/can/src/module.rs b/crates/compiler/can/src/module.rs index 3a45b6ba912..38276a9fd3a 100644 --- a/crates/compiler/can/src/module.rs +++ b/crates/compiler/can/src/module.rs @@ -369,6 +369,12 @@ pub fn canonicalize_module_defs<'a>( PatternType::TopLevelDef, ); + for (_early_return_var, early_return_region) in &scope.early_returns { + env.problem(Problem::ReturnOutsideOfFunction { + region: *early_return_region, + }); + } + let pending_derives = output.pending_derives; // See if any of the new idents we defined went unused. @@ -425,7 +431,7 @@ pub fn canonicalize_module_defs<'a>( ..Default::default() }; - let (mut declarations, mut output) = crate::def::sort_can_defs_new( + let (mut declarations, mut output) = crate::def::sort_top_level_can_defs( &mut env, &mut scope, var_store, @@ -969,6 +975,14 @@ fn fix_values_captured_in_closure_expr( ); } + Return { return_value, .. } => { + fix_values_captured_in_closure_expr( + &mut return_value.value, + no_capture_symbols, + closure_captures, + ); + } + Crash { msg, ret_var: _ } => { fix_values_captured_in_closure_expr( &mut msg.value, diff --git a/crates/compiler/can/src/scope.rs b/crates/compiler/can/src/scope.rs index 33127f7dd76..fe6608f4ffe 100644 --- a/crates/compiler/can/src/scope.rs +++ b/crates/compiler/can/src/scope.rs @@ -48,6 +48,8 @@ pub struct Scope { /// Ignored variables (variables that start with an underscore). /// We won't intern them because they're only used during canonicalization for error reporting. ignored_locals: VecMap, + + pub early_returns: Vec<(Variable, Region)>, } impl Scope { @@ -73,6 +75,7 @@ impl Scope { modules: ScopeModules::new(home, module_name), imported_symbols: default_imports, ignored_locals: VecMap::default(), + early_returns: Vec::default(), } } @@ -429,7 +432,8 @@ impl Scope { self.aliases.contains_key(&name) } - pub fn inner_scope(&mut self, f: F) -> T + /// Enter an inner scope within a definition, e.g. a def or when block. + pub fn inner_def_scope(&mut self, f: F) -> T where F: FnOnce(&mut Scope) -> T, { @@ -462,6 +466,20 @@ impl Scope { result } + /// Enter an inner scope within a child function, e.g. a closure body. + pub fn inner_function_scope(&mut self, f: F) -> T + where + F: FnOnce(&mut Scope) -> T, + { + let early_returns_snapshot = std::mem::take(&mut self.early_returns); + + let result = self.inner_def_scope(f); + + self.early_returns = early_returns_snapshot; + + result + } + pub fn register_debug_idents(&self) { self.home.register_debug_idents(&self.locals.ident_ids) } @@ -868,7 +886,7 @@ mod test { } #[test] - fn inner_scope_does_not_influence_outer() { + fn inner_def_scope_does_not_influence_outer() { let _register_module_debug_names = ModuleIds::default(); let mut scope = Scope::new( ModuleId::ATTR, @@ -882,7 +900,7 @@ mod test { assert!(scope.lookup(&ident, region).is_err()); - scope.inner_scope(|inner| { + scope.inner_def_scope(|inner| { assert!(inner.introduce(ident.clone(), region).is_ok()); }); @@ -908,7 +926,7 @@ mod test { } #[test] - fn idents_with_inner_scope() { + fn idents_with_inner_def_scope() { let _register_module_debug_names = ModuleIds::default(); let mut scope = Scope::new( ModuleId::ATTR, @@ -943,7 +961,7 @@ mod test { &[ident1.clone(), ident2.clone(), ident3.clone(),] ); - scope.inner_scope(|inner| { + scope.inner_def_scope(|inner| { let ident4 = Ident::from("Ångström"); let ident5 = Ident::from("Sirály"); diff --git a/crates/compiler/can/src/task_module.rs b/crates/compiler/can/src/task_module.rs index c494b78f9eb..f762527a478 100644 --- a/crates/compiler/can/src/task_module.rs +++ b/crates/compiler/can/src/task_module.rs @@ -72,6 +72,7 @@ pub fn build_host_exposed_def( function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: var_store.fresh(), + early_returns: vec![], name: task_closure_symbol, captured_symbols, recursive: Recursive::NotRecursive, @@ -98,6 +99,7 @@ pub fn build_host_exposed_def( function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: var_store.fresh(), + early_returns: vec![], name: symbol, captured_symbols: std::vec::Vec::new(), recursive: Recursive::NotRecursive, @@ -126,6 +128,7 @@ pub fn build_host_exposed_def( function_type: var_store.fresh(), closure_type: var_store.fresh(), return_type: var_store.fresh(), + early_returns: vec![], name: task_closure_symbol, captured_symbols, recursive: Recursive::NotRecursive, diff --git a/crates/compiler/can/src/traverse.rs b/crates/compiler/can/src/traverse.rs index 9293f457abf..744e1963b7e 100644 --- a/crates/compiler/can/src/traverse.rs +++ b/crates/compiler/can/src/traverse.rs @@ -22,6 +22,10 @@ pub enum DeclarationInfo<'a> { pattern: Pattern, annotation: Option<&'a Annotation>, }, + Return { + loc_expr: &'a Loc, + expr_var: Variable, + }, Expectation { loc_condition: &'a Loc, }, @@ -50,6 +54,7 @@ impl<'a> DeclarationInfo<'a> { loc_expr, .. } => Region::span_across(&loc_symbol.region, &loc_expr.region), + Return { loc_expr, .. } => loc_expr.region, Expectation { loc_condition } => loc_condition.region, Function { loc_symbol, @@ -67,6 +72,7 @@ impl<'a> DeclarationInfo<'a> { fn var(&self) -> Variable { match self { DeclarationInfo::Value { expr_var, .. } => *expr_var, + DeclarationInfo::Return { expr_var, .. } => *expr_var, DeclarationInfo::Expectation { .. } => Variable::BOOL, DeclarationInfo::Function { expr_var, .. } => *expr_var, DeclarationInfo::Destructure { expr_var, .. } => *expr_var, @@ -185,6 +191,9 @@ pub fn walk_decl(visitor: &mut V, decl: DeclarationInfo<'_>) { Expectation { loc_condition } => { visitor.visit_expr(&loc_condition.value, loc_condition.region, Variable::BOOL); } + Return { loc_expr, expr_var } => { + visitor.visit_expr(&loc_expr.value, loc_expr.region, expr_var); + } Function { loc_symbol, loc_body, @@ -403,6 +412,12 @@ pub fn walk_expr(visitor: &mut V, expr: &Expr, var: Variable) { Variable::NULL, ); } + Expr::Return { + return_value, + return_var, + } => { + visitor.visit_expr(&return_value.value, return_value.region, *return_var); + } Expr::TypedHole(_) => { /* terminal */ } Expr::RuntimeError(..) => { /* terminal */ } } diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index 73f93ca6b01..143b697002a 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -113,6 +113,7 @@ fn constrain_untyped_closure( fn_var: Variable, closure_var: Variable, ret_var: Variable, + early_returns: &[(Variable, Region)], arguments: &[(Variable, AnnotatedMark, Loc)], loc_body_expr: &Loc, captured_symbols: &[(Symbol, Variable)], @@ -134,7 +135,12 @@ fn constrain_untyped_closure( vars.push(closure_var); vars.push(fn_var); - let body_type = constraints.push_expected_type(NoExpectation(return_type_index)); + let body_type = constraints.push_expected_type(ForReason( + Reason::FunctionOutput, + return_type_index, + loc_body_expr.region, + )); + let ret_constraint = constrain_expr( types, constraints, @@ -144,6 +150,21 @@ fn constrain_untyped_closure( body_type, ); + let mut early_return_constraints = Vec::with_capacity(early_returns.len()); + for (early_return_variable, early_return_region) in early_returns { + let early_return_var = constraints.push_variable(*early_return_variable); + let early_return_con = constraints.equal_types( + early_return_var, + body_type, + Category::Return, + *early_return_region, + ); + + early_return_constraints.push(early_return_con); + } + + let early_returns_constraint = constraints.and_constraint(early_return_constraints); + // make sure the captured symbols are sorted! debug_assert_eq!(captured_symbols.to_vec(), { let mut copy = captured_symbols.to_vec(); @@ -185,6 +206,7 @@ fn constrain_untyped_closure( region, fn_var, ), + early_returns_constraint, closure_constraint, ]; @@ -624,6 +646,7 @@ pub fn constrain_expr( function_type: fn_var, closure_type: closure_var, return_type: ret_var, + early_returns, arguments, loc_body: boxed, captured_symbols, @@ -640,6 +663,7 @@ pub fn constrain_expr( *fn_var, *closure_var, *ret_var, + early_returns, arguments, boxed, captured_symbols, @@ -1378,6 +1402,29 @@ pub fn constrain_expr( body_con } + Return { + return_value, + return_var, + } => { + let return_type_index = constraints.push_variable(*return_var); + + let expected_return_value = constraints.push_expected_type(ForReason( + Reason::FunctionOutput, + return_type_index, + return_value.region, + )); + + let return_con = constrain_expr( + types, + constraints, + env, + return_value.region, + &return_value.value, + expected_return_value, + ); + + constraints.exists([*return_var], return_con) + } Tag { tag_union_var: variant_var, ext_var, @@ -1870,6 +1917,7 @@ fn constrain_function_def( expr_var, function_def.closure_type, function_def.return_type, + &function_def.early_returns, &function_def.arguments, loc_body_expr, &function_def.captured_symbols, @@ -2071,6 +2119,7 @@ fn constrain_function_def( expr_var, function_def.closure_type, function_def.return_type, + &function_def.early_returns, &function_def.arguments, loc_expr, &function_def.captured_symbols, @@ -3651,6 +3700,7 @@ fn constraint_recursive_function( expr_var, function_def.closure_type, function_def.return_type, + &function_def.early_returns, &function_def.arguments, loc_expr, &function_def.captured_symbols, @@ -4133,6 +4183,7 @@ fn is_generalizable_expr(mut expr: &Expr) -> bool { | Expect { .. } | ExpectFx { .. } | Dbg { .. } + | Return { .. } | TypedHole(_) | RuntimeError(..) | ZeroArgumentTag { .. } diff --git a/crates/compiler/derive/src/decoding.rs b/crates/compiler/derive/src/decoding.rs index 500e129c1f6..16d0f8b638f 100644 --- a/crates/compiler/derive/src/decoding.rs +++ b/crates/compiler/derive/src/decoding.rs @@ -147,6 +147,7 @@ fn wrap_in_decode_custom_decode_with( function_type: fn_var, closure_type: fn_clos_var, return_type: decode_with_result_var, + early_returns: vec![], name: fn_name, captured_symbols: sorted_inner_decoder_captures, recursive: Recursive::NotRecursive, diff --git a/crates/compiler/derive/src/decoding/record.rs b/crates/compiler/derive/src/decoding/record.rs index ddca4e80ed5..12540abac3e 100644 --- a/crates/compiler/derive/src/decoding/record.rs +++ b/crates/compiler/derive/src/decoding/record.rs @@ -350,6 +350,7 @@ pub(super) fn step_field( function_type, closure_type, return_type: keep_or_skip_var, + early_returns: vec![], name: step_field_closure, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, @@ -586,6 +587,7 @@ fn custom_decoder_lambda(env: &mut Env<'_>, args: DecodingFieldArgs) -> (Variabl function_type: this_custom_callback_var, closure_type: custom_callback_lambda_set_var, return_type: custom_callback_ret_var, + early_returns: vec![], name: custom_closure_symbol, captured_symbols: vec![(state_arg_symbol, state_record_var)], recursive: Recursive::NotRecursive, @@ -993,6 +995,7 @@ pub(super) fn finalizer( function_type: function_var, closure_type, return_type: return_type_var, + early_returns: vec![], name: function_symbol, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, diff --git a/crates/compiler/derive/src/decoding/tuple.rs b/crates/compiler/derive/src/decoding/tuple.rs index 3bfe4be72bc..e9a2ed2a780 100644 --- a/crates/compiler/derive/src/decoding/tuple.rs +++ b/crates/compiler/derive/src/decoding/tuple.rs @@ -556,6 +556,7 @@ fn step_elem( function_type: this_custom_callback_var, closure_type: custom_callback_lambda_set_var, return_type: custom_callback_ret_var, + early_returns: vec![], name: custom_closure_symbol, captured_symbols: vec![(state_arg_symbol, state_record_var)], recursive: Recursive::NotRecursive, @@ -710,6 +711,7 @@ fn step_elem( function_type, closure_type, return_type: keep_or_skip_var, + early_returns: vec![], name: step_elem_closure, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, @@ -896,6 +898,7 @@ fn finalizer( function_type: function_var, closure_type, return_type: return_type_var, + early_returns: vec![], name: function_symbol, captured_symbols: Vec::new(), recursive: Recursive::NotRecursive, diff --git a/crates/compiler/derive/src/encoding.rs b/crates/compiler/derive/src/encoding.rs index 4764c4f1bd1..317539d482d 100644 --- a/crates/compiler/derive/src/encoding.rs +++ b/crates/compiler/derive/src/encoding.rs @@ -188,6 +188,7 @@ fn to_encoder_list(env: &mut Env<'_>, fn_name: Symbol) -> (Expr, Variable) { function_type: to_elem_encoder_fn_var, closure_type: to_elem_encoder_lset, return_type: elem_encoder_var, + early_returns: vec![], name: to_elem_encoder_sym, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -281,6 +282,7 @@ fn to_encoder_list(env: &mut Env<'_>, fn_name: Symbol) -> (Expr, Variable) { function_type: fn_var, closure_type: fn_clos_var, return_type: this_encoder_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -490,6 +492,7 @@ fn to_encoder_record( function_type: fn_var, closure_type: fn_clos_var, return_type: this_encoder_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -672,6 +675,7 @@ fn to_encoder_tuple( function_type: fn_var, closure_type: fn_clos_var, return_type: this_encoder_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -914,6 +918,7 @@ fn to_encoder_tag_union( function_type: fn_var, closure_type: fn_clos_var, return_type: this_encoder_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -1025,6 +1030,7 @@ fn wrap_in_encode_custom( function_type: fn_var, closure_type: fn_clos_var, return_type: Variable::LIST_U8, + early_returns: vec![], name: fn_name, captured_symbols: vec![(captured_symbol, captured_var)], recursive: Recursive::NotRecursive, diff --git a/crates/compiler/derive/src/hash.rs b/crates/compiler/derive/src/hash.rs index 53697d5fef9..1cb06d17093 100644 --- a/crates/compiler/derive/src/hash.rs +++ b/crates/compiler/derive/src/hash.rs @@ -542,6 +542,7 @@ fn build_outer_derived_closure( function_type: fn_var, closure_type: fn_clos_var, return_type: body_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, diff --git a/crates/compiler/derive/src/inspect.rs b/crates/compiler/derive/src/inspect.rs index 7881dd6e979..b0da2dac1e3 100644 --- a/crates/compiler/derive/src/inspect.rs +++ b/crates/compiler/derive/src/inspect.rs @@ -194,6 +194,7 @@ fn to_inspector_list(env: &mut Env<'_>, fn_name: Symbol) -> (Expr, Variable) { function_type: to_elem_inspector_fn_var, closure_type: to_elem_inspector_lset, return_type: elem_inspector_var, + early_returns: vec![], name: to_elem_inspector_sym, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -292,6 +293,7 @@ fn to_inspector_list(env: &mut Env<'_>, fn_name: Symbol) -> (Expr, Variable) { function_type: fn_var, closure_type: fn_clos_var, return_type: this_inspector_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -502,6 +504,7 @@ fn to_inspector_record( function_type: fn_var, closure_type: fn_clos_var, return_type: this_inspector_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -685,6 +688,7 @@ fn to_inspector_tuple( function_type: fn_var, closure_type: fn_clos_var, return_type: this_inspector_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -931,6 +935,7 @@ fn to_inspector_tag_union( function_type: fn_var, closure_type: fn_clos_var, return_type: this_inspector_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![], recursive: Recursive::NotRecursive, @@ -1029,6 +1034,7 @@ fn wrap_in_inspect_custom( function_type: fn_var, closure_type: fn_clos_var, return_type: fmt_var, + early_returns: vec![], name: fn_name, captured_symbols: vec![(captured_symbol, captured_var)], recursive: Recursive::NotRecursive, diff --git a/crates/compiler/fmt/src/expr.rs b/crates/compiler/fmt/src/expr.rs index c3deed750eb..74912b5e8bc 100644 --- a/crates/compiler/fmt/src/expr.rs +++ b/crates/compiler/fmt/src/expr.rs @@ -14,6 +14,7 @@ use roc_parse::ast::{ }; use roc_parse::ast::{StrLiteral, StrSegment}; use roc_parse::ident::Accessor; +use roc_parse::keyword; use roc_region::all::Loc; impl<'a> Formattable for Expr<'a> { @@ -70,6 +71,9 @@ impl<'a> Formattable for Expr<'a> { LowLevelDbg(_, _, _) => unreachable!( "LowLevelDbg should only exist after desugaring, not during formatting" ), + Return(return_value, after_return) => { + return_value.is_multiline() || after_return.is_some() + } If { if_thens: branches, @@ -452,6 +456,9 @@ impl<'a> Formattable for Expr<'a> { LowLevelDbg(_, _, _) => unreachable!( "LowLevelDbg should only exist after desugaring, not during formatting" ), + Return(return_value, after_return) => { + fmt_return(buf, return_value, after_return, parens, newlines, indent); + } If { if_thens: branches, final_else, @@ -1064,6 +1071,33 @@ fn fmt_expect<'a>( continuation.format(buf, indent); } +fn fmt_return<'a>( + buf: &mut Buf, + return_value: &'a Loc>, + after_return: &Option<&'a Loc>>, + parens: Parens, + newlines: Newlines, + indent: u16, +) { + buf.ensure_ends_with_newline(); + buf.indent(indent); + buf.push_str(keyword::RETURN); + + buf.spaces(1); + + let return_indent = if return_value.is_multiline() { + indent + INDENT + } else { + indent + }; + + return_value.format(buf, return_indent); + + if let Some(after_return) = after_return { + after_return.format_with_options(buf, parens, newlines, indent); + } +} + fn fmt_if<'a>( buf: &mut Buf, branches: &'a [(Loc>, Loc>)], diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index ebcbed64a1e..de94fb3ec13 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -14498,4 +14498,120 @@ All branches in an `if` must have the same type! (*)b "### ); + + test_report!( + return_outside_of_function, + indoc!( + r" + someVal = + if 10 > 5 then + x = 5 + return x + else + 6 + + someVal + 2 + " + ), + @r###" + ── RETURN OUTSIDE OF FUNCTION in /code/proj/Main.roc ─────────────────────────── + + This `return` statement doesn't belong to a function: + + 7│ return x + ^^^^^^^^ + + I wouldn't know where to return to if I used it! + "### + ); + + test_report!( + statements_after_return, + indoc!( + r#" + myFunction = \x -> + if x == 2 then + return x + + log! "someData" + useX x 123 + else + x + 5 + + myFunction 2 + "# + ), + @r###" + ── UNREACHABLE CODE in /code/proj/Main.roc ───────────────────────────────────── + + This code won't run because it follows a `return` statement: + + 6│> return x + 7│> + 8│> log! "someData" + 9│> useX x 123 + + Hint: you can move the `return` statement below this block to make the + code that follows it run. + "### + ); + + test_report!( + return_at_end_of_function, + indoc!( + r#" + myFunction = \x -> + y = Num.toStr x + + return y + + myFunction 3 + "# + ), + @r###" + ── UNNECESSARY RETURN in /code/proj/Main.roc ─────────────────────────────────── + + This `return` keyword is redundant: + + 7│ return y + ^^^^^^^^ + + The last expression in a function is treated like a `return` statement. + You can safely remove `return` here. + "### + ); + + test_report!( + mismatch_early_return_with_function_output, + indoc!( + r#" + myFunction = \x -> + if x == 5 then + return "abc" + else + x + + myFunction 3 + "# + ), + @r###" + ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── + + This `return` statement doesn't match the return type of its enclosing + function: + + 5│ if x == 5 then + 6│> return "abc" + 7│ else + 8│ x + + This returns a value of type: + + Str + + But I expected the function to have return type: + + Num * + "### + ); } diff --git a/crates/compiler/lower_params/src/lower.rs b/crates/compiler/lower_params/src/lower.rs index 43f312ce733..6ef675eadea 100644 --- a/crates/compiler/lower_params/src/lower.rs +++ b/crates/compiler/lower_params/src/lower.rs @@ -220,6 +220,7 @@ impl<'a> LowerParams<'a> { function_type: _, closure_type: _, return_type: _, + early_returns: _, recursive: _, arguments: _, }) => { @@ -380,6 +381,12 @@ impl<'a> LowerParams<'a> { expr_stack.push(&mut loc_message.value); expr_stack.push(&mut loc_continuation.value); } + Return { + return_value, + return_var: _, + } => { + expr_stack.push(&mut return_value.value); + } RecordAccessor(_) | ImportParams(_, _, None) | ZeroArgumentTag { @@ -532,6 +539,7 @@ impl<'a> LowerParams<'a> { function_type: self.var_store.fresh(), closure_type: self.var_store.fresh(), return_type: self.var_store.fresh(), + early_returns: vec![], name: self.unique_symbol(), captured_symbols, recursive: roc_can::expr::Recursive::NotRecursive, diff --git a/crates/compiler/lower_params/src/type_error.rs b/crates/compiler/lower_params/src/type_error.rs index da16882d7e0..7e2e78479c6 100644 --- a/crates/compiler/lower_params/src/type_error.rs +++ b/crates/compiler/lower_params/src/type_error.rs @@ -181,7 +181,8 @@ fn remove_for_reason( def_region: _, } | Reason::CrashArg - | Reason::ImportParams(_) => {} + | Reason::ImportParams(_) + | Reason::FunctionOutput => {} } } diff --git a/crates/compiler/mono/src/ir.rs b/crates/compiler/mono/src/ir.rs index 7a79899c199..6f0a81f960d 100644 --- a/crates/compiler/mono/src/ir.rs +++ b/crates/compiler/mono/src/ir.rs @@ -5888,6 +5888,28 @@ pub fn with_hole<'a>( } } } + Return { + return_value, + return_var, + } => { + let return_symbol = possible_reuse_symbol_or_specialize( + env, + procs, + layout_cache, + &return_value.value, + return_var, + ); + + assign_to_symbol( + env, + procs, + layout_cache, + return_var, + *return_value, + return_symbol, + Stmt::Ret(return_symbol), + ) + } TypedHole(_) => runtime_error(env, "Hit a blank"), RuntimeError(e) => runtime_error(env, env.arena.alloc(e.runtime_message())), Crash { msg, ret_var: _ } => { diff --git a/crates/compiler/parse/src/ast.rs b/crates/compiler/parse/src/ast.rs index 94f771bf943..58af74a1ba6 100644 --- a/crates/compiler/parse/src/ast.rs +++ b/crates/compiler/parse/src/ast.rs @@ -520,6 +520,13 @@ pub enum Expr<'a> { &'a [&'a WhenBranch<'a>], ), + Return( + /// The return value + &'a Loc>, + /// The unused code after the return statement + Option<&'a Loc>>, + ), + // Blank Space (e.g. comments, spaces, newlines) before or after an expression. // We preserve this for the formatter; canonicalization ignores it. SpaceBefore(&'a Expr<'a>, &'a [CommentOrNewline<'a>]), @@ -669,6 +676,9 @@ pub fn is_expr_suffixed(expr: &Expr) -> bool { Expr::When(cond, branches) => { is_expr_suffixed(&cond.value) || branches.iter().any(|x| is_when_branch_suffixed(x)) } + Expr::Return(a, b) => { + is_expr_suffixed(&a.value) || b.is_some_and(|loc_b| is_expr_suffixed(&loc_b.value)) + } Expr::SpaceBefore(a, _) => is_expr_suffixed(a), Expr::SpaceAfter(a, _) => is_expr_suffixed(a), Expr::MalformedIdent(_, _) => false, @@ -938,6 +948,15 @@ impl<'a, 'b> RecursiveValueDefIter<'a, 'b> { expr_stack.push(&condition.value); expr_stack.push(&cont.value); } + Return(return_value, after_return) => { + if let Some(after_return) = after_return { + expr_stack.reserve(2); + expr_stack.push(&return_value.value); + expr_stack.push(&after_return.value); + } else { + expr_stack.push(&return_value.value); + } + } Apply(fun, args, _) => { expr_stack.reserve(args.len() + 1); expr_stack.push(&fun.value); @@ -2464,6 +2483,7 @@ impl<'a> Malformed for Expr<'a> { Dbg => false, DbgStmt(condition, continuation) => condition.is_malformed() || continuation.is_malformed(), LowLevelDbg(_, condition, continuation) => condition.is_malformed() || continuation.is_malformed(), + Return(return_value, after_return) => return_value.is_malformed() || after_return.is_some_and(|ar| ar.is_malformed()), Apply(func, args, _) => func.is_malformed() || args.iter().any(|arg| arg.is_malformed()), BinOps(firsts, last) => firsts.iter().any(|(expr, _)| expr.is_malformed()) || last.is_malformed(), UnaryOp(expr, _) => expr.is_malformed(), diff --git a/crates/compiler/parse/src/expr.rs b/crates/compiler/parse/src/expr.rs index a72d6322af7..b1868a9c4da 100644 --- a/crates/compiler/parse/src/expr.rs +++ b/crates/compiler/parse/src/expr.rs @@ -19,7 +19,7 @@ use crate::parser::{ map_with_arena, optional, reset_min_indent, sep_by1, sep_by1_e, set_min_indent, skip_first, skip_second, specialize_err, specialize_err_ref, then, two_bytes, zero_or_more, EClosure, EExpect, EExpr, EIf, EImport, EImportParams, EInParens, EList, ENumber, EPattern, ERecord, - EString, EType, EWhen, Either, ParseResult, Parser, SpaceProblem, + EReturn, EString, EType, EWhen, Either, ParseResult, Parser, SpaceProblem, }; use crate::pattern::closure_param; use crate::state::State; @@ -546,6 +546,7 @@ fn stmt_start<'a>( EExpr::Dbg, dbg_stmt_help(options, preceding_comment) )), + loc(specialize_err(EExpr::Return, return_help(options))), loc(specialize_err(EExpr::Import, map(import(), Stmt::ValueDef))), map( loc(specialize_err(EExpr::Closure, closure_help(options))), @@ -1443,6 +1444,7 @@ fn parse_stmt_operator<'a>( let op_start = loc_op.region.start(); let op_end = loc_op.region.end(); let new_start = state.pos(); + match op { OperatorOrDef::BinOp(BinOp::Minus) if expr_state.end != op_start && op_end == new_start => { parse_negated_term( @@ -2172,6 +2174,7 @@ fn expr_to_pattern_help<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result( } } +fn return_help<'a>(options: ExprParseOptions) -> impl Parser<'a, Stmt<'a>, EReturn<'a>> { + (move |arena: &'a Bump, state: State<'a>, min_indent| { + let (_, return_kw, state) = loc(parser::keyword(keyword::RETURN, EReturn::Return)) + .parse(arena, state, min_indent)?; + + let (_, return_value, state) = parse_block( + options, + arena, + state, + true, + EReturn::IndentReturnValue, + EReturn::ReturnValue, + ) + .map_err(|(_, f)| (MadeProgress, f))?; + + let region = Region::span_across(&return_kw.region, &return_value.region); + + let stmt = Stmt::Expr(Expr::Return( + arena.alloc(Loc::at(region, return_value.value)), + None, + )); + + Ok((MadeProgress, stmt, state)) + }) + .trace("return_help") +} + fn dbg_stmt_help<'a>( options: ExprParseOptions, preceding_comment: Region, @@ -3060,6 +3090,20 @@ fn stmts_to_defs<'a>( while i < stmts.len() { let sp_stmt = stmts[i]; match sp_stmt.item.value { + Stmt::Expr(Expr::Return(return_value, _after_return)) => { + if i == stmts.len() - 1 { + last_expr = Some(Loc::at_zero(Expr::Return(return_value, None))); + } else { + let rest = stmts_to_expr(&stmts[i + 1..], arena)?; + last_expr = Some(Loc::at_zero(Expr::Return( + return_value, + Some(arena.alloc(rest)), + ))); + } + + // don't re-process the rest of the statements, they got consumed by the early return + break; + } Stmt::Expr(e) => { if is_expr_suffixed(&e) && i + 1 < stmts.len() { defs.push_value_def( diff --git a/crates/compiler/parse/src/keyword.rs b/crates/compiler/parse/src/keyword.rs index 88a13875039..056dd2e8e79 100644 --- a/crates/compiler/parse/src/keyword.rs +++ b/crates/compiler/parse/src/keyword.rs @@ -9,6 +9,7 @@ pub const DBG: &str = "dbg"; pub const IMPORT: &str = "import"; pub const EXPECT: &str = "expect"; pub const EXPECT_FX: &str = "expect-fx"; +pub const RETURN: &str = "return"; pub const CRASH: &str = "crash"; // These keywords are valid in imports @@ -21,6 +22,6 @@ pub const WHERE: &str = "where"; // These keywords are valid in headers pub const PLATFORM: &str = "platform"; -pub const KEYWORDS: [&str; 11] = [ - IF, THEN, ELSE, WHEN, AS, IS, DBG, IMPORT, EXPECT, EXPECT_FX, CRASH, +pub const KEYWORDS: [&str; 12] = [ + IF, THEN, ELSE, WHEN, AS, IS, DBG, IMPORT, EXPECT, EXPECT_FX, RETURN, CRASH, ]; diff --git a/crates/compiler/parse/src/lib.rs b/crates/compiler/parse/src/lib.rs index f92899aa89a..0a80414cacb 100644 --- a/crates/compiler/parse/src/lib.rs +++ b/crates/compiler/parse/src/lib.rs @@ -16,7 +16,6 @@ pub mod keyword; pub mod normalize; pub mod number_literal; pub mod pattern; -pub mod problems; pub mod src64; pub mod state; pub mod string_literal; diff --git a/crates/compiler/parse/src/normalize.rs b/crates/compiler/parse/src/normalize.rs index 833aeb13bb0..3ce3ac6dea2 100644 --- a/crates/compiler/parse/src/normalize.rs +++ b/crates/compiler/parse/src/normalize.rs @@ -3,6 +3,7 @@ use bumpalo::Bump; use roc_module::called_via::{BinOp, UnaryOp}; use roc_region::all::{Loc, Position, Region}; +use crate::parser::EReturn; use crate::{ ast::{ AbilityImpls, AbilityMember, AssignedField, Collection, Defs, Expr, FullAst, Header, @@ -756,6 +757,10 @@ impl<'a> Normalize<'a> for Expr<'a> { arena.alloc(a.normalize(arena)), arena.alloc(b.normalize(arena)), ), + Expr::Return(a, b) => Expr::Return( + arena.alloc(a.normalize(arena)), + b.map(|loc_b| &*arena.alloc(loc_b.normalize(arena))), + ), Expr::Apply(a, b, c) => { Expr::Apply(arena.alloc(a.normalize(arena)), b.normalize(arena), c) } @@ -1038,6 +1043,9 @@ impl<'a> Normalize<'a> for EExpr<'a> { EExpr::Expect(inner_err, _pos) => { EExpr::Expect(inner_err.normalize(arena), Position::zero()) } + EExpr::Return(inner_err, _pos) => { + EExpr::Return(inner_err.normalize(arena), Position::zero()) + } EExpr::Dbg(inner_err, _pos) => EExpr::Dbg(inner_err.normalize(arena), Position::zero()), EExpr::Import(inner_err, _pos) => { EExpr::Import(inner_err.normalize(arena), Position::zero()) @@ -1472,6 +1480,20 @@ impl<'a> Normalize<'a> for EExpect<'a> { } } } + +impl<'a> Normalize<'a> for EReturn<'a> { + fn normalize(&self, arena: &'a Bump) -> Self { + match self { + EReturn::Space(inner_err, _) => EReturn::Space(*inner_err, Position::zero()), + EReturn::Return(_) => EReturn::Return(Position::zero()), + EReturn::ReturnValue(inner_err, _) => { + EReturn::ReturnValue(arena.alloc(inner_err.normalize(arena)), Position::zero()) + } + EReturn::IndentReturnValue(_) => EReturn::IndentReturnValue(Position::zero()), + } + } +} + impl<'a> Normalize<'a> for EIf<'a> { fn normalize(&self, arena: &'a Bump) -> Self { match self { diff --git a/crates/compiler/parse/src/parser.rs b/crates/compiler/parse/src/parser.rs index 71ebcf647b5..7d34903666e 100644 --- a/crates/compiler/parse/src/parser.rs +++ b/crates/compiler/parse/src/parser.rs @@ -97,6 +97,7 @@ impl_space_problem! { EPattern<'a>, EProvides<'a>, ERecord<'a>, + EReturn<'a>, ERequires<'a>, EString<'a>, EType<'a>, @@ -337,6 +338,7 @@ pub enum EExpr<'a> { Expect(EExpect<'a>, Position), Dbg(EExpect<'a>, Position), Import(EImport<'a>, Position), + Return(EReturn<'a>, Position), Closure(EClosure<'a>, Position), Underscore(Position), @@ -513,6 +515,14 @@ pub enum EExpect<'a> { IndentCondition(Position), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum EReturn<'a> { + Space(BadInputError, Position), + Return(Position), + ReturnValue(&'a EExpr<'a>, Position), + IndentReturnValue(Position), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum EImport<'a> { Import(Position), diff --git a/crates/compiler/parse/src/problems.rs b/crates/compiler/parse/src/problems.rs deleted file mode 100644 index 28c3318e31f..00000000000 --- a/crates/compiler/parse/src/problems.rs +++ /dev/null @@ -1,25 +0,0 @@ -use roc_region::all::Loc; - -pub type Problems = Vec>; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Problem { - // UNICODE CODE POINT - /// TODO Invalid hex code - Unicode code points must be specified using hexadecimal characters (the numbers 0-9 and letters A-F) - NonHexCharsInUnicodeCodePt, - /// TODO Invalid Unicode code point. It must be no more than \\u{10FFFF}. - UnicodeCodePtTooLarge, - InvalidUnicodeCodePt, - MalformedEscapedUnicode, - NoUnicodeDigits, - - // STRING LITERAL - NewlineInLiteral, - Tab, - CarriageReturn, - NullChar, - UnsupportedEscapedChar, - - // NUMBER LITERAL - OutsideSupportedRange, -} diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 31af6a39aa9..8591c640b8d 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -242,6 +242,15 @@ pub enum Problem { one_occurrence: Region, kind: AliasKind, }, + ReturnOutsideOfFunction { + region: Region, + }, + StatementsAfterReturn { + region: Region, + }, + ReturnAtEndOfFunction { + region: Region, + }, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -323,6 +332,9 @@ impl Problem { Problem::OverAppliedDbg { .. } => RuntimeError, Problem::DefsOnlyUsedInRecursion(_, _) => Warning, Problem::FileProblem { .. } => Fatal, + Problem::ReturnOutsideOfFunction { .. } => Warning, + Problem::StatementsAfterReturn { .. } => Warning, + Problem::ReturnAtEndOfFunction { .. } => Warning, } } @@ -485,7 +497,10 @@ impl Problem { | Problem::UnappliedCrash { region } | Problem::OverAppliedDbg { region } | Problem::UnappliedDbg { region } - | Problem::DefsOnlyUsedInRecursion(_, region) => Some(*region), + | Problem::DefsOnlyUsedInRecursion(_, region) + | Problem::ReturnOutsideOfFunction { region } + | Problem::StatementsAfterReturn { region } + | Problem::ReturnAtEndOfFunction { region } => Some(*region), Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries)) | Problem::BadRecursion(cycle_entries) => { cycle_entries.first().map(|entry| entry.expr_region) diff --git a/crates/compiler/test_gen/src/gen_return.rs b/crates/compiler/test_gen/src/gen_return.rs new file mode 100644 index 00000000000..c2cb6d4c1cc --- /dev/null +++ b/crates/compiler/test_gen/src/gen_return.rs @@ -0,0 +1,109 @@ +#![cfg(not(feature = "gen-wasm"))] + +#[cfg(feature = "gen-llvm")] +use crate::helpers::llvm::assert_evals_to; + +#[cfg(feature = "gen-dev")] +use crate::helpers::dev::assert_evals_to; + +#[cfg(feature = "gen-llvm")] +use crate::helpers::llvm::identity; + +#[cfg(feature = "gen-dev")] +use crate::helpers::dev::identity; + +#[allow(unused_imports)] +use indoc::indoc; +#[allow(unused_imports)] +use roc_std::{RocList, RocResult, RocStr, I128, U128}; + +#[test] +#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))] +fn early_return_nested_ifs() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [main] to "./platform" + + displayN = \n -> + first = Num.toStr n + second = + if n == 1 then + return "early 1" + else + third = Num.toStr (n + 1) + if n == 2 then + return "early 2" + else + third + + "$(first), $(second)" + + main : List Str + main = List.map [1, 2, 3] displayN + "# + ), + RocList::from_slice(&[ + RocStr::from("early 1"), + RocStr::from("early 2"), + RocStr::from("3, 4") + ]), + RocList + ); +} + +#[test] +#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))] +fn early_return_nested_whens() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [main] to "./platform" + + displayN = \n -> + first = Num.toStr n + second = + when n is + 1 -> + return "early 1" + + _ -> + third = Num.toStr (n + 1) + when n is + 2 -> + return "early 2" + + _ -> + third + + "$(first), $(second)" + + main : List Str + main = List.map [1, 2, 3] displayN + "# + ), + RocList::from_slice(&[ + RocStr::from("early 1"), + RocStr::from("early 2"), + RocStr::from("3, 4") + ]), + RocList + ); +} + +#[test] +#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))] +fn early_return_solo() { + assert_evals_to!( + r#" + identity = \x -> + return x + + identity "abc" + "#, + RocStr::from("abc"), + RocStr, + identity, + true + ); +} diff --git a/crates/compiler/test_gen/src/tests.rs b/crates/compiler/test_gen/src/tests.rs index befc7de4457..41fd4988943 100644 --- a/crates/compiler/test_gen/src/tests.rs +++ b/crates/compiler/test_gen/src/tests.rs @@ -16,6 +16,7 @@ pub mod gen_primitives; pub mod gen_records; pub mod gen_refcount; pub mod gen_result; +pub mod gen_return; pub mod gen_set; pub mod gen_str; pub mod gen_tags; diff --git a/crates/compiler/test_mono/generated/dec_refcount_for_usage_after_early_return_in_if.txt b/crates/compiler/test_mono/generated/dec_refcount_for_usage_after_early_return_in_if.txt new file mode 100644 index 00000000000..9486070c43d --- /dev/null +++ b/crates/compiler/test_mono/generated/dec_refcount_for_usage_after_early_return_in_if.txt @@ -0,0 +1,53 @@ +procedure Bool.11 (#Attr.2, #Attr.3): + let Bool.24 : Int1 = lowlevel Eq #Attr.2 #Attr.3; + ret Bool.24; + +procedure Num.19 (#Attr.2, #Attr.3): + let Num.283 : I64 = lowlevel NumAdd #Attr.2 #Attr.3; + ret Num.283; + +procedure Num.96 (#Attr.2): + let Num.282 : Str = lowlevel NumToStr #Attr.2; + ret Num.282; + +procedure Str.3 (#Attr.2, #Attr.3): + let Str.247 : Str = lowlevel StrConcat #Attr.2 #Attr.3; + ret Str.247; + +procedure Test.1 (Test.2): + let Test.3 : Str = CallByName Num.96 Test.2; + joinpoint Test.12 Test.4: + let Test.10 : Str = ", "; + let Test.9 : Str = CallByName Str.3 Test.10 Test.4; + dec Test.4; + let Test.8 : Str = CallByName Str.3 Test.3 Test.9; + dec Test.9; + ret Test.8; + in + let Test.22 : I64 = 1i64; + let Test.20 : Int1 = CallByName Bool.11 Test.2 Test.22; + if Test.20 then + dec Test.3; + let Test.21 : Str = "early 1"; + ret Test.21; + else + let Test.19 : I64 = 1i64; + let Test.18 : I64 = CallByName Num.19 Test.2 Test.19; + let Test.5 : Str = CallByName Num.96 Test.18; + joinpoint Test.14 Test.11: + jump Test.12 Test.11; + in + let Test.17 : I64 = 2i64; + let Test.15 : Int1 = CallByName Bool.11 Test.2 Test.17; + if Test.15 then + dec Test.3; + dec Test.5; + let Test.16 : Str = "early 2"; + ret Test.16; + else + jump Test.14 Test.5; + +procedure Test.0 (): + let Test.7 : I64 = 3i64; + let Test.6 : Str = CallByName Test.1 Test.7; + ret Test.6; diff --git a/crates/compiler/test_mono/src/tests.rs b/crates/compiler/test_mono/src/tests.rs index 148bb4e417f..87d5ed80181 100644 --- a/crates/compiler/test_mono/src/tests.rs +++ b/crates/compiler/test_mono/src/tests.rs @@ -3672,3 +3672,26 @@ fn issue_6606_2() { " ) } + +#[mono_test] +fn dec_refcount_for_usage_after_early_return_in_if() { + indoc!( + r#" + displayN = \n -> + first = Num.toStr n + second = + if n == 1 then + return "early 1" + else + third = Num.toStr (n + 1) + if n == 2 then + return "early 2" + else + third + + "$(first), $(second)" + + displayN 3 + "# + ) +} diff --git a/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.result-ast new file mode 100644 index 00000000000..0c853950355 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.result-ast @@ -0,0 +1 @@ +Expr(Return(IndentReturnValue(@6), @0), @0) \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.roc b/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.roc new file mode 100644 index 00000000000..a09c86384fa --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/fail/empty_return.expr.roc @@ -0,0 +1 @@ +return diff --git a/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.result-ast new file mode 100644 index 00000000000..83572402651 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.result-ast @@ -0,0 +1 @@ +Expr(Start(@0), @0) \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.roc b/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.roc new file mode 100644 index 00000000000..0a65218cf81 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/fail/return_as_single_line_expr.expr.roc @@ -0,0 +1 @@ +x = return 5 diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.formatted.roc new file mode 100644 index 00000000000..037e118de30 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.formatted.roc @@ -0,0 +1,10 @@ +maybeEarlyReturn = \x -> + y = + if x > 5 then + return "abc" + else + x + 2 + + Num.toStr y + +maybeEarlyReturn 10 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.result-ast new file mode 100644 index 00000000000..410e650571c --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.result-ast @@ -0,0 +1,167 @@ +SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-127, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @0-16 Identifier { + ident: "maybeEarlyReturn", + }, + @19-127 Closure( + [ + @20-21 Identifier { + ident: "x", + }, + ], + @29-127 SpaceBefore( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @29-110, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @29-30 Identifier { + ident: "y", + }, + @41-110 SpaceBefore( + If { + if_thens: [ + ( + @44-49 BinOps( + [ + ( + @44-45 Var { + module_name: "", + ident: "x", + }, + @46-47 GreaterThan, + ), + ], + @48-49 Num( + "5", + ), + ), + @67-79 SpaceBefore( + SpaceAfter( + Return( + @67-79 Str( + PlainLine( + "abc", + ), + ), + None, + ), + [ + Newline, + ], + ), + [ + Newline, + ], + ), + ), + ], + final_else: @105-110 SpaceBefore( + BinOps( + [ + ( + @105-106 Var { + module_name: "", + ident: "x", + }, + @107-108 Plus, + ), + ], + @109-110 Num( + "2", + ), + ), + [ + Newline, + ], + ), + indented_else: false, + }, + [ + Newline, + ], + ), + ), + ], + }, + @116-127 SpaceBefore( + Apply( + @116-125 Var { + module_name: "Num", + ident: "toStr", + }, + [ + @126-127 Var { + module_name: "", + ident: "y", + }, + ], + Space, + ), + [ + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], + ), + ), + ), + ], + }, + @129-148 SpaceBefore( + Apply( + @129-145 Var { + module_name: "", + ident: "maybeEarlyReturn", + }, + [ + @146-148 Num( + "10", + ), + ], + Space, + ), + [ + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.roc new file mode 100644 index 00000000000..18a7c88d61b --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_if.expr.roc @@ -0,0 +1,10 @@ +maybeEarlyReturn = \x -> + y = + if x > 5 then + return "abc" + else + x + 2 + + Num.toStr y + +maybeEarlyReturn 10 diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.formatted.roc new file mode 100644 index 00000000000..abd83147f43 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.formatted.roc @@ -0,0 +1,11 @@ +staticValueDef = + someVal = + if 10 > 5 then + x = 5 + return x + else + 6 + + someVal + 2 + +staticValueDef \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.result-ast new file mode 100644 index 00000000000..94e49d5f7fa --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.result-ast @@ -0,0 +1,169 @@ +SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-142, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @0-14 Identifier { + ident: "staticValueDef", + }, + @21-142 SpaceBefore( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @21-125, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @21-28 Identifier { + ident: "someVal", + }, + @39-125 SpaceBefore( + If { + if_thens: [ + ( + @42-48 BinOps( + [ + ( + @42-44 Num( + "10", + ), + @45-46 GreaterThan, + ), + ], + @47-48 Num( + "5", + ), + ), + @67-97 SpaceBefore( + SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @67-72, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @67-68 Identifier { + ident: "x", + }, + @71-72 Num( + "5", + ), + ), + ], + }, + Return( + @86-97 Var { + module_name: "", + ident: "x", + }, + None, + ), + ), + [ + Newline, + ], + ), + [ + Newline, + ], + ), + ), + ], + final_else: @124-125 SpaceBefore( + Num( + "6", + ), + [ + Newline, + ], + ), + indented_else: false, + }, + [ + Newline, + ], + ), + ), + ], + }, + @131-142 SpaceBefore( + BinOps( + [ + ( + @131-138 Var { + module_name: "", + ident: "someVal", + }, + @139-140 Plus, + ), + ], + @141-142 Num( + "2", + ), + ), + [ + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], + ), + ), + ], + }, + @145-159 SpaceBefore( + Var { + module_name: "", + ident: "staticValueDef", + }, + [ + Newline, + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.roc new file mode 100644 index 00000000000..142207a63a6 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_static_def.expr.roc @@ -0,0 +1,12 @@ +staticValueDef = + someVal = + if 10 > 5 then + x = 5 + return x + else + 6 + + someVal + 2 + + +staticValueDef diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.formatted.roc new file mode 100644 index 00000000000..2fde95e9cf6 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.formatted.roc @@ -0,0 +1,11 @@ +maybeEarlyReturn = \x -> + y = + when x is + 5 -> + return "abc" + + _ -> x + 2 + + Num.toStr y + +maybeEarlyRetun 3 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.result-ast new file mode 100644 index 00000000000..ffca41041bd --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.result-ast @@ -0,0 +1,177 @@ +SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-154, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @0-16 Identifier { + ident: "maybeEarlyReturn", + }, + @19-154 Closure( + [ + @20-21 Identifier { + ident: "x", + }, + ], + @29-154 SpaceBefore( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @29-136, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @29-30 Identifier { + ident: "y", + }, + @37-136 SpaceBefore( + When( + @42-43 Var { + module_name: "", + ident: "x", + }, + [ + WhenBranch { + patterns: [ + @55-56 SpaceBefore( + NumLiteral( + "5", + ), + [ + Newline, + ], + ), + ], + value: @80-116 SpaceBefore( + Return( + @80-116 SpaceBefore( + Str( + PlainLine( + "abc", + ), + ), + [ + Newline, + ], + ), + None, + ), + [ + Newline, + ], + ), + guard: None, + }, + WhenBranch { + patterns: [ + @126-127 SpaceBefore( + Underscore( + "", + ), + [ + Newline, + Newline, + ], + ), + ], + value: @131-136 BinOps( + [ + ( + @131-132 Var { + module_name: "", + ident: "x", + }, + @133-134 Plus, + ), + ], + @135-136 Num( + "2", + ), + ), + guard: None, + }, + ], + ), + [ + Newline, + ], + ), + ), + ], + }, + @143-154 SpaceBefore( + Apply( + @143-152 Var { + module_name: "Num", + ident: "toStr", + }, + [ + @153-154 Var { + module_name: "", + ident: "y", + }, + ], + Space, + ), + [ + Newline, + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], + ), + ), + ), + ], + }, + @156-173 SpaceBefore( + Apply( + @156-171 Var { + module_name: "", + ident: "maybeEarlyRetun", + }, + [ + @172-173 Num( + "3", + ), + ], + Space, + ), + [ + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.roc new file mode 100644 index 00000000000..5f00323c6c1 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_in_when.expr.roc @@ -0,0 +1,13 @@ +maybeEarlyReturn = \x -> + y = + when x is + 5 -> + return + "abc" + + _ -> x + 2 + + + Num.toStr y + +maybeEarlyRetun 3 diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.formatted.roc new file mode 100644 index 00000000000..d617d94c5d8 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.formatted.roc @@ -0,0 +1,3 @@ +return something + |> pipeToFunction + |> andAnother \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.result-ast new file mode 100644 index 00000000000..2d1dc21b698 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.result-ast @@ -0,0 +1,45 @@ +SpaceAfter( + Return( + @0-84 SpaceBefore( + BinOps( + [ + ( + @15-24 SpaceAfter( + Var { + module_name: "", + ident: "something", + }, + [ + Newline, + ], + ), + @37-39 Pizza, + ), + ( + @40-54 SpaceAfter( + Var { + module_name: "", + ident: "pipeToFunction", + }, + [ + Newline, + ], + ), + @71-73 Pizza, + ), + ], + @74-84 Var { + module_name: "", + ident: "andAnother", + }, + ), + [ + Newline, + ], + ), + None, + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.roc new file mode 100644 index 00000000000..0d137b50fd9 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_multiline.expr.roc @@ -0,0 +1,4 @@ +return + something + |> pipeToFunction + |> andAnother diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.formatted.roc new file mode 100644 index 00000000000..7c4a392345b --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.formatted.roc @@ -0,0 +1,4 @@ +identityFn = \x -> + return x + +identityFn 45 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.result-ast new file mode 100644 index 00000000000..0041535b2d1 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.result-ast @@ -0,0 +1,68 @@ +SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-33, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Body( + @0-10 Identifier { + ident: "identityFn", + }, + @13-33 Closure( + [ + @14-15 Identifier { + ident: "x", + }, + ], + @21-33 SpaceBefore( + Return( + @21-33 Var { + module_name: "", + ident: "x", + }, + None, + ), + [ + Newline, + ], + ), + ), + ), + ], + }, + @36-49 SpaceBefore( + Apply( + @36-46 Var { + module_name: "", + ident: "identityFn", + }, + [ + @47-49 Num( + "45", + ), + ], + Space, + ), + [ + Newline, + Newline, + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.roc new file mode 100644 index 00000000000..f305aa09b8b --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/return_only_statement.expr.roc @@ -0,0 +1,5 @@ +identityFn = \x -> + return x + + +identityFn 45 diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index 05bf4d7e6a9..a24855ee090 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -196,6 +196,7 @@ mod test_snapshots { fail/double_plus.expr, fail/elm_function_syntax.expr, fail/empty_or_pattern.expr, + fail/empty_return.expr, fail/error_inline_alias_argument_uppercase.expr, fail/error_inline_alias_not_an_alias.expr, fail/error_inline_alias_qualified.expr, @@ -233,6 +234,7 @@ mod test_snapshots { fail/record_type_open.expr, fail/record_type_open_indent.expr, fail/record_type_tab.expr, + fail/return_as_single_line_expr.expr, fail/single_no_end.expr, fail/tab_crash.header, fail/tag_union_end.expr, @@ -463,6 +465,11 @@ mod test_snapshots { pass/record_updater_var_apply.expr, pass/record_with_if.expr, pass/requires_type.header, + pass/return_in_if.expr, + pass/return_in_static_def.expr, + pass/return_in_when.expr, + pass/return_multiline.expr, + pass/return_only_statement.expr, pass/separate_defs.moduledefs, pass/single_arg_closure.expr, pass/single_underscore_closure.expr, diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index 694f70a3b12..0621989e1af 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -3426,6 +3426,7 @@ pub enum Reason { }, CrashArg, ImportParams(ModuleId), + FunctionOutput, } #[derive(PartialEq, Eq, Debug, Clone)] @@ -3475,6 +3476,7 @@ pub enum Category { Expect, Dbg, + Return, Unknown, } diff --git a/crates/compiler/uitest/tests/recursion/generalization_among_large_recursive_group.txt b/crates/compiler/uitest/tests/recursion/generalization_among_large_recursive_group.txt index 2b36a017b16..c446d872225 100644 --- a/crates/compiler/uitest/tests/recursion/generalization_among_large_recursive_group.txt +++ b/crates/compiler/uitest/tests/recursion/generalization_among_large_recursive_group.txt @@ -3,22 +3,22 @@ app "test" provides [main] to "./platform" f = \{} -> -#^{-1} <2897><113>{} -<116>[[f(1)]]-> <112>[Ok <2905>{}]<76>* +#^{-1} <2897><114>{} -<117>[[f(1)]]-> <113>[Ok <2905>{}]<77>* when g {} is -# ^ <2887><2905>{} -<2895>[[g(2)]]-> <68>[Ok <2905>{}]<98>* +# ^ <2887><2905>{} -<2895>[[g(2)]]-> <69>[Ok <2905>{}]<99>* _ -> Ok {} g = \{} -> -#^{-1} <2887><2905>{} -<2895>[[g(2)]]-> <68>[Ok <2905>{}]<98>* +#^{-1} <2887><2905>{} -<2895>[[g(2)]]-> <69>[Ok <2905>{}]<99>* when h {} is -# ^ <2892><2905>{} -<2900>[[h(3)]]-> <90>[Ok <2905>{}]<120>* +# ^ <2892><2905>{} -<2900>[[h(3)]]-> <91>[Ok <2905>{}]<121>* _ -> Ok {} h = \{} -> -#^{-1} <2892><2905>{} -<2900>[[h(3)]]-> <90>[Ok <2905>{}]<120>* +#^{-1} <2892><2905>{} -<2900>[[h(3)]]-> <91>[Ok <2905>{}]<121>* when f {} is -# ^ <2897><113>{} -<116>[[f(1)]]-> <112>[Ok <2905>{}]<76>* +# ^ <2897><114>{} -<117>[[f(1)]]-> <113>[Ok <2905>{}]<77>* _ -> Ok {} main = f {} -# ^ <2907><129>{} -<132>[[f(1)]]-> <134>[Ok <2905>{}]<2906>w_a +# ^ <2907><130>{} -<133>[[f(1)]]-> <129>[Ok <2905>{}]<2906>w_a diff --git a/crates/language_server/src/analysis/completion/visitor.rs b/crates/language_server/src/analysis/completion/visitor.rs index ad0fb8905c5..83e5a188ba7 100644 --- a/crates/language_server/src/analysis/completion/visitor.rs +++ b/crates/language_server/src/analysis/completion/visitor.rs @@ -214,6 +214,7 @@ impl CompletionVisitor<'_> { DeclarationInfo::Value { expr_var, pattern, .. } => self.patterns(pattern, expr_var), + DeclarationInfo::Return { .. } => vec![], DeclarationInfo::Function { expr_var, pattern, diff --git a/crates/language_server/src/analysis/tokens.rs b/crates/language_server/src/analysis/tokens.rs index 8f645484f61..4c1ed316a41 100644 --- a/crates/language_server/src/analysis/tokens.rs +++ b/crates/language_server/src/analysis/tokens.rs @@ -718,6 +718,11 @@ impl IterTokens for Loc> { Expr::When(e, branches) => (e.iter_tokens(arena).into_iter()) .chain(branches.iter_tokens(arena)) .collect_in(arena), + Expr::Return(ret_expr, after_ret) => ret_expr + .iter_tokens(arena) + .into_iter() + .chain(after_ret.iter_tokens(arena)) + .collect_in(arena), Expr::SpaceBefore(e, _) | Expr::SpaceAfter(e, _) => { Loc::at(region, *e).iter_tokens(arena) } diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 7e60ea07294..f8e7865d31e 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -1346,6 +1346,60 @@ pub fn can_problem<'b>( doc = report.doc; title = report.title; } + + Problem::ReturnOutsideOfFunction { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), + alloc.keyword("return"), + alloc.reflow(" statement doesn't belong to a function:"), + ]), + alloc.region(lines.convert_region(region), severity), + alloc.reflow("I wouldn't know where to return to if I used it!"), + ]); + + title = "RETURN OUTSIDE OF FUNCTION".to_string(); + } + + Problem::StatementsAfterReturn { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This code won't run because it follows a "), + alloc.keyword("return"), + alloc.reflow(" statement:"), + ]), + alloc.region(lines.convert_region(region), severity), + alloc.concat([ + alloc.hint("you can move the "), + alloc.keyword("return"), + alloc.reflow( + " statement below this block to make the code that follows it run.", + ), + ]), + ]); + + title = "UNREACHABLE CODE".to_string(); + } + + Problem::ReturnAtEndOfFunction { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), + alloc.keyword("return"), + alloc.reflow(" keyword is redundant:"), + ]), + alloc.region(lines.convert_region(region), severity), + alloc.concat([ + alloc.reflow("The last expression in a function is treated like a "), + alloc.keyword("return"), + alloc.reflow(" statement. You can safely remove "), + alloc.keyword("return"), + alloc.reflow(" here."), + ]), + ]); + + title = "UNNECESSARY RETURN".to_string(); + } }; Report { diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 23fed78a231..33a04f48105 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -1645,6 +1645,40 @@ fn to_expr_report<'b>( unimplemented!("record default field is not implemented yet") } Reason::ImportParams(_) => unreachable!(), + Reason::FunctionOutput => { + let problem = alloc.concat([ + alloc.text("This "), + alloc.keyword("return"), + alloc.reflow( + " statement doesn't match the return type of its enclosing function:", + ), + ]); + + let comparison = type_comparison( + alloc, + found, + expected_type, + ExpectationContext::Arbitrary, + add_category(alloc, alloc.text("It"), &category), + alloc.reflow("But I expected the function to have return type:"), + None, + ); + + Report { + title: "TYPE MISMATCH".to_string(), + filename, + doc: alloc.stack([ + problem, + alloc.region_with_subregion( + lines.convert_region(region), + lines.convert_region(expr_region), + severity, + ), + comparison, + ]), + severity, + } + } }, } } @@ -1956,7 +1990,7 @@ fn format_category<'b>( } Uniqueness => ( - alloc.concat([this_is, alloc.text(" an uniqueness attribute")]), + alloc.concat([this_is, alloc.text(" a uniqueness attribute")]), alloc.text(" of type:"), ), Crash => { @@ -1983,6 +2017,10 @@ fn format_category<'b>( alloc.concat([this_is, alloc.text(" a dbg statement")]), alloc.text(" of type:"), ), + Return => ( + alloc.concat([text!(alloc, "{}his", t), alloc.reflow(" returns a value")]), + alloc.text(" of type:"), + ), } }