Leftover statement warning for pure statements

This commit is contained in:
Agus Zubiaga 2024-10-15 22:33:07 -03:00
parent 6e6382ab23
commit 69e026f8bb
No known key found for this signature in database
7 changed files with 140 additions and 69 deletions

View file

@ -598,6 +598,7 @@ impl Constraints {
| Constraint::Store(..) | Constraint::Store(..)
| Constraint::Lookup(..) | Constraint::Lookup(..)
| Constraint::Pattern(..) | Constraint::Pattern(..)
| Constraint::EffectfulStmt(..)
| Constraint::True | Constraint::True
| Constraint::IsOpenType(_) | Constraint::IsOpenType(_)
| Constraint::IncludesTag(_) | Constraint::IncludesTag(_)
@ -770,6 +771,8 @@ pub enum Constraint {
Index<PatternCategory>, Index<PatternCategory>,
Region, Region,
), ),
/// Expect statement to be effectful
EffectfulStmt(Variable, Region),
/// Used for things that always unify, e.g. blanks and runtime errors /// Used for things that always unify, e.g. blanks and runtime errors
True, True,
SaveTheEnvironment, SaveTheEnvironment,
@ -858,6 +861,9 @@ impl std::fmt::Debug for Constraint {
Self::Pattern(arg0, arg1, arg2, arg3) => { Self::Pattern(arg0, arg1, arg2, arg3) => {
write!(f, "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::True => write!(f, "True"),
Self::SaveTheEnvironment => write!(f, "SaveTheEnvironment"), Self::SaveTheEnvironment => write!(f, "SaveTheEnvironment"),
Self::Let(arg0, arg1) => f.debug_tuple("Let").field(arg0).field(arg1).finish(), Self::Let(arg0, arg1) => f.debug_tuple("Let").field(arg0).field(arg1).finish(),

View file

@ -3481,36 +3481,59 @@ fn constrain_stmt_def(
body_con: Constraint, body_con: Constraint,
fx_var: Variable, fx_var: Variable,
) -> Constraint { ) -> Constraint {
// [purity-inference] TODO: Require fx is effectful let region = def.loc_expr.region;
// Statement expressions must return an empty record // Statement expressions must return an empty record
let empty_record_index = constraints.push_type(types, Types::EMPTY_RECORD); let empty_record_index = constraints.push_type(types, Types::EMPTY_RECORD);
let expected = constraints.push_expected_type(ForReason( let expect_empty_record =
Reason::Stmt, constraints.push_expected_type(ForReason(Reason::Stmt, empty_record_index, region));
empty_record_index,
def.loc_expr.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 expr_con = attach_resolution_constraints(constraints, env, expr_con);
let generalizable = Generalizable(is_generalizable_expr(&def.loc_expr.value)); 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(), std::iter::empty(),
std::iter::empty(), std::iter::empty(),
expr_con, expr_con,
body_con, body_con,
generalizable, 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. /// Create a let-constraint for a non-recursive def.

View file

@ -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!( test_report!(
issue_6240_1, issue_6240_1,
indoc!( indoc!(
@ -14528,38 +14494,38 @@ All branches in an `if` must have the same type!
issue_6240_2, issue_6240_2,
indoc!( indoc!(
r#" r#"
("", "").abcde ("", "").abcde
"# "#
), ),
@r###" @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,
Str, Str,
)a )a
But you are trying to use it as: But you are trying to use it as:
{ abcde : * }b { abcde : * }b
"### "###
); );
test_report!( test_report!(
issue_6240_3, issue_6240_3,
indoc!( indoc!(
r" r"
{}.0 {}.0
" "
), ),
@r###" @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:
@ -14574,6 +14540,37 @@ All branches in an `if` must have the same type!
But you are trying to use it as: But you are trying to use it as:
(*)b (*)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.
"### "###
); );

View file

@ -99,7 +99,8 @@ pub fn remove_module_param_arguments(
| TypeError::IngestedFileUnsupportedType(_, _) | TypeError::IngestedFileUnsupportedType(_, _)
| TypeError::UnexpectedModuleParams(_, _) | TypeError::UnexpectedModuleParams(_, _)
| TypeError::MissingModuleParams(_, _, _) | TypeError::MissingModuleParams(_, _, _)
| TypeError::ModuleParamsMismatch(_, _, _, _) => {} | TypeError::ModuleParamsMismatch(_, _, _, _)
| TypeError::PureStmt(_) => {}
} }
} }
} }

View file

@ -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(index, pool_slice) => {
let let_con = &env.constraints.let_constraints[index.index()]; let let_con = &env.constraints.let_constraints[index.index()];

View file

@ -39,6 +39,7 @@ pub enum TypeError {
UnexpectedModuleParams(Region, ModuleId), UnexpectedModuleParams(Region, ModuleId),
MissingModuleParams(Region, ModuleId, ErrorType), MissingModuleParams(Region, ModuleId, ErrorType),
ModuleParamsMismatch(Region, ModuleId, ErrorType, ErrorType), ModuleParamsMismatch(Region, ModuleId, ErrorType, ErrorType),
PureStmt(Region),
} }
impl TypeError { impl TypeError {
@ -63,6 +64,7 @@ impl TypeError {
TypeError::ModuleParamsMismatch(..) => RuntimeError, TypeError::ModuleParamsMismatch(..) => RuntimeError,
TypeError::IngestedFileBadUtf8(..) => Fatal, TypeError::IngestedFileBadUtf8(..) => Fatal,
TypeError::IngestedFileUnsupportedType(..) => Fatal, TypeError::IngestedFileUnsupportedType(..) => Fatal,
TypeError::PureStmt(..) => Warning,
} }
} }
@ -78,7 +80,8 @@ impl TypeError {
| TypeError::BadPatternMissingAbility(region, ..) | TypeError::BadPatternMissingAbility(region, ..)
| TypeError::UnexpectedModuleParams(region, ..) | TypeError::UnexpectedModuleParams(region, ..)
| TypeError::MissingModuleParams(region, ..) | TypeError::MissingModuleParams(region, ..)
| TypeError::ModuleParamsMismatch(region, ..) => Some(*region), | TypeError::ModuleParamsMismatch(region, ..)
| TypeError::PureStmt(region) => Some(*region),
TypeError::UnfulfilledAbility(ab, ..) => ab.region(), TypeError::UnfulfilledAbility(ab, ..) => ab.region(),
TypeError::Exhaustive(e) => Some(e.region()), TypeError::Exhaustive(e) => Some(e.region()),
TypeError::CircularDef(c) => c.first().map(|ce| ce.symbol_region), TypeError::CircularDef(c) => c.first().map(|ce| ce.symbol_region),

View file

@ -314,6 +314,22 @@ pub fn type_problem<'b>(
severity, 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,
})
}
} }
} }