Skip to content

Commit

Permalink
Fix scope management issue when deep-inferring callee.
Browse files Browse the repository at this point in the history
  Fixes #941.

  However, this currently breaks the stdlib somehow with some FreeUnique on the shrinker step of the optimizer.
  • Loading branch information
KtorZ committed May 15, 2024
1 parent b546e42 commit eadf709
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 39 deletions.
33 changes: 33 additions & 0 deletions crates/aiken-lang/src/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2483,3 +2483,36 @@ fn not_indexable() {
Err((_, Error::NotIndexable { .. }))
))
}

#[test]
fn out_of_scope_access() {
let source_code = r#"
pub fn a(x: Int) {
b(x)
}
fn b(y: Int) {
x + y
}
"#;

assert!(matches!(
dbg!(check_validator(parse(source_code))),
Err((_, Error::UnknownVariable { .. }))
))
}

#[test]
fn mutually_recursive_1() {
let source_code = r#"
pub fn foo(x) {
bar(x)
}
pub fn bar(y) {
foo(y)
}
"#;

assert!(check(parse(source_code)).is_ok());
}
11 changes: 9 additions & 2 deletions crates/aiken-lang/src/tipo/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::Type;
use crate::{
ast::{Annotation, BinOp, CallArg, LogicalOpChainKind, Span, UntypedPattern},
ast::{Annotation, BinOp, CallArg, LogicalOpChainKind, Span, UntypedFunction, UntypedPattern},
error::ExtraData,
expr::{self, UntypedExpr},
format::Formatter,
Expand Down Expand Up @@ -1028,6 +1028,12 @@ The best thing to do from here is to remove it."#))]
#[label("unbound generic at boundary")]
location: Span,
},

#[error("Cannot infer caller without inferring callee first")]
MustInferFirst {
function: UntypedFunction,
location: Span,
},
}

