diff --git a/crates/compiler/can/src/constraint.rs b/crates/compiler/can/src/constraint.rs index d15f88280e..46e93b8ebf 100644 --- a/crates/compiler/can/src/constraint.rs +++ b/crates/compiler/can/src/constraint.rs @@ -598,6 +598,7 @@ impl Constraints { | Constraint::Store(..) | Constraint::Lookup(..) | Constraint::Pattern(..) + | Constraint::EffectfulStmt(..) | Constraint::True | Constraint::IsOpenType(_) | Constraint::IncludesTag(_) @@ -770,6 +771,8 @@ pub enum Constraint { Index, Region, ), + /// Expect statement to be effectful + EffectfulStmt(Variable, Region), /// Used for things that always unify, e.g. blanks and runtime errors True, SaveTheEnvironment, @@ -858,6 +861,9 @@ impl std::fmt::Debug for Constraint { Self::Pattern(arg0, arg1, arg2, arg3) => { write!(f, "Pattern({arg0:?}, {arg1:?}, {arg2:?}, {arg3:?})") } + Self::EffectfulStmt(arg0, arg1) => { + write!(f, "EffectfulStmt({arg0:?}, {arg1:?})") + } Self::True => write!(f, "True"), Self::SaveTheEnvironment => write!(f, "SaveTheEnvironment"), Self::Let(arg0, arg1) => f.debug_tuple("Let").field(arg0).field(arg1).finish(), diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index aed4faa92e..ff3a124ad4 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -3481,36 +3481,59 @@ fn constrain_stmt_def( body_con: Constraint, fx_var: Variable, ) -> Constraint { - // [purity-inference] TODO: Require fx is effectful + let region = def.loc_expr.region; // Statement expressions must return an empty record let empty_record_index = constraints.push_type(types, Types::EMPTY_RECORD); - let expected = constraints.push_expected_type(ForReason( - Reason::Stmt, - empty_record_index, - def.loc_expr.region, - )); + let expect_empty_record = + constraints.push_expected_type(ForReason(Reason::Stmt, empty_record_index, region)); + + let expr_con = env.with_enclosing_fx(fx_var, None, |env| { + constrain_expr( + types, + constraints, + env, + region, + &def.loc_expr.value, + expect_empty_record, + ) + }); - let expr_con = constrain_expr( - types, - constraints, - env, - def.loc_expr.region, - &def.loc_expr.value, - expected, - ); let expr_con = attach_resolution_constraints(constraints, env, expr_con); let generalizable = Generalizable(is_generalizable_expr(&def.loc_expr.value)); - constraints.let_constraint( + let body_con = constraints.let_constraint( std::iter::empty(), std::iter::empty(), std::iter::empty(), expr_con, body_con, generalizable, - ) + ); + + // Stmt expr must be effectful, otherwise it's dead code + let effectful_constraint = Constraint::EffectfulStmt(fx_var, region); + + // We have to unify the stmt fx with the enclosing fx + // since we used the former to constrain the expr. + let enclosing_fx_index = match env.enclosing_fx { + Some(enclosing_fn) => { + let enclosing_fx_index = constraints.push_variable(enclosing_fn.fx_var); + + constraints.push_expected_type(ForReason(Reason::Stmt, enclosing_fx_index, region)) + } + None => constraints.push_expected_type(ForReason( + // Statements are not allowed in top-level defs + Reason::Stmt, + constraints.push_variable(Variable::PURE), + region, + )), + }; + let enclosing_fx_constraint = + constraints.equal_types_var(fx_var, enclosing_fx_index, Category::Unknown, region); + + constraints.and_constraint([body_con, effectful_constraint, enclosing_fx_constraint]) } /// Create a let-constraint for a non-recursive def. diff --git a/crates/compiler/load/tests/test_reporting.rs b/crates/compiler/load/tests/test_reporting.rs index 55c5abfaa1..e98eae796e 100644 --- a/crates/compiler/load/tests/test_reporting.rs +++ b/crates/compiler/load/tests/test_reporting.rs @@ -14471,40 +14471,6 @@ All branches in an `if` must have the same type! "# ); - // TODO: add the following tests after built-in Tasks are added - // https://github.com/roc-lang/roc/pull/6836 - - // test_report!( - // suffixed_stmt_invalid_type, - // indoc!( - // r###" - // app "test" provides [main] to "./platform" - - // main : Task U64 _ -> _ - // main = \task -> - // task! - // 42 - // "### - // ), - // @r"" - // ); - - // test_report!( - // suffixed_expr_invalid_type, - // indoc!( - // r###" - // app "test" provides [main] to "./platform" - - // main : Task U64 _ -> _ - // main = \task -> - // result : U32 - // result = task! - // result - // "### - // ), - // @r"" - // ); - test_report!( issue_6240_1, indoc!( @@ -14528,38 +14494,38 @@ All branches in an `if` must have the same type! issue_6240_2, indoc!( r#" - ("", "").abcde - "# + ("", "").abcde + "# ), @r###" - ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── + ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── - This expression is used in an unexpected way: + This expression is used in an unexpected way: - 4│ ("", "").abcde - ^^^^^^^^^^^^^^ + 4│ ("", "").abcde + ^^^^^^^^^^^^^^ - It is a tuple of type: + It is a tuple of type: - ( - Str, - Str, - )a + ( + Str, + Str, + )a - But you are trying to use it as: + But you are trying to use it as: - { abcde : * }b - "### + { abcde : * }b + "### ); test_report!( issue_6240_3, indoc!( r" - {}.0 - " + {}.0 + " ), - @r###" + @r" ── TYPE MISMATCH in /code/proj/Main.roc ──────────────────────────────────────── This expression is used in an unexpected way: @@ -14574,6 +14540,37 @@ All branches in an `if` must have the same type! But you are trying to use it as: (*)b + " + ); + + test_report!( + leftover_statement, + indoc!( + r#" + app [main] { pf: platform "../../../../../examples/cli/effects-platform/main.roc" } + + import pf.Effect + + main = \{} -> + identity {} + + Effect.putLine! "hello" + + identity = \x -> x + "# + ), + @r###" + ── LEFTOVER STATEMENT in /code/proj/Main.roc ─────────────────────────────────── + + This statement does not produce any effects: + + 6│ identity {} + ^^^^^^^^^^^ + + Standalone statements are only useful if they call effectful + functions. + + Did you forget to use its result? If not, feel free to remove it. "### ); diff --git a/crates/compiler/lower_params/src/type_error.rs b/crates/compiler/lower_params/src/type_error.rs index 20c4f3a76a..e0d2aea405 100644 --- a/crates/compiler/lower_params/src/type_error.rs +++ b/crates/compiler/lower_params/src/type_error.rs @@ -99,7 +99,8 @@ pub fn remove_module_param_arguments( | TypeError::IngestedFileUnsupportedType(_, _) | TypeError::UnexpectedModuleParams(_, _) | TypeError::MissingModuleParams(_, _, _) - | TypeError::ModuleParamsMismatch(_, _, _, _) => {} + | TypeError::ModuleParamsMismatch(_, _, _, _) + | TypeError::PureStmt(_) => {} } } } diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 08df3add1c..05152c5cf4 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -777,6 +777,31 @@ fn solve( } } } + EffectfulStmt(variable, region) => { + let content = env.subs.get_content_without_compacting(*variable); + + match content { + Content::Pure | Content::FlexVar(_) => { + let problem = TypeError::PureStmt(*region); + problems.push(problem); + + state + } + Content::Effectful => state, + Content::RigidVar(_) + | Content::FlexAbleVar(_, _) + | Content::RigidAbleVar(_, _) + | Content::RecursionVar { .. } + | Content::LambdaSet(_) + | Content::ErasedLambda + | Content::Structure(_) + | Content::Alias(_, _, _, _) + | Content::RangedNumber(_) + | Content::Error => { + internal_error!("ExpectEffectful: unexpected content: {:?}", content) + } + } + } Let(index, pool_slice) => { let let_con = &env.constraints.let_constraints[index.index()]; diff --git a/crates/compiler/solve_problem/src/lib.rs b/crates/compiler/solve_problem/src/lib.rs index 9eabe601ba..4e49041a1d 100644 --- a/crates/compiler/solve_problem/src/lib.rs +++ b/crates/compiler/solve_problem/src/lib.rs @@ -39,6 +39,7 @@ pub enum TypeError { UnexpectedModuleParams(Region, ModuleId), MissingModuleParams(Region, ModuleId, ErrorType), ModuleParamsMismatch(Region, ModuleId, ErrorType, ErrorType), + PureStmt(Region), } impl TypeError { @@ -63,6 +64,7 @@ impl TypeError { TypeError::ModuleParamsMismatch(..) => RuntimeError, TypeError::IngestedFileBadUtf8(..) => Fatal, TypeError::IngestedFileUnsupportedType(..) => Fatal, + TypeError::PureStmt(..) => Warning, } } @@ -78,7 +80,8 @@ impl TypeError { | TypeError::BadPatternMissingAbility(region, ..) | TypeError::UnexpectedModuleParams(region, ..) | TypeError::MissingModuleParams(region, ..) - | TypeError::ModuleParamsMismatch(region, ..) => Some(*region), + | TypeError::ModuleParamsMismatch(region, ..) + | TypeError::PureStmt(region) => Some(*region), TypeError::UnfulfilledAbility(ab, ..) => ab.region(), TypeError::Exhaustive(e) => Some(e.region()), TypeError::CircularDef(c) => c.first().map(|ce| ce.symbol_region), diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index f05a78bc27..09bf62bc18 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -314,6 +314,22 @@ pub fn type_problem<'b>( severity, }) } + PureStmt(region) => { + let stack = [ + alloc.reflow("This statement does not produce any effects:"), + alloc.region(lines.convert_region(region), severity), + alloc.reflow( + "Standalone statements are only useful if they call effectful functions.", + ), + alloc.reflow("Did you forget to use its result? If not, feel free to remove it."), + ]; + Some(Report { + title: "LEFTOVER STATEMENT".to_string(), + filename, + doc: alloc.stack(stack), + severity, + }) + } } }