Remove unused let-binding from type-checking

The typed-AST produced as a result of type-checking the program will
  no longer contain unused let-bindings. They still raise warnings in
  the code so that developers are aware that they are being ignore.

  This is mainly done to prevent mistakes for people coming from an
  imperative background who may think that things like:

  ```
  let _ = foo(...)
  ```

  should have some side-effects. It does not, and it's similar to
  assigned variables that are never used / evaluated. We now properly
  strip those elements from the AST when encountered and raise proper
  warnings, even for discarded values.
This commit is contained in:
KtorZ 2023-03-16 16:51:30 +01:00 committed by Lucas
parent 45ea7acc6a
commit a5e505e6b0
3 changed files with 146 additions and 69 deletions

View File

@ -1,6 +1,8 @@
use crate::{
ast::{ModuleKind, Tracing, TypedModule, UntypedModule},
builtins, parser,
ast::{Definition, ModuleKind, Tracing, TypedModule, UntypedModule},
builtins,
expr::TypedExpr,
parser,
tipo::error::{Error, Warning},
IdGenerator,
};
@ -319,3 +321,55 @@ fn utf8_hex_literal_warning() {
Warning::Utf8ByteArrayIsValidHexString { .. }
))
}
#[test]
fn discarded_let_bindings() {
let source_code = r#"
fn foo() {
let result = when 42 is {
1 -> {
let unused = "foo"
Void
}
_ -> {
Void
}
}
let _ = "foo"
result
}
"#;
let (warnings, ast) = check(parse(source_code)).unwrap();
assert!(matches!(warnings[0], Warning::UnusedVariable { ref name, .. } if name == "unused"));
assert!(matches!(warnings[1], Warning::UnusedVariable { ref name, .. } if name == "_"));
// Controls that unused let-bindings have been erased from the transformed AST.
match ast.definitions.first() {
Some(Definition::Fn(def)) => match &def.body {
TypedExpr::Sequence { expressions, .. } => {
assert_eq!(expressions.len(), 2);
assert!(
matches!(expressions[1], TypedExpr::Var { .. }),
"last expression isn't return variable"
);
match &expressions[0] {
TypedExpr::Assignment { value, .. } => match **value {
TypedExpr::When { ref clauses, .. } => {
assert!(
matches!(clauses[0].then, TypedExpr::Sequence { ref expressions, ..} if expressions.len() == 1)
)
}
_ => unreachable!("first expression isn't when/is"),
},
_ => unreachable!("first expression isn't assignment"),
}
}
_ => unreachable!("body isn't a Sequence"),
},
_ => unreachable!("ast isn't a Fn"),
}
}

View File

