Allow ignored defs with an effectful RHS

This commit is contained in:
Agus Zubiaga 2024-10-24 00:24:49 -03:00
parent 175a2b5683
commit c9f001b041
No known key found for this signature in database
13 changed files with 136 additions and 126 deletions

View file

@ -0,0 +1,8 @@
app [main!] { pf: platform "../../../../examples/cli/effects-platform/main.roc" }
import pf.Effect
main! : {} => {}
main! = \{} ->
_ = Effect.getLine! {}
Effect.putLine! "I asked for input and I ignored it. Deal with it! 😎"

View file

@ -663,7 +663,7 @@ impl Constraints {
| Constraint::Store(..)
| Constraint::Lookup(..)
| Constraint::Pattern(..)
| Constraint::EffectfulStmt(..)
| Constraint::ExpectEffectful(..)
| Constraint::FxCall(_)
| Constraint::FxSuffix(_)
| Constraint::FlexToPure(_)
@ -845,8 +845,8 @@ pub enum Constraint {
FxSuffix(Index<FxSuffixConstraint>),
/// Set an fx var as pure if flex (no effectful functions were called)
FlexToPure(Variable),
/// Expect statement to be effectful
EffectfulStmt(Variable, Region),
/// Expect statement or ignored def to be effectful
ExpectEffectful(Variable, ExpectEffectfulReason, Region),
/// Used for things that always unify, e.g. blanks and runtime errors
True,
SaveTheEnvironment,
@ -937,6 +937,7 @@ pub struct FxExpectation {
pub enum FxCallKind {
Call(Option<Symbol>),
Stmt,
Ignored,
}
#[derive(Debug, Clone, Copy)]
@ -962,6 +963,12 @@ impl FxSuffixKind {
}
}
#[derive(Debug, Clone, Copy)]
pub enum ExpectEffectfulReason {
Stmt,
Ignored,
}
/// Custom impl to limit vertical space used by the debug output
impl std::fmt::Debug for Constraint {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@ -984,8 +991,8 @@ impl std::fmt::Debug for Constraint {
Self::FxSuffix(arg0) => {
write!(f, "FxSuffix({arg0:?})")
}
Self::EffectfulStmt(arg0, arg1) => {
write!(f, "EffectfulStmt({arg0:?}, {arg1:?})")
Self::ExpectEffectful(arg0, arg1, arg2) => {
write!(f, "EffectfulStmt({arg0:?}, {arg1:?}, {arg2:?})")
}
Self::FlexToPure(arg0) => {
write!(f, "FlexToPure({arg0:?})")

View file

@ -390,6 +390,7 @@ fn deep_copy_expr_help<C: CopyEnv>(env: &mut C, copied: &mut Vec<Variable>, expr
kind: match kind {
DefKind::Let => DefKind::Let,
DefKind::Stmt(v) => DefKind::Stmt(sub!(*v)),
DefKind::Ignored(v) => DefKind::Ignored(sub!(*v)),
},
},
)

View file

@ -112,6 +112,7 @@ fn def<'a>(c: &Ctx, f: &'a Arena<'a>, d: &'a Def) -> DocBuilder<'a, Arena<'a>> {
match kind {
DefKind::Let => def_help(c, f, &loc_pattern.value, &loc_expr.value),
DefKind::Ignored(_) => def_help(c, f, &loc_pattern.value, &loc_expr.value),
DefKind::Stmt(_) => expr(c, EPrec::Free, f, &loc_expr.value),
}
}

View file

@ -97,6 +97,8 @@ pub enum DefKind {
Let,
/// A standalone statement with an fx variable
Stmt(Variable),
/// Ignored result, must be effectful
Ignored(Variable),
}
impl DefKind {
@ -104,6 +106,19 @@ impl DefKind {
match self {
DefKind::Let => DefKind::Let,
DefKind::Stmt(v) => DefKind::Stmt(f(v)),
DefKind::Ignored(v) => DefKind::Ignored(f(v)),
}
}
pub fn from_pattern(var_store: &mut VarStore, pattern: &Loc<Pattern>) -> Self {
if BindingsFromPattern::new(pattern)
.peekable()
.peek()
.is_none()
{
DefKind::Ignored(var_store.fresh())
} else {
DefKind::Let
}
}
}
@ -1212,11 +1227,7 @@ fn canonicalize_value_defs<'a>(
for (def_index, pending_def) in pending_value_defs.iter().enumerate() {
if let Some(loc_pattern) = pending_def.loc_pattern() {
let mut new_bindings = BindingsFromPattern::new(loc_pattern).peekable();
if new_bindings.peek().is_none() {
env.problem(Problem::NoIdentifiersIntroduced(loc_pattern.region));
}
let new_bindings = BindingsFromPattern::new(loc_pattern).peekable();
for (s, r) in new_bindings {
// store the top-level defs, used to ensure that closures won't capture them
@ -2439,6 +2450,8 @@ fn canonicalize_pending_value_def<'a>(
}
Body(loc_can_pattern, loc_expr) => {
//
let def_kind = DefKind::from_pattern(var_store, &loc_can_pattern);
canonicalize_pending_body(
env,
output,
@ -2447,7 +2460,7 @@ fn canonicalize_pending_value_def<'a>(
loc_can_pattern,
loc_expr,
None,
DefKind::Let,
def_kind,
)
}
Stmt(loc_expr) => {

View file

@ -8,8 +8,8 @@ use crate::builtins::{
use crate::pattern::{constrain_pattern, PatternState};
use roc_can::annotation::IntroducedVariables;
use roc_can::constraint::{
Constraint, Constraints, ExpectedTypeIndex, FxCallKind, FxExpectation, Generalizable,
OpportunisticResolve, TypeOrVar,
Constraint, Constraints, ExpectEffectfulReason, ExpectedTypeIndex, FxCallKind, FxExpectation,
Generalizable, OpportunisticResolve, TypeOrVar,
};
use roc_can::def::{Def, DefKind};
use roc_can::exhaustive::{sketch_pattern_to_rows, sketch_when_branches, ExhaustiveContext};
@ -1458,10 +1458,13 @@ pub fn constrain_expr(
while let Some(def) = stack.pop() {
body_con = match def.kind {
DefKind::Let => constrain_let_def(types, constraints, env, def, body_con),
DefKind::Let => constrain_let_def(types, constraints, env, def, body_con, None),
DefKind::Stmt(fx_var) => {
constrain_stmt_def(types, constraints, env, def, body_con, fx_var)
}
DefKind::Ignored(fx_var) => {
constrain_let_def(types, constraints, env, def, body_con, Some(fx_var))
}
};
}
@ -3434,6 +3437,7 @@ fn constrain_let_def(
env: &mut Env,
def: &Def,
body_con: Constraint,
ignored_fx_var: Option<Variable>,
) -> Constraint {
match &def.annotation {
Some(annotation) => constrain_typed_def(types, constraints, env, def, body_con, annotation),
@ -3448,14 +3452,49 @@ fn constrain_let_def(
// no annotation, so no extra work with rigids
let expected = constraints.push_expected_type(NoExpectation(expr_type_index));
let expr_con = constrain_expr(
types,
constraints,
env,
def.loc_expr.region,
&def.loc_expr.value,
expected,
);
let expr_con = match ignored_fx_var {
None => constrain_expr(
types,
constraints,
env,
def.loc_expr.region,
&def.loc_expr.value,
expected,
),
Some(fx_var) => {
let expr_con = env.with_fx_expectation(fx_var, None, |env| {
constrain_expr(
types,
constraints,
env,
def.loc_expr.region,
&def.loc_expr.value,
expected,
)
});
// Ignored def must be effectful, otherwise it's dead code
let effectful_constraint = Constraint::ExpectEffectful(
fx_var,
ExpectEffectfulReason::Ignored,
def.loc_pattern.region,
);
let enclosing_fx_constraint = constraints.fx_call(
fx_var,
FxCallKind::Ignored,
def.loc_pattern.region,
env.fx_expectation,
);
constraints.and_constraint([
expr_con,
enclosing_fx_constraint,
effectful_constraint,
])
}
};
let expr_con = attach_resolution_constraints(constraints, env, expr_con);
let generalizable = Generalizable(is_generalizable_expr(&def.loc_expr.value));
@ -3528,7 +3567,8 @@ fn constrain_stmt_def(
);
// Stmt expr must be effectful, otherwise it's dead code
let effectful_constraint = Constraint::EffectfulStmt(fx_var, region);
let effectful_constraint =
Constraint::ExpectEffectful(fx_var, ExpectEffectfulReason::Stmt, region);
let fx_call_kind = match fn_name {
None => FxCallKind::Stmt,

View file

@ -10170,7 +10170,7 @@ All branches in an `if` must have the same type!
main = \n -> n + 2
"#
),
@r"
@r###"
DUPLICATE NAME in /code/proj/Main.roc
The `main` name is first defined here:
@ -10185,19 +10185,7 @@ All branches in an `if` must have the same type!
Since these variables have the same name, it's easy to use the wrong
one by accident. Give one of them a new name.
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
5 main = \n -> n + 2
^^^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
"
"###
);
test_report!(
@ -10843,51 +10831,47 @@ All branches in an `if` must have the same type!
@r###"
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
This assignment doesn't introduce any new variables:
4 Pair _ _ = Pair 0 1
^^^^^^^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
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.
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
This assignment doesn't introduce any new variables:
6 _ = Pair 0 1
^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
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.
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
This assignment doesn't introduce any new variables:
8 {} = {}
^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
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.
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
This assignment doesn't introduce any new variables:
10 Foo = Foo
^^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
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.
"###
);
@ -10906,55 +10890,7 @@ All branches in an `if` must have the same type!
Foo = Foo
"#
),
@r"
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
3 Pair _ _ = Pair 0 1
^^^^^^^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
5 _ = Pair 0 1
^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
7 {} = {}
^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
UNNECESSARY DEFINITION in /code/proj/Main.roc
This destructure assignment doesn't introduce any new variables:
9 Foo = Foo
^^^
If you don't need to use the value on the right-hand-side of this
assignment, consider removing the assignment. Since Roc is purely
functional, assignments that don't introduce variables cannot affect a
program's behavior!
"
@""
);
test_report!(

View file

@ -102,7 +102,7 @@ pub fn remove_module_param_arguments(
| TypeError::ModuleParamsMismatch(_, _, _, _)
| TypeError::FxInPureFunction(_, _, _)
| TypeError::FxInTopLevel(_, _)
| TypeError::PureStmt(_)
| TypeError::ExpectedEffectful(_, _)
| TypeError::UnsuffixedEffectfulFunction(_, _)
| TypeError::SuffixedPureFunction(_, _) => {}
}

View file

@ -195,7 +195,6 @@ pub enum Problem {
unbound_symbol: Symbol,
region: Region,
},
NoIdentifiersIntroduced(Region),
OverloadedSpecialization {
overload: Region,
original_opaque: Symbol,
@ -318,7 +317,6 @@ impl Problem {
Problem::ImplementsNonRequired { .. } => Warning,
Problem::DoesNotImplementAbility { .. } => RuntimeError,
Problem::NotBoundInAllPatterns { .. } => RuntimeError,
Problem::NoIdentifiersIntroduced(_) => Warning,
Problem::OverloadedSpecialization { .. } => Warning, // Ideally, will compile
Problem::UnnecessaryOutputWildcard { .. } => Warning,
// TODO: sometimes this can just be a warning, e.g. if you have [1, .., .., 2] but we
@ -483,7 +481,6 @@ impl Problem {
| Problem::NotAnAbility(region)
| Problem::ImplementsNonRequired { region, .. }
| Problem::DoesNotImplementAbility { region, .. }
| Problem::NoIdentifiersIntroduced(region)
| Problem::OverloadedSpecialization {
overload: region, ..
}

View file

@ -856,12 +856,12 @@ fn solve(
solve_suffix_fx(env, problems, *kind, actual, region);
state
}
EffectfulStmt(variable, region) => {
ExpectEffectful(variable, reason, region) => {
let content = env.subs.get_content_without_compacting(*variable);
match content {
Content::Pure | Content::FlexVar(_) => {
let problem = TypeError::PureStmt(*region);
let problem = TypeError::ExpectedEffectful(*region, *reason);
problems.push(problem);
state

View file

@ -1,7 +1,7 @@
//! Provides types to describe problems that can occur during solving.
use std::{path::PathBuf, str::Utf8Error};
use roc_can::constraint::FxSuffixKind;
use roc_can::constraint::{ExpectEffectfulReason, FxSuffixKind};
use roc_can::{
constraint::FxCallKind,
expected::{Expected, PExpected},
@ -45,7 +45,7 @@ pub enum TypeError {
ModuleParamsMismatch(Region, ModuleId, ErrorType, ErrorType),
FxInPureFunction(Region, FxCallKind, Option<Region>),
FxInTopLevel(Region, FxCallKind),
PureStmt(Region),
ExpectedEffectful(Region, ExpectEffectfulReason),
UnsuffixedEffectfulFunction(Region, FxSuffixKind),
SuffixedPureFunction(Region, FxSuffixKind),
}
@ -72,7 +72,7 @@ impl TypeError {
TypeError::ModuleParamsMismatch(..) => RuntimeError,
TypeError::IngestedFileBadUtf8(..) => Fatal,
TypeError::IngestedFileUnsupportedType(..) => Fatal,
TypeError::PureStmt(..) => Warning,
TypeError::ExpectedEffectful(..) => Warning,
TypeError::FxInPureFunction(_, _, _) => Warning,
TypeError::FxInTopLevel(_, _) => Warning,
TypeError::UnsuffixedEffectfulFunction(_, _) => Warning,
@ -95,7 +95,7 @@ impl TypeError {
| TypeError::ModuleParamsMismatch(region, ..)
| TypeError::FxInPureFunction(region, _, _)
| TypeError::FxInTopLevel(region, _)
| TypeError::PureStmt(region)
| TypeError::ExpectedEffectful(region, _)
| TypeError::UnsuffixedEffectfulFunction(region, _)
| TypeError::SuffixedPureFunction(region, _) => Some(*region),
TypeError::UnfulfilledAbility(ab, ..) => ab.region(),

View file

@ -1189,14 +1189,6 @@ pub fn can_problem<'b>(
]);
title = "NAME NOT BOUND IN ALL PATTERNS".to_string();
}
Problem::NoIdentifiersIntroduced(region) => {
doc = alloc.stack([
alloc.reflow("This destructure assignment doesn't introduce any new variables:"),
alloc.region(lines.convert_region(region), severity),
alloc.reflow("If you don't need to use the value on the right-hand-side of this assignment, consider removing the assignment. Since Roc is purely functional, assignments that don't introduce variables cannot affect a program's behavior!"),
]);
title = "UNNECESSARY DEFINITION".to_string();
}
Problem::OverloadedSpecialization {
ability_member,
overload,

View file

@ -4,7 +4,7 @@ use crate::error::canonicalize::{to_circular_def_doc, CIRCULAR_DEF};
use crate::report::{Annotation, Report, RocDocAllocator, RocDocBuilder};
use itertools::EitherOrBoth;
use itertools::Itertools;
use roc_can::constraint::{FxCallKind, FxSuffixKind};
use roc_can::constraint::{ExpectEffectfulReason, FxCallKind, FxSuffixKind};
use roc_can::expected::{Expected, PExpected};
use roc_collections::all::{HumanIndex, MutSet, SendMap};
use roc_collections::VecMap;
@ -368,7 +368,7 @@ pub fn type_problem<'b>(
severity,
})
}
PureStmt(region) => {
ExpectedEffectful(region, ExpectEffectfulReason::Stmt) => {
let stack = [
alloc.reflow("This statement does not produce any effects:"),
alloc.region(lines.convert_region(region), severity),
@ -384,6 +384,20 @@ pub fn type_problem<'b>(
severity,
})
}
ExpectedEffectful(region, ExpectEffectfulReason::Ignored) => {
let stack = [
alloc.reflow("This assignment doesn't introduce any new variables:"),
alloc.region(lines.convert_region(region), severity),
alloc.reflow("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.")
];
Some(Report {
title: "UNNECESSARY DEFINITION".to_string(),
filename,
doc: alloc.stack(stack),
severity,
})
}
UnsuffixedEffectfulFunction(
region,
kind @ (FxSuffixKind::Let(symbol) | FxSuffixKind::Pattern(symbol)),
@ -5522,5 +5536,6 @@ fn describe_fx_call_kind<'b>(
]),
FxCallKind::Call(None) => alloc.reflow("This expression calls an effectful function:"),
FxCallKind::Stmt => alloc.reflow("This statement calls an effectful function:"),
FxCallKind::Ignored => alloc.reflow("This ignored def calls an effectful function:"),
}
}