diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 952f15b5b4..1c50a2234b 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -411,6 +411,106 @@ impl Expr { Self::RuntimeError(..) => Category::Unknown, } } + + pub fn contains_any_early_returns(&self) -> bool { + match self { + Self::Num { .. } + | Self::Int { .. } + | Self::Float { .. } + | Self::Str { .. } + | Self::IngestedFile { .. } + | Self::SingleQuote { .. } + | Self::Var { .. } + | Self::AbilityMember { .. } + | Self::ParamsVar { .. } + | Self::Closure(..) + | Self::EmptyRecord + | Self::RecordAccessor(_) + | Self::ZeroArgumentTag { .. } + | Self::OpaqueWrapFunction(_) + | Self::RuntimeError(..) => false, + Self::Return { .. } => true, + Self::List { loc_elems, .. } => loc_elems + .iter() + .any(|elem| elem.value.contains_any_early_returns()), + Self::When { + loc_cond, branches, .. + } => { + loc_cond.value.contains_any_early_returns() + || branches.iter().any(|branch| { + branch + .guard + .as_ref() + .is_some_and(|guard| guard.value.contains_any_early_returns()) + || branch.value.value.contains_any_early_returns() + }) + } + Self::If { + branches, + final_else, + .. + } => { + final_else.value.contains_any_early_returns() + || branches.iter().any(|(cond, then)| { + cond.value.contains_any_early_returns() + || then.value.contains_any_early_returns() + }) + } + Self::LetRec(defs, expr, _cycle_mark) => { + expr.value.contains_any_early_returns() + || defs + .iter() + .any(|def| def.loc_expr.value.contains_any_early_returns()) + } + Self::LetNonRec(def, expr) => { + def.loc_expr.value.contains_any_early_returns() + || expr.value.contains_any_early_returns() + } + Self::Call(_func, args, _called_via) => args + .iter() + .any(|(_var, arg_expr)| arg_expr.value.contains_any_early_returns()), + Self::RunLowLevel { args, .. } | Self::ForeignCall { args, .. } => args + .iter() + .any(|(_var, arg_expr)| arg_expr.contains_any_early_returns()), + Self::Tuple { elems, .. } => elems + .iter() + .any(|(_var, loc_elem)| loc_elem.value.contains_any_early_returns()), + Self::Record { fields, .. } => fields + .iter() + .any(|(_field_name, field)| field.loc_expr.value.contains_any_early_returns()), + Self::RecordAccess { loc_expr, .. } => loc_expr.value.contains_any_early_returns(), + Self::TupleAccess { loc_expr, .. } => loc_expr.value.contains_any_early_returns(), + Self::RecordUpdate { updates, .. } => { + updates.iter().any(|(_field_name, field_update)| { + field_update.loc_expr.value.contains_any_early_returns() + }) + } + Self::ImportParams(_module_id, _region, params) => params + .as_ref() + .is_some_and(|(_var, p)| p.contains_any_early_returns()), + Self::Tag { arguments, .. } => arguments + .iter() + .any(|(_var, arg)| arg.value.contains_any_early_returns()), + Self::OpaqueRef { argument, .. } => argument.1.value.contains_any_early_returns(), + Self::Crash { msg, .. } => msg.value.contains_any_early_returns(), + Self::Dbg { + loc_message, + loc_continuation, + .. + } => { + loc_message.value.contains_any_early_returns() + || loc_continuation.value.contains_any_early_returns() + } + Self::Expect { + loc_condition, + loc_continuation, + .. + } => { + loc_condition.value.contains_any_early_returns() + || loc_continuation.value.contains_any_early_returns() + } + } + } } /// Stores exhaustiveness-checking metadata for a closure argument that may diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index c6ab333fef..a271afffec 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -3437,12 +3437,18 @@ fn constrain_let_def( ) }); - // Ignored def must be effectful, otherwise it's dead code - let effectful_constraint = Constraint::ExpectEffectful( - fx_var, - ExpectEffectfulReason::Ignored, - def.loc_pattern.region, - ); + let effectful_constraint = if def.loc_expr.value.contains_any_early_returns() { + // If the statement has early returns, it doesn't need to be effectful to + // potentially affect the output of the containing function + Constraint::True + } else { + // If there are no early returns, it must be effectful or else it's dead code + Constraint::ExpectEffectful( + fx_var, + ExpectEffectfulReason::Ignored, + def.loc_pattern.region, + ) + }; let enclosing_fx_constraint = constraints.fx_call( fx_var, @@ -3529,9 +3535,14 @@ fn constrain_stmt_def( generalizable, ); - // Stmt expr must be effectful, otherwise it's dead code - let effectful_constraint = - Constraint::ExpectEffectful(fx_var, ExpectEffectfulReason::Stmt, region); + let effectful_constraint = if def.loc_expr.value.contains_any_early_returns() { + // If the statement has early returns, it doesn't need to be effectful to + // potentially affect the output of the containing function + Constraint::True + } else { + // If there are no early returns, it must be effectful or else it's dead code + Constraint::ExpectEffectful(fx_var, ExpectEffectfulReason::Stmt, region) + }; let fx_call_kind = match fn_name { None => FxCallKind::Stmt, diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index 11e275c376..82bb13101c 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -14655,6 +14655,52 @@ All branches in an `if` must have the same type! "### ); + test_report!( + try_in_bare_statement, + indoc!( + r#" + app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" } + + import pf.Effect + + validateNum = \num -> + if num > 5 then + Ok {} + else + Err TooBig + + main! = \{} -> + Effect.putLine! "hello" + + # this returns {}, so it's ignored + try validateNum 10 + + # this returns a value, so we are incorrectly + # dropping the parsed value + try List.get [1, 2, 3] 5 + + Ok {} + "# + ), + @r###" + ── IGNORED RESULT in /code/proj/Main.roc ─────────────────────────────────────── + + The result of this expression is ignored: + + 19│ try List.get [1, 2, 3] 5 + ^^^^^^^^^^^^^^^^^^^^^^^^ + + Standalone statements are required to produce an empty record, but the + type of this one is: + + Num * + + If you still want to ignore it, assign it to `_`, like this: + + _ = File.delete! "data.json" + "### + ); + test_report!( mismatch_only_early_returns, indoc!( @@ -14690,6 +14736,123 @@ All branches in an `if` must have the same type! "### ); + test_report!( + try_with_ignored_output, + indoc!( + r#" + app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" } + + import pf.Effect + + main! = \{} -> + Effect.putLine! "hello" + + # not ignored, warning + try List.get [1, 2, 3] 5 + + # ignored, OK + _ = try List.get [1, 2, 3] 5 + _ignored = try List.get [1, 2, 3] 5 + + Ok {} + "# + ), + @r###" + ── IGNORED RESULT in /code/proj/Main.roc ─────────────────────────────────────── + + The result of this expression is ignored: + + 9│ try List.get [1, 2, 3] 5 + ^^^^^^^^^^^^^^^^^^^^^^^^ + + Standalone statements are required to produce an empty record, but the + type of this one is: + + Num * + + If you still want to ignore it, assign it to `_`, like this: + + _ = File.delete! "data.json" + "### + ); + + test_report!( + no_early_return_in_bare_statement, + indoc!( + r#" + app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" } + + import pf.Effect + + main! = \{} -> + Effect.putLine! "hello" + + Num.toStr 123 + + Ok {} + "# + ), + @r#" + ── IGNORED RESULT in /code/proj/Main.roc ─────────────────────────────────────── + + The result of this call to `Num.toStr` is ignored: + + 8│ Num.toStr 123 + ^^^^^^^^^ + + Standalone statements are required to produce an empty record, but the + type of this one is: + + Str + + If you still want to ignore it, assign it to `_`, like this: + + _ = File.delete! "data.json" + + ── LEFTOVER STATEMENT in /code/proj/Main.roc ─────────────────────────────────── + + This statement does not produce any effects: + + 8│ Num.toStr 123 + ^^^^^^^^^^^^^ + + Standalone statements are only useful if they call effectful + functions. + + Did you forget to use its result? If not, feel free to remove it. + "# + ); + + test_report!( + no_early_return_in_ignored_statement, + indoc!( + r#" + app [main!] { pf: platform "../../../../../crates/cli/tests/test-projects/test-platform-effects-zig/main.roc" } + + import pf.Effect + + main! = \{} -> + Effect.putLine! "hello" + + _ignored = Num.toStr 123 + + Ok {} + "# + ), + @r" + ── UNNECESSARY DEFINITION in /code/proj/Main.roc ─────────────────────────────── + + This assignment doesn't introduce any new variables: + + 8│ _ignored = Num.toStr 123 + ^^^^^^^^ + + Since it doesn't call any effectful functions, this assignment cannot + affect the program's behavior. If you don't need to use the value on + the right-hand side, consider removing the assignment. + " + ); + test_report!( mismatch_early_return_annotated_function, indoc!(