@ -912,8 +912,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
annotation: &Option<Annotation>,
location: Span,
) -> Result<TypedExpr, Error> {
let typed_value =
self.in_new_scope(|value_typer| value_typer.infer(untyped_value.clone()))?;
let typed_value = self.infer(untyped_value.clone())?;
let mut value_typ = typed_value.tipo();
let value_is_data = value_typ.is_data();
@ -938,6 +937,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
untyped_pattern.clone(),
value_typ.clone(),
Some(ann_typ),
kind.is_let(),
)?
} else {
if value_is_data && !untyped_pattern.is_var() && !untyped_pattern.is_discard() {
@ -963,6 +963,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
untyped_pattern.clone(),
value_typ.clone(),
None,
kind.is_let(),
)?
};
@ -1102,22 +1103,17 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
location,
} = clause;
let (guard, then, typed_pattern, typed_alternatives) =
self.in_new_scope(|clause_typer| {
// Check the types
let (typed_pattern, typed_alternatives) = clause_typer.infer_clause_pattern(
pattern,
alternative_patterns,
subjects,
&location,
)?;
let (guard, then, typed_pattern, typed_alternatives) = self.in_new_scope(|scope| {
// Check the types
let (typed_pattern, typed_alternatives) =
scope.infer_clause_pattern(pattern, alternative_patterns, subjects, &location)?;
let guard = clause_typer.infer_optional_clause_guard(guard)?;
let guard = scope.infer_optional_clause_guard(guard)?;
let then = clause_typer.infer(then)?;
let then = scope.infer(then)?;
Ok::<_, Error>((guard, then, typed_pattern, typed_alternatives))
})?;
Ok::<_, Error>((guard, then, typed_pattern, typed_alternatives))
})?;
Ok(Clause {
location,
@ -1421,7 +1417,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
false,
)?;
let body = self.in_new_scope(|body_typer| body_typer.infer(first.body.clone()))?;
let body = self.infer(first.body.clone())?;
let tipo = body.tipo();
@ -1441,7 +1437,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
false,
)?;
let body = self.in_new_scope(|body_typer| body_typer.infer(branch.body.clone()))?;
let body = self.infer(branch.body.clone())?;
self.unify(
tipo.clone(),
@ -1457,7 +1453,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
});
}
let typed_final_else = self.in_new_scope(|body_typer| body_typer.infer(final_else))?;
let typed_final_else = self.infer(final_else)?;
self.unify(
tipo.clone(),
@ -1507,30 +1503,28 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
) -> Result<(Vec<TypedArg>, TypedExpr), Error> {
self.assert_no_assignment(&body)?;
let (body_rigid_names, body_infer) = self.in_new_scope(|body_typer| {
for (arg, t) in args.iter().zip(args.iter().map(|arg| arg.tipo.clone())) {
match &arg.arg_name {
ArgName::Named { name, .. } => {
body_typer.environment.insert_variable(
name.to_string(),
ValueConstructorVariant::LocalVariable {
location: arg.location,
},
t,
);
for (arg, t) in args.iter().zip(args.iter().map(|arg| arg.tipo.clone())) {
match &arg.arg_name {
ArgName::Named { name, .. } => {
self.environment.insert_variable(
name.to_string(),
ValueConstructorVariant::LocalVariable {
location: arg.location,
},
t,
);
body_typer.environment.init_usage(
name.to_string(),
EntityKind::Variable,
arg.location,
);
}
ArgName::Discarded { .. } => (),
};
}
self.environment.init_usage(
name.to_string(),
EntityKind::Variable,
arg.location,
);
}
ArgName::Discarded { .. } => (),
};
}
(body_typer.hydrator.rigid_names(), body_typer.infer(body))
});
let (body_rigid_names, body_infer) = (self.hydrator.rigid_names(), self.infer(body));
let body = body_infer.map_err(|e| e.with_unify_error_rigid_names(&body_rigid_names))?;
@ -1625,26 +1619,51 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
}
fn infer_seq(&mut self, location: Span, untyped: Vec<UntypedExpr>) -> Result<TypedExpr, Error> {
let count = untyped.len();
let sequence = self.in_new_scope(|scope| {
let count = untyped.len();
let mut expressions = Vec::with_capacity(count);
let mut expressions = Vec::with_capacity(count);
for (i, expression) in untyped.into_iter().enumerate() {
match i.cmp(&(count - 1)) {
// When the expression is the last in a sequence, we enforce it is NOT
// an assignment (kind of treat assignments like statements).
Ordering::Equal => self.assert_no_assignment(&expression)?,
for (i, expression) in untyped.into_iter().enumerate() {
match i.cmp(&(count - 1)) {
// When the expression is the last in a sequence, we enforce it is NOT
// an assignment (kind of treat assignments like statements).
Ordering::Equal => scope.assert_no_assignment(&expression)?,
// This isn't the final expression in the sequence, so it *must*
// be a let-binding; we do not allow anything else.
Ordering::Less => self.assert_assignment(&expression)?,
// This isn't the final expression in the sequence, so it *must*
// be a let-binding; we do not allow anything else.
Ordering::Less => scope.assert_assignment(&expression)?,
// Can't actually happen
Ordering::Greater => (),
// Can't actually happen
Ordering::Greater => (),
}
expressions.push(scope.infer(expression)?);
}
expressions.push(self.infer(expression)?);
}
Ok(expressions)
})?;
let unused = self
.environment
.warnings
.iter()
.filter_map(|w| match w {
Warning::UnusedVariable { location, .. } => Some(*location),
_ => None,
})
.collect::<Vec<_>>();
let expressions = sequence
.into_iter()
.filter(|expr| {
if let TypedExpr::Assignment { pattern, .. } = expr {
!unused.contains(&pattern.location())
} else {
true
}
})
.collect::<Vec<_>>();
Ok(TypedExpr::Sequence {
location,
@ -1874,11 +1893,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
let return_type = self.new_unbound_var();
for subject in subjects {
let subject = self.in_new_scope(|subject_typer| {
let subject = subject_typer.infer(subject)?;
Ok::<_, Error>(subject)
})?;
let subject = self.infer(subject)?;
subject_types.push(subject.tipo());

View File

@ -145,7 +145,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
// Unify each pattern in the multi-pattern with the corresponding subject
let mut typed_multi = Vec::with_capacity(multi_pattern.len());
for (pattern, subject_type) in multi_pattern.into_iter().zip(subjects) {
let pattern = self.unify(pattern, subject_type.clone(), None)?;
let pattern = self.unify(pattern, subject_type.clone(), None, false)?;
typed_multi.push(pattern);
}
Ok(typed_multi)
@ -159,9 +159,17 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
pattern: UntypedPattern,
tipo: Arc<Type>,
ann_type: Option<Arc<Type>>,
is_assignment: bool,
) -> Result<TypedPattern, Error> {
match pattern {
Pattern::Discard { name, location } => Ok(Pattern::Discard { name, location }),
Pattern::Discard { name, location } => {
if is_assignment {
// Register declaration for the unused variable detection
self.environment
.init_usage(name.to_string(), EntityKind::Variable, location);
};
Ok(Pattern::Discard { name, location })
}
Pattern::Var { name, location } => {
self.insert_variable(&name, ann_type.unwrap_or(tipo), location, location)?;
@ -181,7 +189,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
pattern.location(),
)?;
let pattern = self.unify(*pattern, tipo, ann_type)?;
let pattern = self.unify(*pattern, tipo, ann_type, false)?;
Ok(Pattern::Assign {
name,
@ -209,11 +217,11 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
let elements = elements
.into_iter()
.map(|element| self.unify(element, tipo.clone(), None))
.map(|element| self.unify(element, tipo.clone(), None, false))
.try_collect()?;
let tail = match tail {
Some(tail) => Some(Box::new(self.unify(*tail, list(tipo), None)?)),
Some(tail) => Some(Box::new(self.unify(*tail, list(tipo), None, false)?)),
None => None,
};
@ -246,7 +254,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
let mut patterns = vec![];
for (pattern, typ) in elems.into_iter().zip(type_elems) {
let typed_pattern = self.unify(pattern, typ.clone(), None)?;
let typed_pattern = self.unify(pattern, typ.clone(), None, false)?;
patterns.push(typed_pattern);
}
@ -268,7 +276,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
let mut patterns = vec![];
for (pattern, type_) in elems.into_iter().zip(elems_types) {
let typed_pattern = self.unify(pattern, type_, None)?;
let typed_pattern = self.unify(pattern, type_, None, false)?;
patterns.push(typed_pattern);
}
@ -408,7 +416,7 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
label,
} = arg;
let value = self.unify(value, typ.clone(), None)?;
let value = self.unify(value, typ.clone(), None, false)?;
Ok::<_, Error>(CallArg {
value,