impl ExtraData for Error {
Expand Down Expand Up @@ -1085,7 +1091,8 @@ impl ExtraData for Error {
| Error::GenericLeftAtBoundary { .. }
| Error::UnexpectedMultiPatternAssignment { .. }
| Error::ExpectOnOpaqueType { .. }
| Error::ValidatorMustReturnBool { .. } => None,
| Error::ValidatorMustReturnBool { .. }
| Error::MustInferFirst { .. } => None,

Error::UnknownType { name, .. }
| Error::UnknownTypeConstructor { name, .. }
Expand Down
101 changes: 68 additions & 33 deletions crates/aiken-lang/src/tipo/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ use crate::{
line_numbers::LineNumbers,
tipo::{fields::FieldMap, PatternConstructor, TypeVar},
};
use std::{cmp::Ordering, collections::HashMap, ops::Deref, rc::Rc};
use std::{
cmp::Ordering,
collections::{BTreeSet, HashMap},
ops::Deref,
rc::Rc,
};
use vec1::Vec1;

pub(crate) fn infer_function(
Expand Down Expand Up @@ -66,33 +71,67 @@ pub(crate) fn infer_function(
.function_types()
.unwrap_or_else(|| panic!("Preregistered type for fn {name} was not a fn"));

// Infer the type using the preregistered args + return types as a starting point
let (tipo, arguments, body, safe_to_generalise) = environment.in_new_scope(|environment| {
let args = arguments
.iter()
.zip(&args_types)
.map(|(arg_name, tipo)| arg_name.to_owned().set_type(tipo.clone()))
.collect();
// ━━━ open new scope ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
let initial_scope = environment.open_new_scope();

let hydrator = hydrators
.remove(name)
.unwrap_or_else(|| panic!("Could not find hydrator for fn {name}"));
let arguments = arguments
.iter()
.zip(&args_types)
.map(|(arg_name, tipo)| arg_name.to_owned().set_type(tipo.clone()))
.collect();

let mut expr_typer = ExprTyper::new(environment, hydrators, lines, tracing);
let hydrator = hydrators
.remove(name)
.unwrap_or_else(|| panic!("Could not find hydrator for fn {name}"));

expr_typer.hydrator = hydrator;
let mut expr_typer = ExprTyper::new(environment, lines, tracing);
expr_typer.hydrator = hydrator;
expr_typer.unseen = BTreeSet::from_iter(hydrators.keys().cloned());

let (args, body, return_type) =
expr_typer.infer_fn_with_known_types(args, body.to_owned(), Some(return_type))?;
// Infer the type using the preregistered args + return types as a starting point
let inferred =
expr_typer.infer_fn_with_known_types(arguments, body.to_owned(), Some(return_type));

// We try to always perform a deep-first inferrence. So callee are inferred before callers,
// since this provides better -- and necessary -- information in particular with regards to
// generics.
//
// In principle, the compiler requires function definitions to be processed *in order*. So if
// A calls B, we must have inferred B before A. This is detected during inferrence, and we
// raise an error about it. Here however, we backtrack from that error and infer the caller
// first. Then, re-attempt to infer the current function. It may takes multiple attempts, but
// should eventually succeed.
//
// Note that we need to close the scope before backtracking to not mess with the scope of the
// callee. Otherwise, identifiers present in the caller's scope may become available to the
// callee.
if let Err(Error::MustInferFirst { function, .. }) = inferred {
hydrators.insert(name.to_string(), expr_typer.hydrator);

environment.close_scope(initial_scope);

infer_function(
&function,
environment.current_module,
hydrators,
environment,
lines,
tracing,
)?;

let args_types = args.iter().map(|a| a.tipo.clone()).collect();
return infer_function(fun, module_name, hydrators, environment, lines, tracing);
}

let tipo = function(args_types, return_type);
let (arguments, body, return_type) = inferred?;

let args_types = arguments.iter().map(|a| a.tipo.clone()).collect();

let tipo = function(args_types, return_type);

let safe_to_generalise = !expr_typer.ungeneralised_function_used;
let safe_to_generalise = !expr_typer.ungeneralised_function_used;

Ok::<_, Error>((tipo, args, body, safe_to_generalise))
})?;
environment.close_scope(initial_scope);
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

// Assert that the inferred type matches the type of any recursive call
environment.unify(preregistered_type, tipo.clone(), *location, false)?;
Expand Down Expand Up @@ -147,15 +186,16 @@ pub(crate) struct ExprTyper<'a, 'b> {

pub(crate) environment: &'a mut Environment<'b>,

pub(crate) hydrators: &'a mut HashMap<String, Hydrator>,

// We tweak the tracing behavior during type-check. Traces are either kept or left out of the
// typed AST depending on this setting.
pub(crate) tracing: Tracing,

// Type hydrator for creating types from annotations
pub(crate) hydrator: Hydrator,

// A static set of remaining function names that are known but not yet inferred
pub(crate) unseen: BTreeSet<String>,

// We keep track of whether any ungeneralised functions have been used
// to determine whether it is safe to generalise this expression after
// it has been inferred.
Expand All @@ -165,14 +205,13 @@ pub(crate) struct ExprTyper<'a, 'b> {
impl<'a, 'b> ExprTyper<'a, 'b> {
pub fn new(
environment: &'a mut Environment<'b>,
hydrators: &'a mut HashMap<String, Hydrator>,
lines: &'a LineNumbers,
tracing: Tracing,
) -> Self {
Self {
hydrator: Hydrator::new(),
unseen: BTreeSet::new(),
environment,
hydrators,
tracing,
ungeneralised_function_used: false,
lines,
Expand Down Expand Up @@ -2346,15 +2385,11 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
// NOTE: Recursive functions should not run into this multiple time.
// If we have no hydrator for this function, it means that we have already
// encountered it.
if self.hydrators.get(&fun.name).is_some() {
infer_function(
fun,
self.environment.current_module,
self.hydrators,
self.environment,
self.lines,
self.tracing,
)?;
if self.unseen.contains(&fun.name) {
return Err(Error::MustInferFirst {
function: fun.clone(),
location: *location,
});
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/aiken-lang/src/tipo/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ fn infer_definition(
});
}

let typed_via = ExprTyper::new(environment, hydrators, lines, tracing)
.infer(arg.via.clone())?;
let typed_via =
ExprTyper::new(environment, lines, tracing).infer(arg.via.clone())?;

let hydrator: &mut Hydrator = hydrators.get_mut(&f.name).unwrap();

Expand Down Expand Up @@ -624,8 +624,8 @@ fn infer_definition(
value,
tipo: _,
}) => {
let typed_expr = ExprTyper::new(environment, hydrators, lines, tracing)
.infer_const(&annotation, *value)?;
let typed_expr =
ExprTyper::new(environment, lines, tracing).infer_const(&annotation, *value)?;

let tipo = typed_expr.tipo();

Expand Down

0 comments on commit eadf709

Please sign in to comment.