From 6121aa9ea79afb19ded59b4e1a13c028ce990e58 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Fri, 30 Dec 2022 20:59:18 +0100 Subject: [PATCH 01/22] If old name is interchangeable, use instead This means that old unions are preserved. However, this doesn't quite explain to me why we get rid of the unnecessary unions. One would expect that we instead get even more unnecessary unions. As of now I can't quite yet explain this change in behaviour, but should be documented once I understand the change in behaviour. --- src/check/constrain/unify/finished.rs | 14 +++++++----- src/check/constrain/unify/function.rs | 2 +- src/check/name/generic.rs | 2 +- src/check/name/mod.rs | 22 +++++++++---------- src/check/name/string_name/mod.rs | 2 +- src/check/name/true_name/mod.rs | 2 +- tests/resource/valid/call/input.mamba | 7 ++++++ tests/resource/valid/call/input_check.py | 7 ++++++ .../valid/operation/arithmetic_check.py | 3 +-- .../valid/readme_example/factorial_check.py | 3 +-- tests/system/valid/call.rs | 5 +++++ 11 files changed, 44 insertions(+), 25 deletions(-) create mode 100644 tests/resource/valid/call/input.mamba create mode 100644 tests/resource/valid/call/input_check.py diff --git a/src/check/constrain/unify/finished.rs b/src/check/constrain/unify/finished.rs index e81821d50..571abe649 100644 --- a/src/check/constrain/unify/finished.rs +++ b/src/check/constrain/unify/finished.rs @@ -20,17 +20,19 @@ impl Finished { /// [Name]. /// Ignores [Any] type, and trims from union. pub fn push_ty(&mut self, ctx: &Context, pos: Position, name: &Name) -> TypeResult<()> { - if *name == Name::any() { - return Ok(()); - } - - // trim undefined should not be needed, underlying issue with current logic + // trim temp should not be needed, underlying issue with current logic let name = name.trim_any().trim_temp(); for class in &name.names { ctx.class(class, pos)?; } - let name = self.pos_to_name.get(&pos).map_or(name.clone(), |s_name| s_name.union(&name)); + let name = self.pos_to_name.get(&pos) + .map_or(name.clone(), |old_name| if old_name.is_interchangeable { + old_name.clone() + } else { + old_name.union(&name) + }); + if self.pos_to_name.insert(pos, name.clone()).is_none() { trace!("{:width$}type at {}: {}", "", pos, name, width = 0); } diff --git a/src/check/constrain/unify/function.rs b/src/check/constrain/unify/function.rs index 7a4dc7c77..5af84850c 100644 --- a/src/check/constrain/unify/function.rs +++ b/src/check/constrain/unify/function.rs @@ -200,7 +200,7 @@ fn unify_fun_arg( let name = names .iter() .fold(Name::empty(), |name, f_name| name.union(f_name)) - .as_any(); + .is_interchangeable(); let ctx_arg_ty = Expected::new(expected.pos, &Type { name }); constr.push("function argument", &ctx_arg_ty, expected); added += 1; diff --git a/src/check/name/generic.rs b/src/check/name/generic.rs index a94b04637..ca448cf20 100644 --- a/src/check/name/generic.rs +++ b/src/check/name/generic.rs @@ -22,7 +22,7 @@ impl TryFrom<&AST> for Name { } else { vec![TrueName::try_from(ast)?].into_iter().collect::>() }; - Ok(Name { names, any: false }) + Ok(Name { names, is_interchangeable: false }) } } diff --git a/src/check/name/mod.rs b/src/check/name/mod.rs index bab70ff90..73c641037 100644 --- a/src/check/name/mod.rs +++ b/src/check/name/mod.rs @@ -64,7 +64,7 @@ pub trait ColType { #[derive(Debug, Clone, Eq)] pub struct Name { pub names: HashSet, - pub any: bool, + pub is_interchangeable: bool, } impl Any for Name { @@ -173,7 +173,7 @@ impl Union for Name { names }; - Name { names, any: self.any || name.any } + Name { names, is_interchangeable: self.is_interchangeable || name.is_interchangeable } } } @@ -241,7 +241,7 @@ impl Display for Name { impl From<&TrueName> for Name { fn from(name: &TrueName) -> Self { let names: HashSet = HashSet::from_iter(vec![name.clone()]); - Name { names, any: false } + Name { names, is_interchangeable: false } } } @@ -262,9 +262,9 @@ impl IsSuperSet for Name { let any_superset: Vec = self.names.iter().map(is_superset).collect::>()?; - if !any_superset.clone().iter().any(|b| *b) && !other.any { + if !any_superset.clone().iter().any(|b| *b) && !other.is_interchangeable { return Ok(false); - } else if any_superset.clone().iter().any(|b| *b) && other.any { + } else if any_superset.clone().iter().any(|b| *b) && other.is_interchangeable { return Ok(true); } } @@ -293,7 +293,7 @@ impl Empty for Name { } fn empty() -> Name { - Name { names: HashSet::new(), any: false } + Name { names: HashSet::new(), is_interchangeable: false } } } @@ -301,7 +301,7 @@ impl Substitute for Name { fn substitute(&self, generics: &HashMap, pos: Position) -> TypeResult { let names = self.names.iter().map(|n| n.substitute(generics, pos)).collect::>()?; - Ok(Name { names, any: self.any }) + Ok(Name { names, is_interchangeable: self.is_interchangeable }) } } @@ -322,8 +322,8 @@ impl Name { /// Any means that if one check if another [is_superset_of] self, then it will be true if it is /// just a superset of one. - pub fn as_any(&self) -> Self { - Name { any: true, ..self.clone() } + pub fn is_interchangeable(&self) -> Self { + Name { is_interchangeable: true, ..self.clone() } } pub fn contains(&self, item: &TrueName) -> bool { @@ -402,7 +402,7 @@ mod tests { #[test] fn is_superset_any_only_one() { - let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).as_any(); + let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(); let union_2 = Name::from(BOOL); let ctx = Context::default().into_with_primitives().unwrap(); @@ -411,7 +411,7 @@ mod tests { #[test] fn is_superset_any_no_one() { - let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).as_any(); + let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(); let union_2 = Name::from(FLOAT); let ctx = Context::default().into_with_primitives().unwrap(); diff --git a/src/check/name/string_name/mod.rs b/src/check/name/string_name/mod.rs index da9289b57..33a428e65 100644 --- a/src/check/name/string_name/mod.rs +++ b/src/check/name/string_name/mod.rs @@ -139,7 +139,7 @@ impl IsSuperSet for StringName { impl From<&StringName> for Name { fn from(name: &StringName) -> Self { - Name { names: HashSet::from_iter(vec![TrueName::from(name)]), any: false } + Name { names: HashSet::from_iter(vec![TrueName::from(name)]), is_interchangeable: false } } } diff --git a/src/check/name/true_name/mod.rs b/src/check/name/true_name/mod.rs index f1037650a..ba376f7c3 100644 --- a/src/check/name/true_name/mod.rs +++ b/src/check/name/true_name/mod.rs @@ -171,7 +171,7 @@ impl From<&TrueName> for StringName { impl From<&Vec> for Name { fn from(names: &Vec) -> Self { let names: HashSet = HashSet::from_iter(names.iter().cloned()); - Name { names, any: false } + Name { names, is_interchangeable: false } } } diff --git a/tests/resource/valid/call/input.mamba b/tests/resource/valid/call/input.mamba new file mode 100644 index 000000000..9f13b3c3d --- /dev/null +++ b/tests/resource/valid/call/input.mamba @@ -0,0 +1,7 @@ +def num := input("Compute factorial: ") + +if num.is_digit() then + def result := Int(num) + print("Factorial {num} is: {result}.") +else + print("Input was not an integer.") diff --git a/tests/resource/valid/call/input_check.py b/tests/resource/valid/call/input_check.py new file mode 100644 index 000000000..3eff08ea7 --- /dev/null +++ b/tests/resource/valid/call/input_check.py @@ -0,0 +1,7 @@ +num: str = input("Compute factorial: ") + +if num.is_digit(): + result: int = int(num) + print(f"Factorial {num} is: {result}.") +else: + print("Input was not an integer.") diff --git a/tests/resource/valid/operation/arithmetic_check.py b/tests/resource/valid/operation/arithmetic_check.py index 0c4b30afa..4ae115dfc 100644 --- a/tests/resource/valid/operation/arithmetic_check.py +++ b/tests/resource/valid/operation/arithmetic_check.py @@ -1,7 +1,6 @@ import math -from typing import Union -a: Union[float, int] = 10 +a: int = 10 b: int = 20 c: int = a + b diff --git a/tests/resource/valid/readme_example/factorial_check.py b/tests/resource/valid/readme_example/factorial_check.py index 9c8f32cf6..2668207fa 100644 --- a/tests/resource/valid/readme_example/factorial_check.py +++ b/tests/resource/valid/readme_example/factorial_check.py @@ -1,4 +1,3 @@ -from typing import Union def factorial(x: int) -> int: match x: case 0: @@ -6,7 +5,7 @@ def factorial(x: int) -> int: case n: return n * factorial(n - 1) -num: Union[flot, int, str] = input("Compute factorial: ") +num: str = input("Compute factorial: ") if num.is_digit(): result = factorial(int(num)) diff --git a/tests/system/valid/call.rs b/tests/system/valid/call.rs index 9ed4111d2..728520257 100644 --- a/tests/system/valid/call.rs +++ b/tests/system/valid/call.rs @@ -4,3 +4,8 @@ use crate::system::{OutTestRet, test_directory}; fn call_with_class_child() -> OutTestRet { test_directory(true, &["call"], &["call", "target"], "call_with_class_child") } + +#[test] +fn input() -> OutTestRet { + test_directory(true, &["call"], &["call", "target"], "input") +} From fc91f5d22bc29b74c319fbfde116d6088a70be37 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Sun, 1 Jan 2023 23:18:26 +0100 Subject: [PATCH 02/22] If expr of var declaration empty, ignore Typically an indication that something somewhere else went wrong. However, an extra failsafe isn't too bad, though it does negatively affect coverage if we fix the problem elsewhere of course, though perhaps we're treating the metric with that line of thought. --- src/generate/convert/definition.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/generate/convert/definition.rs b/src/generate/convert/definition.rs index d8649f771..9ad17756d 100644 --- a/src/generate/convert/definition.rs +++ b/src/generate/convert/definition.rs @@ -42,7 +42,12 @@ pub fn convert_def(ast: &ASTTy, imp: &mut Imports, state: &State, ctx: &Context) match (ty, expression) { (Some(ty), _) => Some(Box::from(convert_node(ty, imp, &state, ctx)?)), (_, Some(expr)) if state.annotate => { - expr.clone().ty.map(|name| name.to_py(imp)).map(Box::from) + let ty = expr.clone().ty.map(|name| name.to_py(imp)).map(Box::from); + if let Some(core) = ty { + if *core == Core::Empty { None } else { Some(core) } + } else { + None + } } _ => None, } From 7444a100a38f94149e980509c6089c4064748852 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Mon, 2 Jan 2023 12:34:36 +0100 Subject: [PATCH 03/22] Fix any_super and is_superset_of in Name --- src/check/constrain/unify/function.rs | 2 +- src/check/name/mod.rs | 48 ++++++++++++++++++--------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/check/constrain/unify/function.rs b/src/check/constrain/unify/function.rs index 5af84850c..0f18d6c45 100644 --- a/src/check/constrain/unify/function.rs +++ b/src/check/constrain/unify/function.rs @@ -200,7 +200,7 @@ fn unify_fun_arg( let name = names .iter() .fold(Name::empty(), |name, f_name| name.union(f_name)) - .is_interchangeable(); + .is_interchangeable(true); let ctx_arg_ty = Expected::new(expected.pos, &Type { name }); constr.push("function argument", &ctx_arg_ty, expected); added += 1; diff --git a/src/check/name/mod.rs b/src/check/name/mod.rs index 73c641037..4f8cfacef 100644 --- a/src/check/name/mod.rs +++ b/src/check/name/mod.rs @@ -257,19 +257,19 @@ impl IsSuperSet for Name { return Ok(false); } + let mut self_is_super_of = vec![]; for name in &other.names { let is_superset = |s_name: &TrueName| s_name.is_superset_of(name, ctx, pos); - let any_superset: Vec = - self.names.iter().map(is_superset).collect::>()?; + let any_superset: Vec<_> = self.names.iter().map(is_superset).collect::>()?; - if !any_superset.clone().iter().any(|b| *b) && !other.is_interchangeable { - return Ok(false); - } else if any_superset.clone().iter().any(|b| *b) && other.is_interchangeable { - return Ok(true); + if !other.is_interchangeable && any_superset.clone().iter().all(|b| !*b) { + return Ok(false); // not a single of self was super } + + self_is_super_of.push(any_superset.iter().any(|b| *b)) } - Ok(true) + Ok(if other.is_interchangeable { self_is_super_of.iter().any(|b| *b)} else { true }) } } @@ -322,8 +322,8 @@ impl Name { /// Any means that if one check if another [is_superset_of] self, then it will be true if it is /// just a superset of one. - pub fn is_interchangeable(&self) -> Self { - Name { is_interchangeable: true, ..self.clone() } + pub fn is_interchangeable(&self, is_interchangeable: bool) -> Self { + Name { is_interchangeable, ..self.clone() } } pub fn contains(&self, item: &TrueName) -> bool { @@ -348,7 +348,7 @@ mod tests { use std::collections::HashSet; use crate::check::context::{clss, Context, LookupClass}; - use crate::check::context::clss::{BOOL, FLOAT, HasParent, INT, STRING, TUPLE}; + use crate::check::context::clss::{BOOL, FLOAT, HasParent, INT, RANGE, STRING, TUPLE}; use crate::check::ident::Identifier; use crate::check::name::{ColType, Empty, IsSuperSet, match_name, Name, Nullable, Union}; use crate::check::name::name_variant::NameVariant; @@ -402,7 +402,7 @@ mod tests { #[test] fn is_superset_any_only_one() { - let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(); + let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(true); let union_2 = Name::from(BOOL); let ctx = Context::default().into_with_primitives().unwrap(); @@ -411,11 +411,29 @@ mod tests { #[test] fn is_superset_any_no_one() { - let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(); - let union_2 = Name::from(FLOAT); + let union_1 = Name::from(&vec![TrueName::from(BOOL), TrueName::from(INT)]).is_interchangeable(true); + let union_2 = Name::from(RANGE); let ctx = Context::default().into_with_primitives().unwrap(); - assert!(union_2.is_superset_of(&union_1, &ctx, Position::default()).unwrap()) + assert!(!union_2.is_superset_of(&union_1, &ctx, Position::default()).unwrap()) + } + + #[test] + fn is_superset_any_no_one_2() { + let union_1 = Name::from(&vec![TrueName::from(INT)]).is_interchangeable(true); + let union_2 = Name::from(STRING); + + let ctx = Context::default().into_with_primitives().unwrap(); + assert!(!union_2.is_superset_of(&union_1, &ctx, Position::default()).unwrap()) + } + + #[test] + fn is_superset_any_nullable() { + let union_1 = Name::from(&vec![TrueName::from(BOOL).as_nullable(), TrueName::from(INT)]).is_interchangeable(true); + let union_2 = Name::from(BOOL); + + let ctx = Context::default().into_with_primitives().unwrap(); + assert!(!union_2.is_superset_of(&union_1, &ctx, Position::default()).unwrap()) } #[test] @@ -639,7 +657,7 @@ mod tests { #[test] fn range_has_collection_int_as_parent() -> TypeResult<()> { - let range_name = Name::from(clss::RANGE); + let range_name = Name::from(RANGE); let int_name = Name::from(INT); let ctx = Context::default().into_with_primitives().unwrap(); From a5d2acda18f417d333083602a9be0f8d87a098e9 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Mon, 2 Jan 2023 14:06:16 +0100 Subject: [PATCH 04/22] [ci skip] Simplify Expression try_from AST By treating all primitives as their type, we are unable to properly substitute. However, does cause some tests to fail which relied on this behaviour: If expressions, however, no longer appear to be typed. --- src/check/constrain/constraint/expected.rs | 11 +---- src/check/constrain/generate/operation.rs | 44 ++++++------------- tests/check/invalid/call.rs | 12 +++++ .../type/call/calls_wrong_primitive.mamba | 3 ++ .../invalid/type/call/calls_wrong_type.mamba | 7 +++ 5 files changed, 38 insertions(+), 39 deletions(-) create mode 100644 tests/resource/invalid/type/call/calls_wrong_primitive.mamba create mode 100644 tests/resource/invalid/type/call/calls_wrong_type.mamba diff --git a/src/check/constrain/constraint/expected.rs b/src/check/constrain/constraint/expected.rs index 77ad51cd0..2b9e52803 100644 --- a/src/check/constrain/constraint/expected.rs +++ b/src/check/constrain/constraint/expected.rs @@ -8,7 +8,7 @@ use std::ops::Deref; use itertools::{EitherOrBoth, Itertools}; use crate::check::constrain::constraint::expected::Expect::*; -use crate::check::context::clss::{BOOL, FLOAT, INT, NONE, STRING}; +use crate::check::context::clss::NONE; use crate::check::name::{Any, Name, Nullable}; use crate::check::name::string_name::StringName; use crate::check::result::{TypeErr, TypeResult}; @@ -99,14 +99,7 @@ impl TryFrom<(&AST, &HashMap)> for Expect { } }); - Ok(match &ast.node { - Node::Int { .. } | Node::ENum { .. } => Type { name: Name::from(INT) }, - Node::Real { .. } => Type { name: Name::from(FLOAT) }, - Node::Bool { .. } => Type { name: Name::from(BOOL) }, - Node::Str { .. } => Type { name: Name::from(STRING) }, - Node::Undefined => Expect::none(), - _ => Expression { ast }, - }) + Ok(Expression { ast }) } } diff --git a/src/check/constrain/generate/operation.rs b/src/check/constrain/generate/operation.rs index 05ea49aa6..994817fe5 100644 --- a/src/check/constrain/generate/operation.rs +++ b/src/check/constrain/generate/operation.rs @@ -7,8 +7,8 @@ use crate::check::constrain::constraint::expected::Expect::*; use crate::check::constrain::generate::{Constrained, gen_vec, generate}; use crate::check::constrain::generate::collection::{constr_col, gen_collection_lookup}; use crate::check::constrain::generate::env::Environment; -use crate::check::context::{clss, Context, LookupClass}; -use crate::check::context::clss::{FLOAT, INT, STRING}; +use crate::check::context::{Context, LookupClass}; +use crate::check::context::clss::{BOOL, FLOAT, INT, RANGE, SLICE, STRING}; use crate::check::context::function::{ADD, DIV, EQ, FDIV, GE, GEQ, LE, LEQ, MOD, MUL, NEQ, POW, SQRT, SUB}; use crate::check::name::{Any, Name}; use crate::check::name::string_name::StringName; @@ -32,19 +32,11 @@ pub fn gen_op( Ok(env.clone()) } Node::Range { .. } => { - constr.add( - "range", - &Expected::new(ast.pos, &Type { name: Name::from(clss::RANGE) }), - &Expected::try_from((ast, &env.var_mappings))?, - ); + primitive(ast, RANGE, env, constr)?; constr_range(ast, env, ctx, constr, "range", true) } Node::Slice { .. } => { - constr.add( - "slice", - &Expected::new(ast.pos, &Type { name: Name::from(clss::SLICE) }), - &Expected::try_from((ast, &env.var_mappings))?, - ); + primitive(ast, SLICE, env, constr)?; constr_range(ast, env, ctx, constr, "slice", false) } @@ -57,24 +49,16 @@ pub fn gen_op( let c = Constraint::stringy("string", &Expected::try_from((expr, &env.var_mappings))?); constr.add_constr(&c); } - - let name = Name::from(STRING); - let left = Expected::try_from((ast, &env.var_mappings))?; - constr.add("string", &left, &Expected::new(ast.pos, &Type { name })); - Ok(env.clone()) + primitive(ast, STRING, env, constr) } Node::Bool { .. } => { - constr.add_constr(&Constraint::truthy( - "if else", - &Expected::try_from((ast, &env.var_mappings))?, - )); - Ok(env.clone()) + let truthy = Constraint::truthy("bool", &Expected::try_from((ast, &env.var_mappings))?); + constr.add_constr(&truthy); + primitive(ast, BOOL, env, constr) } Node::Undefined => { - constr.add_constr(&Constraint::undefined( - "undefined", - &Expected::try_from((ast, &env.var_mappings))?, - )); + let undef = Constraint::undefined("undefined", &Expected::try_from((ast, &env.var_mappings))?); + constr.add_constr(&undef); Ok(env.clone()) } @@ -147,7 +131,7 @@ pub fn gen_op( } Node::Is { left, right } | Node::IsN { left, right } => { - let bool = Expected::new(ast.pos, &Type { name: Name::from(clss::BOOL) }); + let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); bin_op(left, right, env, ctx, constr) } @@ -164,7 +148,7 @@ pub fn gen_op( } Node::Not { expr } => { - let bool = Expected::new(ast.pos, &Type { name: Name::from(clss::BOOL) }); + let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); constr.add_constr(&Constraint::truthy( "not", @@ -174,7 +158,7 @@ pub fn gen_op( Ok(env.clone()) } Node::And { left, right } | Node::Or { left, right } => { - let bool = Expected::new(ast.pos, &Type { name: Name::from(clss::BOOL) }); + let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); constr.add_constr(&Constraint::truthy( @@ -317,7 +301,7 @@ fn impl_bool_op( ); } - let ty = Type { name: Name::from(clss::BOOL) }; + let ty = Type { name: Name::from(BOOL) }; constr.add( "bool operation", &Expected::try_from((ast, &env.var_mappings))?, diff --git a/tests/check/invalid/call.rs b/tests/check/invalid/call.rs index f1079d9cc..16f74efca 100644 --- a/tests/check/invalid/call.rs +++ b/tests/check/invalid/call.rs @@ -8,3 +8,15 @@ fn access_list_with_string() { let source = resource_content(false, &["type", "call"], "call_with_parent.mamba"); check_all(&[*parse(&source).unwrap()]).unwrap_err(); } + +#[test] +fn calls_wrong_primitive() { + let source = resource_content(false, &["type", "call"], "calls_wrong_primitive.mamba"); + check_all(&[*parse(&source).unwrap()]).unwrap_err(); +} + +#[test] +fn calls_wrong_type() { + let source = resource_content(false, &["type", "call"], "calls_wrong_type.mamba"); + check_all(&[*parse(&source).unwrap()]).unwrap_err(); +} diff --git a/tests/resource/invalid/type/call/calls_wrong_primitive.mamba b/tests/resource/invalid/type/call/calls_wrong_primitive.mamba new file mode 100644 index 000000000..d7cb4dc8e --- /dev/null +++ b/tests/resource/invalid/type/call/calls_wrong_primitive.mamba @@ -0,0 +1,3 @@ +def fun_b(x: Int) => print("a") + +fun_b("asdf") diff --git a/tests/resource/invalid/type/call/calls_wrong_type.mamba b/tests/resource/invalid/type/call/calls_wrong_type.mamba new file mode 100644 index 000000000..6796b482b --- /dev/null +++ b/tests/resource/invalid/type/call/calls_wrong_type.mamba @@ -0,0 +1,7 @@ +class MyClass +class OtherClass + +def fun_b(x: MyClass) => print("a") + +def other_class := OtherClass() +fun_b(other_class) From 77e4444cef6d3de67ab76f10c7465d627cde6dad Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Mon, 2 Jan 2023 14:53:15 +0100 Subject: [PATCH 05/22] [ci skip] Split up constraints for if else arms If as a whole is now typed. However, not when used, such as in function body or variable definition. --- src/check/constrain/constraint/builder.rs | 5 ++++- src/check/constrain/generate/control_flow.rs | 15 +++++++++------ src/check/constrain/mod.rs | 7 ++++--- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index eceb22c45..d745fc67f 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -5,6 +5,8 @@ use crate::check::name::string_name::StringName; use crate::check::result::{TypeErr, TypeResult}; use crate::common::position::Position; +const PADDING: usize = 2; + /// Constraint Builder. /// /// Allows us to build sets of constraints. @@ -74,7 +76,8 @@ impl ConstrBuilder { } pub fn add_constr(&mut self, constraint: &Constraint) { - trace!("Constr[{}]: {} == {}, {}: {}", self.level, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); + let gap = String::from_utf8(vec![b' '; self.level * PADDING]).unwrap(); + trace!("{gap}Constr[{}]: {} == {}, {}: {}", self.finished.len(), constraint.left.pos, constraint.right.pos, constraint.msg, constraint); self.constraints[self.level].1.push(constraint.clone()) } diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index a3723f824..cf86c5a3f 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -49,20 +49,23 @@ pub fn gen_flow( constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; + let if_expr_exp = Expected::try_from((ast, &env.var_mappings))?; + constr.new_set(true); let then_env = generate(then, env, ctx, constr)?; + let then_exp = Expected::try_from((then, &then_env.var_mappings))?; constr.exit_set(then.pos)?; + constr.new_set(true); let else_env = generate(el, env, ctx, constr)?; - constr.exit_set(el.pos)?; - if env.is_expr { - let if_expr = Expected::try_from((ast, &env.var_mappings))?; - let then = Expected::try_from((then, &then_env.var_mappings))?; let el = Expected::try_from((el, &else_env.var_mappings))?; + constr.add("else branch equal to if", &el, &if_expr_exp); + } + constr.exit_set(el.pos)?; - constr.add("if then branch", &if_expr, &then); - constr.add("if else branch", &if_expr, &el); + if env.is_expr { + constr.add("then branch equal to if", &then_exp, &if_expr_exp); } Ok(env.union(&then_env.intersect(&else_env))) diff --git a/src/check/constrain/mod.rs b/src/check/constrain/mod.rs index c62a39ed8..2e89b255d 100644 --- a/src/check/constrain/mod.rs +++ b/src/check/constrain/mod.rs @@ -31,14 +31,14 @@ mod tests { let ast = parse(src).unwrap(); let finished = constraints(&ast, &Context::default().into_with_primitives().unwrap()).unwrap().pos_to_name; - assert_eq!(finished.len(), 3); - let pos_bool = Position::new(CaretPos::new(1, 4), CaretPos::new(1, 8)); assert_eq!(finished[&pos_bool], Name::from("Bool")); let pos_10 = Position::new(CaretPos::new(1, 14), CaretPos::new(1, 16)); assert_eq!(finished[&pos_10], Name::from("Int")); let pos_20 = Position::new(CaretPos::new(1, 22), CaretPos::new(1, 24)); assert_eq!(finished[&pos_20], Name::from("Int")); + + assert_eq!(finished.len(), 3); } #[test] @@ -47,7 +47,6 @@ mod tests { let ast = parse(src).unwrap(); let finished = constraints(&ast, &Context::default().into_with_primitives().unwrap()).unwrap().pos_to_name; - assert_eq!(finished.len(), 5); let pos_20 = Position::new(CaretPos::new(1, 31), CaretPos::new(1, 33)); assert_eq!(finished[&pos_20], Name::from("Int")); let pos_10 = Position::new(CaretPos::new(1, 23), CaretPos::new(1, 25)); @@ -60,6 +59,8 @@ mod tests { let pos_var = Position::new(CaretPos::new(1, 5), CaretPos::new(1, 6)); assert_eq!(finished[&pos_var], Name::from("Int")); + + assert_eq!(finished.len(), 5); } #[test] From 8007f5158aaaad346df55dc41951df0a2920e0ef Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Mon, 2 Jan 2023 15:17:24 +0100 Subject: [PATCH 06/22] [ci skip] Also gen then outside if This is to ensure that the constraints are available outside the if. For instance, when we want to substitute the if with the then branch. We don't use the environment, so we shouldn't have any variable contamination. --- src/check/constrain/generate/control_flow.rs | 1 + src/check/constrain/generate/definition.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index cf86c5a3f..57f2060b3 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -65,6 +65,7 @@ pub fn gen_flow( constr.exit_set(el.pos)?; if env.is_expr { + generate(then, &then_env, ctx, constr)?; // Also gen in outer constr set constr.add("then branch equal to if", &then_exp, &if_expr_exp); } diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 53c274874..378bebabd 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -204,6 +204,7 @@ pub fn identifier_from_var( constr.add(&msg, &var_expect, &expr_expect); } + Ok(env_with_var) } From 42f7d23f879aca81e40c20555b5d2204f1a15a91 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 13:57:09 +0100 Subject: [PATCH 07/22] Constraintbuilder deals with diverging paths We can now create multiple sets which are added to in parallel for diverging paths. The number of active sets is determined by the difference between two internal unsigend numbers. --- src/check/constrain/constraint/builder.rs | 83 +++++++++++-------- src/check/constrain/generate/class.rs | 4 +- src/check/constrain/generate/control_flow.rs | 30 +++---- src/check/constrain/generate/definition.rs | 4 +- src/check/constrain/generate/resources.rs | 11 ++- tests/check/invalid/function.rs | 10 ++- ...ned.mamba => return_if_else_none_el.mamba} | 0 .../function/return_if_else_none_then.mamba | 1 + 8 files changed, 80 insertions(+), 63 deletions(-) rename tests/resource/invalid/type/function/{return_if_else_undefined.mamba => return_if_else_none_el.mamba} (100%) create mode 100644 tests/resource/invalid/type/function/return_if_else_none_then.mamba diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index d745fc67f..187f50dd6 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -1,3 +1,5 @@ +use std::cmp::max; + use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; @@ -16,57 +18,72 @@ const PADDING: usize = 2; /// /// The level indicates how deep we are. A level of 0 indicates that we are at /// the top-level of a script. +/// +/// We use sets to type check all possible execution paths. +/// We can have multiple sets open at a time. +/// When a constraint is added, we add it to each open path. #[derive(Debug)] pub struct ConstrBuilder { pub level: usize, - finished: Vec<(Vec, Vec)>, + pub active_ceil: usize, + constraints: Vec<(Vec, Vec)>, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { level: 0, finished: vec![], constraints: vec![(vec![], vec![])] } + ConstrBuilder { level: 0, active_ceil: 1, constraints: vec![(vec![], vec![])] } } pub fn is_top_level(&self) -> bool { self.level == 0 } - pub fn new_set_in_class(&mut self, inherit_class: bool, class: &StringName) { - self.new_set(true); - if self.level > 0 && inherit_class { - let mut previous = self.constraints[self.level - 1].0.clone(); - self.constraints[self.level].0.append(&mut previous); + pub fn new_set_in_class(&mut self, class: &StringName) -> usize { + let lvl = self.new_set(); + for i in self.level..self.active_ceil { + self.constraints[i].0.push(class.clone()); } - self.constraints[self.level].0.push(class.clone()); + lvl } /// Remove all constraints with where either parent or child is expected pub fn remove_expected(&mut self, expected: &Expected) { - self.constraints[self.level].1 = self.constraints[self.level] - .1 - .clone() - .drain_filter(|con| { - !con.left.expect.same_value(&expected.expect) - && !con.right.expect.same_value(&expected.expect) - }) - .collect() + for i in self.level..self.active_ceil { + self.constraints[i].1 = self.constraints[i] + .1 + .clone() + .drain_filter(|con| { + !con.left.expect.same_value(&expected.expect) + && !con.right.expect.same_value(&expected.expect) + }) + .collect() + } } - pub fn new_set(&mut self, inherit: bool) { - self.constraints.push(if inherit { + /// Create new set, and create marker so that we know what set to exit to upon exit. + pub fn new_set(&mut self) -> usize { + self.constraints.push( (self.constraints[self.level].0.clone(), self.constraints[self.level].1.clone()) - } else { - (vec![], vec![]) - }); - self.level += 1; + ); + + self.active_ceil += 1; + return self.active_ceil; } - pub fn exit_set(&mut self, pos: Position) -> TypeResult<()> { - if self.level == 0 { + pub fn exit_set_to(&mut self, level: usize, pos: Position) -> TypeResult<()> { + let level = max(1, level); + if self.active_ceil == 1 { return Err(vec![TypeErr::new(pos, "Cannot exit top-level set")]); + } else if level > self.active_ceil { + return Err(vec![TypeErr::new(pos, "Exiting constraint set which doesn't exist")]); + } + + if level > self.level { + self.active_ceil = level; // self.level untouched + } else { + self.level = level; + self.active_ceil = self.level + 1; } - self.finished.push(self.constraints.remove(self.level)); - self.level -= 1; Ok(()) } @@ -75,10 +92,14 @@ impl ConstrBuilder { self.add_constr(&Constraint::new(msg, parent, child)); } + /// Add constraint to currently all op sets. + /// The open sets are the sets at levels between the self.level and active ceiling. pub fn add_constr(&mut self, constraint: &Constraint) { let gap = String::from_utf8(vec![b' '; self.level * PADDING]).unwrap(); - trace!("{gap}Constr[{}]: {} == {}, {}: {}", self.finished.len(), constraint.left.pos, constraint.right.pos, constraint.msg, constraint); - self.constraints[self.level].1.push(constraint.clone()) + for i in self.level..self.active_ceil { + trace!("{gap}Constr[{}]: {} == {}, {}: {}", i, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); + self.constraints[i].1.push(constraint.clone()) + } } pub fn current_class(&self) -> Option { @@ -86,11 +107,7 @@ impl ConstrBuilder { constraints.last().cloned() } - // It is not redundant - #[allow(clippy::redundant_clone)] pub fn all_constr(self) -> Vec { - let mut finished = self.finished.clone(); - finished.append(&mut self.constraints.clone()); - finished.iter().map(Constraints::from).collect() + self.constraints.iter().map(Constraints::from).collect() } } diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index ebd4bd40d..11f9bd2b7 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -47,7 +47,7 @@ pub fn constrain_class_body( constr: &mut ConstrBuilder, ) -> Constrained { let class_name = StringName::try_from(ty.deref())?; - constr.new_set_in_class(true, &class_name); + let class_lvl = constr.new_set_in_class(&class_name); let class_ty_exp = Type { name: Name::from(&class_name) }; let mut env_with_class_fields = env.in_class(&Expected::new(ty.pos, &class_ty_exp)); @@ -62,7 +62,7 @@ pub fn constrain_class_body( ); gen_vec(statements, &env_with_class_fields, true, ctx, constr)?; - constr.exit_set(ty.pos)?; + constr.exit_set_to(class_lvl, ty.pos)?; Ok(env_with_class_fields.clone()) } diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index 57f2060b3..f4c6c34cf 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -51,22 +51,18 @@ pub fn gen_flow( let if_expr_exp = Expected::try_from((ast, &env.var_mappings))?; - constr.new_set(true); + constr.new_set(); let then_env = generate(then, env, ctx, constr)?; let then_exp = Expected::try_from((then, &then_env.var_mappings))?; - constr.exit_set(then.pos)?; - - constr.new_set(true); - let else_env = generate(el, env, ctx, constr)?; if env.is_expr { - let el = Expected::try_from((el, &else_env.var_mappings))?; - constr.add("else branch equal to if", &el, &if_expr_exp); + constr.add("then branch equal to if", &then_exp, &if_expr_exp); } - constr.exit_set(el.pos)?; + constr.new_set(); + let else_env = generate(el, env, ctx, constr)?; if env.is_expr { - generate(then, &then_env, ctx, constr)?; // Also gen in outer constr set - constr.add("then branch equal to if", &then_exp, &if_expr_exp); + let el = Expected::try_from((el, &else_env.var_mappings))?; + constr.add("else branch equal to if", &el, &then_exp); } Ok(env.union(&then_env.intersect(&else_env))) @@ -76,9 +72,8 @@ pub fn gen_flow( constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; - constr.new_set(true); + constr.new_set(); generate(then, env, ctx, constr)?; - constr.exit_set(then.pos)?; Ok(env.clone()) } @@ -90,24 +85,24 @@ pub fn gen_flow( } Node::For { expr, col, body } => { - constr.new_set(true); + let loop_lvl = constr.new_set(); let col_env = generate(col, env, ctx, constr)?; let is_define_mode = col_env.is_def_mode; let lookup_env = gen_collection_lookup(expr, col, &col_env.is_def_mode(true), constr)?; generate(body, &lookup_env.in_loop().is_def_mode(is_define_mode), ctx, constr)?; - constr.exit_set(ast.pos)?; + constr.exit_set_to(loop_lvl, ast.pos)?; Ok(env.clone()) } Node::While { cond, body } => { - constr.new_set(true); + let while_lvl = constr.new_set(); let cond_exp = Expected::try_from((cond, &env.var_mappings))?; constr.add_constr(&Constraint::truthy("while condition", &cond_exp)); generate(cond, env, ctx, constr)?; generate(body, &env.in_loop(), ctx, constr)?; - constr.exit_set(ast.pos)?; + constr.exit_set_to(while_lvl, ast.pos)?; Ok(env.clone()) } @@ -125,7 +120,7 @@ fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Enviro for case in cases { match &case.node { Node::Case { cond, body } => { - constr.new_set(true); + constr.new_set(); let cond_env = generate(cond, &env.is_def_mode(true), ctx, constr)?; generate(body, &cond_env.is_def_mode(is_define_mode), ctx, constr)?; @@ -141,7 +136,6 @@ fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Enviro let exp_body = Expected::try_from((body, &cond_env.var_mappings))?; constr.add("match arm body", &exp_body, &exp_ast); - constr.exit_set(case.pos)?; } _ => return Err(vec![TypeErr::new(case.pos, "Expected case")]) } diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 378bebabd..63195d702 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -28,7 +28,7 @@ pub fn gen_def( ) -> Constrained { match &ast.node { Node::FunDef { args: fun_args, ret: ret_ty, body, raises, id, .. } => { - constr.new_set(true); + let fun_lvl = constr.new_set(); let non_nullable_class_vars: HashSet = match &id.node { Node::Id { lit } if *lit == function::INIT => { @@ -98,7 +98,7 @@ pub fn gen_def( return Err(unassigned.iter().map(|msg| TypeErr::new(id.pos, msg)).collect()); } - constr.exit_set(ast.pos)?; + constr.exit_set_to(fun_lvl, ast.pos)?; Ok(env.clone()) } Node::FunArg { .. } => { diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index 8f3195767..13068b1e0 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -19,7 +19,7 @@ pub fn gen_resources( ) -> Constrained { match &ast.node { Node::With { resource, alias: Some((alias, mutable, ty)), expr } => { - constr.new_set(true); + let with_lvl = constr.new_set(); let resource_exp = Expected::try_from((resource, &env.var_mappings))?; constr.add("with as", &resource_exp, &Expected::try_from((alias, &env.var_mappings))?); constr.add("with as", &resource_exp, &Expected::new(resource.pos, &Expect::any())); @@ -31,7 +31,7 @@ pub fn gen_resources( let resource_env = generate(resource, env, ctx, constr)?; - constr.new_set(true); + constr.new_set(); constr.remove_expected(&resource_exp); let resource_env = identifier_from_var( alias, @@ -44,12 +44,11 @@ pub fn gen_resources( )?; generate(expr, &resource_env, ctx, constr)?; - constr.exit_set(ast.pos)?; - constr.exit_set(ast.pos)?; + constr.exit_set_to(with_lvl, ast.pos)?; Ok(env.clone()) } Node::With { resource, expr, .. } => { - constr.new_set(true); + let with_lvl = constr.new_set(); constr.add( "with", &Expected::try_from((resource, &env.var_mappings))?, @@ -58,7 +57,7 @@ pub fn gen_resources( let resource_env = generate(resource, env, ctx, constr)?; - constr.exit_set(ast.pos)?; + constr.exit_set_to(with_lvl, ast.pos)?; generate(expr, &resource_env, ctx, constr)?; Ok(env.clone()) } diff --git a/tests/check/invalid/function.rs b/tests/check/invalid/function.rs index 9b8931376..1d640c2a5 100644 --- a/tests/check/invalid/function.rs +++ b/tests/check/invalid/function.rs @@ -118,8 +118,14 @@ fn wrong_return_type() { } #[test] -fn return_if_else_undefined() { - let source = resource_content(false, &["type", "function"], "return_if_else_undefined.mamba"); +fn return_if_else_none_el() { + let source = resource_content(false, &["type", "function"], "return_if_else_none_el.mamba"); + check_all(&[*parse(&source).unwrap()]).unwrap_err(); +} + +#[test] +fn return_if_else_none_then() { + let source = resource_content(false, &["type", "function"], "return_if_else_none_then.mamba"); check_all(&[*parse(&source).unwrap()]).unwrap_err(); } diff --git a/tests/resource/invalid/type/function/return_if_else_undefined.mamba b/tests/resource/invalid/type/function/return_if_else_none_el.mamba similarity index 100% rename from tests/resource/invalid/type/function/return_if_else_undefined.mamba rename to tests/resource/invalid/type/function/return_if_else_none_el.mamba diff --git a/tests/resource/invalid/type/function/return_if_else_none_then.mamba b/tests/resource/invalid/type/function/return_if_else_none_then.mamba new file mode 100644 index 000000000..aa969be76 --- /dev/null +++ b/tests/resource/invalid/type/function/return_if_else_none_then.mamba @@ -0,0 +1 @@ +def f() -> Int => if True then None else 10 From 8056c60db70fd38d014d7c2fc902e214f52c357d Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 14:52:30 +0100 Subject: [PATCH 08/22] [ci skip] Remove old constr, prevent contamination We do now have cross contamination with variable names. Are not properly shadowed now. Remove drain filter in ConstrBuilder The method using this was used in a very specific context in with. If not reverted, can now compile with stable rust! --- src/check/constrain/constraint/builder.rs | 72 ++++++++++------------- src/check/constrain/generate/resources.rs | 1 - src/lib.rs | 2 - 3 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 187f50dd6..578e99b13 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -1,5 +1,7 @@ use std::cmp::max; +use itertools::enumerate; + use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; @@ -24,64 +26,50 @@ const PADDING: usize = 2; /// When a constraint is added, we add it to each open path. #[derive(Debug)] pub struct ConstrBuilder { - pub level: usize, - pub active_ceil: usize, - + finished: Vec<(Vec, Vec)>, constraints: Vec<(Vec, Vec)>, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { level: 0, active_ceil: 1, constraints: vec![(vec![], vec![])] } + ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])] } } - pub fn is_top_level(&self) -> bool { self.level == 0 } + pub fn is_top_level(&self) -> bool { self.constraints.len() == 1 } pub fn new_set_in_class(&mut self, class: &StringName) -> usize { let lvl = self.new_set(); - for i in self.level..self.active_ceil { - self.constraints[i].0.push(class.clone()); + for constraints in &mut self.constraints { + constraints.0.push(class.clone()); } lvl } - /// Remove all constraints with where either parent or child is expected - pub fn remove_expected(&mut self, expected: &Expected) { - for i in self.level..self.active_ceil { - self.constraints[i].1 = self.constraints[i] - .1 - .clone() - .drain_filter(|con| { - !con.left.expect.same_value(&expected.expect) - && !con.right.expect.same_value(&expected.expect) - }) - .collect() - } - } - /// Create new set, and create marker so that we know what set to exit to upon exit. + /// + /// Output may also be ignored. + /// Useful if we don't want to close the set locally but leave open. pub fn new_set(&mut self) -> usize { - self.constraints.push( - (self.constraints[self.level].0.clone(), self.constraints[self.level].1.clone()) - ); - - self.active_ceil += 1; - return self.active_ceil; + let inherited_constraints = self.constraints.last().expect("Can never be empty"); + self.constraints.push(inherited_constraints.clone()); + return self.constraints.len(); } + /// Return to specified level given. + /// + /// - Error if already top-level. + /// - Error if level greater than ceiling, as we cannot exit non-existent sets. pub fn exit_set_to(&mut self, level: usize, pos: Position) -> TypeResult<()> { let level = max(1, level); - if self.active_ceil == 1 { + if level == 0 { return Err(vec![TypeErr::new(pos, "Cannot exit top-level set")]); - } else if level > self.active_ceil { + } else if level > self.constraints.len() { return Err(vec![TypeErr::new(pos, "Exiting constraint set which doesn't exist")]); } - if level > self.level { - self.active_ceil = level; // self.level untouched - } else { - self.level = level; - self.active_ceil = self.level + 1; + for i in (level..self.constraints.len()).rev() { + // Equivalent to pop, but remove has better panic message for debugging + self.finished.push(self.constraints.remove(i)) } Ok(()) @@ -95,19 +83,23 @@ impl ConstrBuilder { /// Add constraint to currently all op sets. /// The open sets are the sets at levels between the self.level and active ceiling. pub fn add_constr(&mut self, constraint: &Constraint) { - let gap = String::from_utf8(vec![b' '; self.level * PADDING]).unwrap(); - for i in self.level..self.active_ceil { + let gap = String::from_utf8(vec![b' '; self.constraints.len() * PADDING]).unwrap(); + + for (i, constraints) in enumerate(&mut self.constraints) { trace!("{gap}Constr[{}]: {} == {}, {}: {}", i, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); - self.constraints[i].1.push(constraint.clone()) + constraints.1.push(constraint.clone()) } } pub fn current_class(&self) -> Option { - let constraints = self.constraints[self.level].clone().0; - constraints.last().cloned() + self.constraints.last().map(|constraints| { + constraints.0.last() + }).flatten().cloned() } pub fn all_constr(self) -> Vec { - self.constraints.iter().map(Constraints::from).collect() + let (mut finished, mut constraints) = (self.finished, self.constraints); + finished.append(&mut constraints); + finished.iter().map(Constraints::from).collect() } } diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index 13068b1e0..e3e43b2d1 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -32,7 +32,6 @@ pub fn gen_resources( let resource_env = generate(resource, env, ctx, constr)?; constr.new_set(); - constr.remove_expected(&resource_exp); let resource_env = identifier_from_var( alias, ty, diff --git a/src/lib.rs b/src/lib.rs index 83255b4f6..7519aa4e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,3 @@ -#![feature(drain_filter)] - extern crate ansi_term; extern crate core; #[macro_use] From e6d40392644f43b839e98fe10c20c9185f374620 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 17:00:02 +0100 Subject: [PATCH 09/22] Move variable mapping to constraint builder --- src/check/constrain/constraint/builder.rs | 34 ++++++++-- src/check/constrain/constraint/expected.rs | 18 +++--- src/check/constrain/generate/call.rs | 32 +++++----- src/check/constrain/generate/class.rs | 7 ++- src/check/constrain/generate/collection.rs | 28 ++++----- src/check/constrain/generate/control_flow.rs | 38 +++-------- src/check/constrain/generate/definition.rs | 27 ++++---- src/check/constrain/generate/env.rs | 61 +++++------------- src/check/constrain/generate/expression.rs | 4 +- src/check/constrain/generate/operation.rs | 66 ++++++++++---------- src/check/constrain/generate/resources.rs | 6 +- src/check/constrain/generate/statement.rs | 2 +- 12 files changed, 154 insertions(+), 169 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 578e99b13..e2901ae36 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -1,4 +1,5 @@ use std::cmp::max; +use std::collections::HashMap; use itertools::enumerate; @@ -11,6 +12,8 @@ use crate::common::position::Position; const PADDING: usize = 2; +pub type VarMapping = HashMap; + /// Constraint Builder. /// /// Allows us to build sets of constraints. @@ -27,16 +30,36 @@ const PADDING: usize = 2; #[derive(Debug)] pub struct ConstrBuilder { finished: Vec<(Vec, Vec)>, - constraints: Vec<(Vec, Vec)>, + constraints: Vec<(Vec, Vec, VarMapping)>, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])] } + ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![], HashMap::new())] } } pub fn is_top_level(&self) -> bool { self.constraints.len() == 1 } + pub fn var_mappings(&self) -> VarMapping { + let last = self.constraints.last().expect("Can never be empty"); + last.2.clone() + } + + /// Insert variable for mapping in current constraint set. + /// + /// This prevents shadowed variables from contaminating previous constraints. + /// + /// Differs from environment since environment is used to check that variables are defined at a + /// certain location. + pub fn insert_var(&mut self, var: &str) { + for constraint in &mut self.constraints { + let mapping = constraint.2.clone(); + if let Some((var, offset)) = mapping.get(var) { + constraint.2.insert(String::from(var), (var.clone(), offset + 1)); + } + } + } + pub fn new_set_in_class(&mut self, class: &StringName) -> usize { let lvl = self.new_set(); for constraints in &mut self.constraints { @@ -69,7 +92,8 @@ impl ConstrBuilder { for i in (level..self.constraints.len()).rev() { // Equivalent to pop, but remove has better panic message for debugging - self.finished.push(self.constraints.remove(i)) + let (class_names, constraints, _) = self.constraints.remove(i); + self.finished.push((class_names, constraints)) } Ok(()) @@ -98,7 +122,9 @@ impl ConstrBuilder { } pub fn all_constr(self) -> Vec { - let (mut finished, mut constraints) = (self.finished, self.constraints); + let (mut finished, constraints) = (self.finished, self.constraints); + let mut constraints = constraints.into_iter().map(|(n, n1, _)| (n, n1)).collect(); + finished.append(&mut constraints); finished.iter().map(Constraints::from).collect() } diff --git a/src/check/constrain/constraint/expected.rs b/src/check/constrain/constraint/expected.rs index 2b9e52803..75504c0c3 100644 --- a/src/check/constrain/constraint/expected.rs +++ b/src/check/constrain/constraint/expected.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::convert::TryFrom; use std::fmt; use std::fmt::{Display, Error, Formatter}; @@ -7,6 +6,7 @@ use std::ops::Deref; use itertools::{EitherOrBoth, Itertools}; +use crate::check::constrain::constraint::builder::VarMapping; use crate::check::constrain::constraint::expected::Expect::*; use crate::check::context::clss::NONE; use crate::check::name::{Any, Name, Nullable}; @@ -40,13 +40,13 @@ impl AsRef for Expected { } } -impl TryFrom<(&AST, &HashMap)> for Expected { +impl TryFrom<(&AST, &VarMapping)> for Expected { type Error = Vec; /// Creates Expected from AST. /// /// If primitive or Constructor, constructs Type. - fn try_from((ast, mappings): (&AST, &HashMap)) -> TypeResult { + fn try_from((ast, mappings): (&AST, &VarMapping)) -> TypeResult { let ast = match &ast.node { Node::Block { statements } => statements.last().unwrap_or(ast), _ => ast, @@ -56,10 +56,10 @@ impl TryFrom<(&AST, &HashMap)> for Expected { } } -impl TryFrom<(&Box, &HashMap)> for Expected { +impl TryFrom<(&Box, &VarMapping)> for Expected { type Error = Vec; - fn try_from((ast, mappings): (&Box, &HashMap)) -> TypeResult { + fn try_from((ast, mappings): (&Box, &VarMapping)) -> TypeResult { Expected::try_from((ast.deref(), mappings)) } } @@ -79,18 +79,18 @@ impl Any for Expect { fn any() -> Self { Type { name: Name::any() } } } -impl TryFrom<(&AST, &HashMap)> for Expect { +impl TryFrom<(&AST, &VarMapping)> for Expect { type Error = Vec; /// Also substitutes any identifiers with new ones from the environment if the environment /// has a mapping. /// This means that we forget about shadowed variables and continue with the new ones. - fn try_from((ast, mappings): (&AST, &HashMap)) -> TypeResult { + fn try_from((ast, mappings): (&AST, &VarMapping)) -> TypeResult { let ast = ast.map(&|node: &Node| { if let Node::Id { lit } = node { - if let Some(name) = mappings.get(lit) { + if let Some((var, offset)) = mappings.get(lit) { // Always use name currently defined in environment - Node::Id { lit: name.clone() } + Node::Id { lit: format!("{var}&{offset}") } } else { node.clone() } diff --git a/src/check/constrain/generate/call.rs b/src/check/constrain/generate/call.rs index 3d8b267ce..d1f6571c9 100644 --- a/src/check/constrain/generate/call.rs +++ b/src/check/constrain/generate/call.rs @@ -32,7 +32,7 @@ pub fn gen_call( match &ast.node { Node::Reassign { left, right, op } => { let identifier = check_reassignable(left)?; - check_iden_mut(&identifier, env, left.pos)?; + check_iden_mut(&identifier, env, constr, left.pos)?; if let NodeOp::Assign = op { let env_assigned_to: Environment = identifier @@ -47,8 +47,8 @@ pub fn gen_call( constr.add( "reassign", - &Expected::try_from((left, &env.var_mappings))?, - &Expected::try_from((right, &env.var_mappings))?, + &Expected::try_from((left, &constr.var_mappings()))?, + &Expected::try_from((right, &constr.var_mappings()))?, ); generate(right, &env_assigned_to, ctx, constr)?; generate(left, &env_assigned_to, ctx, constr)?; @@ -64,7 +64,7 @@ pub fn gen_call( Ok(if f_name == StringName::from(function::PRINT) { args .iter() - .map(|arg| Expected::try_from((arg, &env.var_mappings))) + .map(|arg| Expected::try_from((arg, &constr.var_mappings()))) .collect::>>()? .iter() .map(|exp| Constraint::stringy("print", exp)) @@ -72,9 +72,9 @@ pub fn gen_call( let name = Name::empty(); let parent = Expected::new(ast.pos, &Type { name }); - constr.add("print", &parent, &Expected::try_from((ast, &env.var_mappings))?); + constr.add("print", &parent, &Expected::try_from((ast, &constr.var_mappings()))?); env.clone() - } else if let Some(functions) = env.get_var(&f_name.name) { + } else if let Some(functions) = env.get_var(&f_name.name, &constr.var_mappings()) { if !f_name.generics.is_empty() { let msg = "Anonymous function call cannot have generics"; return Err(vec![TypeErr::new(name.pos, msg)]); @@ -84,7 +84,7 @@ pub fn gen_call( let last_pos = args.last().map_or_else(|| name.pos, |a| a.pos); let args = args .iter() - .map(|a| Expected::try_from((a, &env.var_mappings))) + .map(|a| Expected::try_from((a, &constr.var_mappings()))) .collect::>()?; let right = Expected::new(last_pos, &Function { name: f_name.clone(), args }); constr.add("function call", &right, &fun_exp); @@ -98,7 +98,7 @@ pub fn gen_call( // entire AST is either fun ret ty or statement constr.add( "function call", - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &fun_ret_exp, ); @@ -116,7 +116,7 @@ pub fn gen_call( constr.add( "index range", &Expected::new(range.pos, &Type { name }), - &Expected::try_from((range, &env.var_mappings))?, + &Expected::try_from((range, &constr.var_mappings()))?, ); let (temp_type, env) = env.temp_var(); @@ -126,14 +126,14 @@ pub fn gen_call( let exp_col = Expected::new(ast.pos, &exp_col); constr.add("type of indexed collection", &exp_col, - &Expected::try_from((item, &env.var_mappings))?, + &Expected::try_from((item, &constr.var_mappings()))?, ); // Must be after above constraint constr.add( "index of collection", &Expected::new(ast.pos, &temp_collection_type), - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, ); generate(item, &env, ctx, constr)?; @@ -144,11 +144,11 @@ pub fn gen_call( } } -fn check_iden_mut(id: &Identifier, env: &Environment, pos: Position) -> TypeResult<()> { +fn check_iden_mut(id: &Identifier, env: &Environment, constr: &mut ConstrBuilder, pos: Position) -> TypeResult<()> { let errors: Vec = id .fields(pos)? .iter() - .flat_map(|(f_mut, var)| match env.get_var(var) { + .flat_map(|(f_mut, var)| match env.get_var(var, &constr.var_mappings()) { Some(exps) if *f_mut => exps .iter() .filter(|(is_mut, _)| !*is_mut) @@ -240,7 +240,7 @@ fn property_call( let args = vec![last_inst.clone()] .iter() .chain(args) - .map(|ast| Expected::try_from((ast, &env.var_mappings))) + .map(|ast| Expected::try_from((ast, &constr.var_mappings()))) .collect::>()?; let function = Function { name: StringName::try_from(name)?, args }; @@ -254,7 +254,7 @@ fn property_call( let (instance, property) = (Box::from(ast.clone()), Box::from(acc)); AST::new(ast.pos, Node::PropertyCall { instance, property }) }); - let entire_call_as_ast = Expected::try_from((&entire_call_as_ast, &env.var_mappings))?; + let entire_call_as_ast = Expected::try_from((&entire_call_as_ast, &constr.var_mappings()))?; let ast_without_access = match instance.len().cmp(&1) { Ordering::Less => { @@ -273,7 +273,7 @@ fn property_call( let access = Expected::new( ast_without_access.pos.union(access.pos), &Access { - entity: Box::new(Expected::try_from((&ast_without_access, &env.var_mappings))?), + entity: Box::new(Expected::try_from((&ast_without_access, &constr.var_mappings()))?), name: Box::new(access), }, ); diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 11f9bd2b7..74377039b 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -57,7 +57,7 @@ pub fn constrain_class_body( constr.add( "class body", - &Expected::try_from((&AST { pos: ty.pos, node: Node::new_self() }, &env_with_class_fields.var_mappings))?, + &Expected::try_from((&AST { pos: ty.pos, node: Node::new_self() }, &constr.var_mappings()))?, &Expected::new(ty.pos, &class_ty_exp), ); @@ -78,10 +78,11 @@ pub fn property_from_field( instance: Box::new(AST { pos, node: Node::new_self() }), property: Box::new(AST { pos, node: Node::Id { lit: field.name.clone() } }), }; - let property_call = Expected::try_from((&AST::new(pos, node), &env.var_mappings))?; + let property_call = Expected::try_from((&AST::new(pos, node), &constr.var_mappings()))?; let field_ty = Expected::new(pos, &Type { name: field.ty.clone() }); - let env = env.insert_var(field.mutable, &field.name, &field_ty); + constr.insert_var(&field.name); + let env = env.insert_var(field.mutable, &field.name, &field_ty, &constr.var_mappings()); constr.add("class field type", &field_ty, &property_call); let access = Expected::new(pos, &Access { diff --git a/src/check/constrain/generate/collection.rs b/src/check/constrain/generate/collection.rs index b73a420fc..0501b6406 100644 --- a/src/check/constrain/generate/collection.rs +++ b/src/check/constrain/generate/collection.rs @@ -21,12 +21,12 @@ pub fn gen_coll( match &ast.node { Node::Set { elements } | Node::List { elements } => { let res = gen_vec(elements, env, false, ctx, constr)?; - constr_col(ast, env, constr, None)?; + constr_col(ast, constr, None)?; Ok(res) } Node::Tuple { elements } => { let res = gen_vec(elements, env, env.is_def_mode, ctx, constr)?; - constr_col(ast, env, constr, None)?; + constr_col(ast, constr, None)?; Ok(res) } @@ -38,16 +38,16 @@ pub fn gen_coll( return Err(vec![TypeErr::new(cond.pos, &msg)]); }; - let item = Expected::try_from((left, &env.var_mappings))?; + let item = Expected::try_from((left, &constr.var_mappings()))?; let col_exp = Expected::new(right.pos, &Collection { ty: Box::new(item) }); - let cond_exp = Expected::try_from((right, &env.var_mappings))?; + let cond_exp = Expected::try_from((right, &constr.var_mappings()))?; constr.add("comprehension collection type", &col_exp, &cond_exp); generate(cond, &conds_env.is_def_mode(false), ctx, constr)?; if let Some(conditions) = conditions.strip_prefix(&[cond.clone()]) { for cond in conditions { generate(cond, &conds_env.is_def_mode(false), ctx, constr)?; - let cond = Expected::try_from((cond, &conds_env.var_mappings))?; + let cond = Expected::try_from((cond, &constr.var_mappings()))?; constr.add_constr(&Constraint::truthy("comprehension condition", &cond)); } } @@ -67,7 +67,6 @@ pub fn gen_coll( /// The assumption here being that every element in the set has the same type. pub fn constr_col( collection: &AST, - env: &Environment, constr: &mut ConstrBuilder, temp_type: Option, ) -> TypeResult<()> { @@ -75,11 +74,11 @@ pub fn constr_col( Node::Set { elements } | Node::List { elements } => { let ty = if let Some(first) = elements.first() { for element in elements { - let parent = Expected::try_from((first, &env.var_mappings))?; - let child = Expected::try_from((element, &env.var_mappings))?; + let parent = Expected::try_from((first, &constr.var_mappings()))?; + let child = Expected::try_from((element, &constr.var_mappings()))?; constr.add("collection item", &parent, &child) } - Box::from(Expected::new(first.pos, &Expect::try_from((first, &env.var_mappings))?)) + Box::from(Expected::new(first.pos, &Expect::try_from((first, &constr.var_mappings()))?)) } else { Box::from(Expected::new(collection.pos, &Expect::any())) }; @@ -87,7 +86,7 @@ pub fn constr_col( ("collection", Collection { ty }) } Node::Tuple { elements } => { - let map = |ast: &AST| Expected::try_from((ast, &env.var_mappings)); + let map = |ast: &AST| Expected::try_from((ast, &constr.var_mappings())); let elements = elements.iter().map(map).collect::>()?; ("tuple", Tuple { elements }) } @@ -100,7 +99,7 @@ pub fn constr_col( }; let col_exp = Expected::new(collection.pos, &col); - constr.add(msg, &col_exp, &Expected::try_from((collection, &env.var_mappings))?); + constr.add(msg, &col_exp, &Expected::try_from((collection, &constr.var_mappings()))?); Ok(()) } @@ -116,14 +115,15 @@ pub fn gen_collection_lookup( let mut env = env.clone(); // Make col constraint before inserting environment, in case shadowed here - let col_exp = Expected::try_from((col, &env.var_mappings))?; + let col_exp = Expected::try_from((col, &constr.var_mappings()))?; for (mutable, var) in Identifier::try_from(lookup)?.fields(lookup.pos)? { - env = env.insert_var(mutable, &var, &Expected::new(lookup.pos, &Expect::any())); + constr.insert_var(&var); + env = env.insert_var(mutable, &var, &Expected::new(lookup.pos, &Expect::any()), &constr.var_mappings()); } let col_ty_exp = Expected::new( col.pos, - &Collection { ty: Box::from(Expected::try_from((lookup, &env.var_mappings))?) }, + &Collection { ty: Box::from(Expected::try_from((lookup, &constr.var_mappings()))?) }, ); constr.add("collection lookup", &col_ty_exp, &col_exp); diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index f4c6c34cf..21f509851 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -45,15 +45,15 @@ pub fn gen_flow( } Node::IfElse { cond, then, el: Some(el) } => { - let left = Expected::try_from((cond, &env.var_mappings))?; + let left = Expected::try_from((cond, &constr.var_mappings()))?; constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; - let if_expr_exp = Expected::try_from((ast, &env.var_mappings))?; + let if_expr_exp = Expected::try_from((ast, &constr.var_mappings()))?; constr.new_set(); let then_env = generate(then, env, ctx, constr)?; - let then_exp = Expected::try_from((then, &then_env.var_mappings))?; + let then_exp = Expected::try_from((then, &constr.var_mappings()))?; if env.is_expr { constr.add("then branch equal to if", &then_exp, &if_expr_exp); } @@ -61,14 +61,14 @@ pub fn gen_flow( constr.new_set(); let else_env = generate(el, env, ctx, constr)?; if env.is_expr { - let el = Expected::try_from((el, &else_env.var_mappings))?; + let el = Expected::try_from((el, &constr.var_mappings()))?; constr.add("else branch equal to if", &el, &then_exp); } Ok(env.union(&then_env.intersect(&else_env))) } Node::IfElse { cond, then, .. } => { - let left = Expected::try_from((cond, &env.var_mappings))?; + let left = Expected::try_from((cond, &constr.var_mappings()))?; constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; @@ -97,7 +97,7 @@ pub fn gen_flow( } Node::While { cond, body } => { let while_lvl = constr.new_set(); - let cond_exp = Expected::try_from((cond, &env.var_mappings))?; + let cond_exp = Expected::try_from((cond, &constr.var_mappings()))?; constr.add_constr(&Constraint::truthy("while condition", &cond_exp)); generate(cond, env, ctx, constr)?; @@ -115,7 +115,7 @@ pub fn gen_flow( fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Environment, ctx: &Context, constr: &mut ConstrBuilder) -> Constrained<()> { let is_define_mode = env.is_def_mode; - let exp_ast = Expected::try_from((ast, &env.var_mappings))?; + let exp_ast = Expected::try_from((ast, &constr.var_mappings()))?; for case in cases { match &case.node { @@ -128,13 +128,13 @@ fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Enviro if let Node::ExpressionType { expr: ref cond, .. } = cond.node { if let Some(expr) = expr { constr.add("match expression and arm condition", - &Expected::try_from((expr, &env.var_mappings))?, - &Expected::try_from((cond, &env.var_mappings))?, + &Expected::try_from((expr, &constr.var_mappings()))?, + &Expected::try_from((cond, &constr.var_mappings()))?, ); } } - let exp_body = Expected::try_from((body, &cond_env.var_mappings))?; + let exp_body = Expected::try_from((body, &constr.var_mappings()))?; constr.add("match arm body", &exp_body, &exp_ast); } _ => return Err(vec![TypeErr::new(case.pos, "Expected case")]) @@ -142,21 +142,3 @@ fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Enviro } Ok(()) } - -#[cfg(test)] -mod tests { - use crate::check::constrain::constraint::builder::ConstrBuilder; - use crate::check::constrain::generate::env::Environment; - use crate::check::constrain::generate::generate; - use crate::check::context::Context; - use crate::parse::parse; - - #[test] - fn if_else_env_empty() { - let src = "if True then 10 else 20"; - let ast = parse(src).unwrap(); - let env = generate(&ast, &Environment::default(), &Context::default(), &mut ConstrBuilder::new()).unwrap(); - - assert!(env.var_mappings.is_empty()); - } -} diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 63195d702..11d415778 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -77,7 +77,7 @@ pub fn gen_def( if let Some(ret_ty) = ret_ty { let name = Name::try_from(ret_ty)?; let ret_ty_raises_exp = Expected::new(body.pos, &Type { name: name.clone() }); - constr.add("fun body type", &ret_ty_raises_exp, &Expected::try_from((body, &body_env.var_mappings))?); + constr.add("fun body type", &ret_ty_raises_exp, &Expected::try_from((body, &constr.var_mappings()))?); let ret_ty_exp = Expected::new(ret_ty.pos, &Type { name }); let body_env = body_env.return_type(&ret_ty_exp).is_expr(true); @@ -135,8 +135,8 @@ pub fn constrain_args( } let self_exp = Expected::new(var.pos, self_type); - env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp); - let left = Expected::try_from((var, &env_with_args.var_mappings))?; + env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mappings()); + let left = Expected::try_from((var, &constr.var_mappings()))?; constr.add("arguments", &left, &Expected::new(var.pos, self_type)); } else { env_with_args = identifier_from_var(var, ty, default, *mutable, ctx, constr, &env_with_args)?; @@ -168,38 +168,40 @@ pub fn identifier_from_var( let name = Name::try_from(ty.deref())?; for (f_name, (f_mut, name)) in match_name(&identifier, &name, var.pos)? { let ty = Expected::new(var.pos, &Type { name: name.clone() }); - env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &ty); + constr.insert_var(&f_name); + env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &ty, &constr.var_mappings()); } } else { let any = Expected::new(var.pos, &Expect::any()); for (f_mut, f_name) in identifier.fields(var.pos)? { - env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &any); + constr.insert_var(&f_name); + env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &any, &constr.var_mappings()); } }; if identifier.is_tuple() { - let tup_exps = identifier_to_tuple(var.pos, &identifier, &env_with_var)?; + let tup_exps = identifier_to_tuple(var.pos, &identifier, &env_with_var, constr)?; if let Some(ty) = ty { - let ty_exp = Expected::try_from((ty, &env_with_var.var_mappings))?; + let ty_exp = Expected::try_from((ty, &constr.var_mappings()))?; for tup_exp in &tup_exps { constr.add("type and tuple", &ty_exp, tup_exp); } } if let Some(expr) = expr { - let expr_expt = Expected::try_from((expr, &env_with_var.var_mappings))?; + let expr_expt = Expected::try_from((expr, &constr.var_mappings()))?; for tup_exp in &tup_exps { constr.add("tuple and expression", &expr_expt, tup_exp); } } } - let var_expect = Expected::try_from((var, &env_with_var.var_mappings))?; + let var_expect = Expected::try_from((var, &constr.var_mappings()))?; if let Some(ty) = ty { let ty_exp = Expected::new(var.pos, &Type { name: Name::try_from(ty.deref())? }); constr.add("variable and type", &ty_exp, &var_expect); } if let Some(expr) = expr { - let expr_expect = Expected::try_from((expr, &env_with_var.var_mappings))?; + let expr_expect = Expected::try_from((expr, &constr.var_mappings()))?; let msg = format!("variable and expression: `{}`", expr.node); constr.add(&msg, &var_expect, &expr_expect); } @@ -214,10 +216,11 @@ fn identifier_to_tuple( pos: Position, iden: &Identifier, env: &Environment, + constr: &mut ConstrBuilder, ) -> TypeResult> { match &iden { Identifier::Single(_, var) => { - if let Some(expected) = env.get_var(&var.object(pos)?) { + if let Some(expected) = env.get_var(&var.object(pos)?, &constr.var_mappings()) { Ok(expected.iter().map(|(_, exp)| exp.clone()).collect()) } else { let msg = format!("'{iden}' is undefined in this scope"); @@ -227,7 +230,7 @@ fn identifier_to_tuple( Identifier::Multi(idens) => { // Every item in the tuple is a union of expected let tuple_unions: Vec> = - idens.iter().map(|i| identifier_to_tuple(pos, i, env)).collect::>()?; + idens.iter().map(|i| identifier_to_tuple(pos, i, env, constr)).collect::>()?; // .. So we create permutation of every possible tuple combination let tuple_unions: Vec> = diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index fa6a54bf7..9faf631c8 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; +use crate::check::constrain::constraint::builder::VarMapping; use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::context::arg::SELF; use crate::check::name; @@ -16,7 +17,6 @@ pub struct Environment { pub raises_caught: HashSet, pub class_type: Option, - pub var_mappings: HashMap, pub unassigned: HashSet, temp_type: usize, vars: HashMap>, @@ -28,7 +28,7 @@ impl Environment { /// This adds a self variable with the class expected, and class_type is set /// to the expected class type. pub fn in_class(&self, class: &Expected) -> Environment { - let env = self.insert_var(false, &String::from(SELF), class); + let env = self.insert_var(false, &String::from(SELF), class, &HashMap::new()); Environment { class_type: Some(class.expect.clone()), ..env } } @@ -51,27 +51,18 @@ impl Environment { /// /// If the var was previously defined, it is renamed, and the rename mapping is stored. /// In future, if we get a variable, if it was renamed, the mapping is returned instead. - pub fn insert_var(&self, mutable: bool, var: &str, expect: &Expected) -> Environment { + pub fn insert_var(&self, mutable: bool, var: &str, expect: &Expected, var_mappings: &VarMapping) -> Environment { let expected_set = vec![(mutable, expect.clone())].into_iter().collect::>(); - let (mut vars, mut var_mappings) = (self.vars.clone(), self.var_mappings.clone()); - - // Never shadow self - let var = if vars.contains_key(var) && var != SELF { - let mut offset = 0; - let mut new_var = format!("{}@{}", var, offset); - while self.vars.contains_key(&new_var) { - offset += 1; - new_var = format!("{var}@{offset}"); - } + let mut vars = self.vars.clone(); - var_mappings.insert(String::from(var), new_var.clone()); - new_var + let var = if let Some((var, offset)) = var_mappings.get(var) { + format!("{var}@{offset}") } else { String::from(var) }; - vars.insert(var, expected_set); - Environment { vars, var_mappings, ..self.clone() } + vars.insert(var.clone(), expected_set); + Environment { vars, ..self.clone() } } /// Insert raises which are properly handled. @@ -104,14 +95,13 @@ impl Environment { /// This is useful for detecting shadowing. /// /// Return true variable [TrueName], whether it's mutable and it's expected value - pub fn get_var(&self, var: &str) -> Option> { - for (old, new) in &self.var_mappings { - if old == var { - return self.get_var(new); - } - } - - self.vars.get(var).cloned() + pub fn get_var(&self, var: &str, var_mappings: &VarMapping) -> Option> { + let var_name = if let Some((var, offset)) = var_mappings.get(var) { + format!("{var}@{offset}") + } else { + String::from(var) + }; + self.vars.get(&var_name).cloned() } /// Union between two environments @@ -131,12 +121,7 @@ impl Environment { } } - let mut var_mappings = self.var_mappings.clone(); - for (key, value) in &other.var_mappings { - var_mappings.insert(key.clone(), value.clone()); - } - - Environment { vars, var_mappings, ..self.clone() } + Environment { vars, ..self.clone() } } /// Intersection between two environments. @@ -166,19 +151,7 @@ impl Environment { } } - let to_remove: Vec = self - .var_mappings - .iter() - .filter(|(key, _)| other.var_mappings.contains_key(*key)) - .map(|(key, _)| key.clone()) - .collect(); - - let mut var_mappings = self.var_mappings.clone(); - for key in &to_remove { - var_mappings.remove(key); - } - - Environment { vars, var_mappings, ..self.clone() } + Environment { vars, ..self.clone() } } /// Get a name for a temporary type. diff --git a/src/check/constrain/generate/expression.rs b/src/check/constrain/generate/expression.rs index ea470d64d..0c875148b 100644 --- a/src/check/constrain/generate/expression.rs +++ b/src/check/constrain/generate/expression.rs @@ -27,7 +27,7 @@ pub fn gen_expr( Node::Question { left, right } => { constr.add( "question", - &Expected::try_from((left, &env.var_mappings))?, + &Expected::try_from((left, &constr.var_mappings()))?, &Expected::new(left.pos, &Expect::none()), ); @@ -50,7 +50,7 @@ fn match_id(ast: &AST, ty: &OptAST, mutable: bool, env: &Environment, ctx: &Cont match &ast.node { Node::Id { lit } => if env.is_def_mode { identifier_from_var(ast, ty, &None, mutable, ctx, constr, env) - } else if env.get_var(lit).is_some() { + } else if env.get_var(lit, &constr.var_mappings()).is_some() { Ok(env.clone()) } else { Err(vec![TypeErr::new(ast.pos, &format!("Undefined variable: {lit}"))]) diff --git a/src/check/constrain/generate/operation.rs b/src/check/constrain/generate/operation.rs index 994817fe5..15313055b 100644 --- a/src/check/constrain/generate/operation.rs +++ b/src/check/constrain/generate/operation.rs @@ -24,7 +24,7 @@ pub fn gen_op( ) -> Constrained { match &ast.node { Node::In { left, right } => { - constr_col(right, env, constr, None)?; + constr_col(right, constr, None)?; gen_collection_lookup(left, right, env, constr)?; generate(right, env, ctx, constr)?; @@ -46,18 +46,18 @@ pub fn gen_op( Node::Str { expressions, .. } => { gen_vec(expressions, env, false, ctx, constr)?; for expr in expressions { - let c = Constraint::stringy("string", &Expected::try_from((expr, &env.var_mappings))?); + let c = Constraint::stringy("string", &Expected::try_from((expr, &constr.var_mappings()))?); constr.add_constr(&c); } primitive(ast, STRING, env, constr) } Node::Bool { .. } => { - let truthy = Constraint::truthy("bool", &Expected::try_from((ast, &env.var_mappings))?); + let truthy = Constraint::truthy("bool", &Expected::try_from((ast, &constr.var_mappings()))?); constr.add_constr(&truthy); primitive(ast, BOOL, env, constr) } Node::Undefined => { - let undef = Constraint::undefined("undefined", &Expected::try_from((ast, &env.var_mappings))?); + let undef = Constraint::undefined("undefined", &Expected::try_from((ast, &constr.var_mappings()))?); constr.add_constr(&undef); Ok(env.clone()) } @@ -82,49 +82,49 @@ pub fn gen_op( let ty = Type { name: Name::from(FLOAT) }; constr.add( "square root", - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &Expected::new(ast.pos, &ty), ); let access = Expected::new( expr.pos, &Access { - entity: Box::new(Expected::try_from((expr, &env.var_mappings))?), + entity: Box::new(Expected::try_from((expr, &constr.var_mappings()))?), name: Box::from(Expected::new( expr.pos, &Function { name: StringName::from(SQRT), - args: vec![Expected::try_from((expr, &env.var_mappings))?], + args: vec![Expected::try_from((expr, &constr.var_mappings()))?], }, )), }, ); - constr.add("square root", &Expected::try_from((ast, &env.var_mappings))?, &access); + constr.add("square root", &Expected::try_from((ast, &constr.var_mappings()))?, &access); generate(expr, env, ctx, constr) } Node::BOneCmpl { expr } => { - let left = Expected::try_from((expr, &env.var_mappings))?; + let left = Expected::try_from((expr, &constr.var_mappings()))?; constr.add("binary compliment", &left, &Expected::new(expr.pos, &Expect::any())); generate(expr, env, ctx, constr)?; Ok(env.clone()) } Node::BAnd { left, right } | Node::BOr { left, right } | Node::BXOr { left, right } => { - let l_exp = Expected::try_from((left, &env.var_mappings))?; + let l_exp = Expected::try_from((left, &constr.var_mappings()))?; constr.add("binary logical op", &l_exp, &Expected::new(left.pos, &Expect::any())); - let l_exp = Expected::try_from((right, &env.var_mappings))?; + let l_exp = Expected::try_from((right, &constr.var_mappings()))?; constr.add("binary logical op", &l_exp, &Expected::new(right.pos, &Expect::any())); bin_op(left, right, env, ctx, constr) } Node::BLShift { left, right } | Node::BRShift { left, right } => { - let l_exp = Expected::try_from((left, &env.var_mappings))?; + let l_exp = Expected::try_from((left, &constr.var_mappings()))?; constr.add("binary shift", &l_exp, &Expected::new(right.pos, &Expect::any())); let name = Name::from(INT); - let l_exp = Expected::try_from((right, &env.var_mappings))?; + let l_exp = Expected::try_from((right, &constr.var_mappings()))?; constr.add("binary shift", &l_exp, &Expected::new(right.pos, &Type { name })); bin_op(left, right, env, ctx, constr) @@ -132,7 +132,7 @@ pub fn gen_op( Node::Is { left, right } | Node::IsN { left, right } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); bin_op(left, right, env, ctx, constr) } Node::IsA { left, right } | Node::IsNA { left, right } => if let Node::Id { .. } = right.node { @@ -149,25 +149,25 @@ pub fn gen_op( Node::Not { expr } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); constr.add_constr(&Constraint::truthy( "not", - &Expected::try_from((expr, &env.var_mappings))?, + &Expected::try_from((expr, &constr.var_mappings()))?, )); generate(expr, env, ctx, constr)?; Ok(env.clone()) } Node::And { left, right } | Node::Or { left, right } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &env.var_mappings))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); constr.add_constr(&Constraint::truthy( "and", - &Expected::try_from((left, &env.var_mappings))?, + &Expected::try_from((left, &constr.var_mappings()))?, )); constr.add_constr(&Constraint::truthy( "and", - &Expected::try_from((right, &env.var_mappings))?, + &Expected::try_from((right, &constr.var_mappings()))?, )); bin_op(left, right, env, ctx, constr) @@ -199,25 +199,25 @@ pub fn constr_range( constr.add( &format!("{range_slice} from"), - &Expected::try_from((from, &env.var_mappings))?, + &Expected::try_from((from, &constr.var_mappings()))?, int_exp, ); constr.add( &format!("{range_slice} to"), - &Expected::try_from((to, &env.var_mappings))?, + &Expected::try_from((to, &constr.var_mappings()))?, int_exp, ); if let Some(step) = step { constr.add( &format!("{range_slice} step"), - &Expected::try_from((step, &env.var_mappings))?, + &Expected::try_from((step, &constr.var_mappings()))?, int_exp, ); } if contr_coll { let col = Expected::new(ast.pos, &Collection { ty: Box::from(int_exp.clone()) }); - constr.add("range collection", &col, &Expected::try_from((ast, &env.var_mappings))?); + constr.add("range collection", &col, &Expected::try_from((ast, &constr.var_mappings()))?); } generate(from, env, ctx, constr)?; @@ -230,7 +230,7 @@ fn primitive(ast: &AST, ty: &str, env: &Environment, constr: &mut ConstrBuilder) let name = Name::from(ty); constr.add( format!("{ty} primitive").as_str(), - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &Expected::new(ast.pos, &Type { name }), ); Ok(env.clone()) @@ -247,18 +247,18 @@ fn impl_magic( ) -> Constrained { constr.add( format!("{fun} operation").as_str(), - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &Expected::new( left.pos, &Access { - entity: Box::new(Expected::try_from((left, &env.var_mappings))?), + entity: Box::new(Expected::try_from((left, &constr.var_mappings()))?), name: Box::new(Expected::new( left.pos, &Function { name: StringName::from(fun), args: vec![ - Expected::try_from((left, &env.var_mappings))?, - Expected::try_from((right, &env.var_mappings))?, + Expected::try_from((left, &constr.var_mappings()))?, + Expected::try_from((right, &constr.var_mappings()))?, ], }, )), @@ -281,18 +281,18 @@ fn impl_bool_op( if fun != EQ && fun != NEQ { constr.add( "bool operation", - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &Expected::new( left.pos, &Access { - entity: Box::new(Expected::try_from((left, &env.var_mappings))?), + entity: Box::new(Expected::try_from((left, &constr.var_mappings()))?), name: Box::new(Expected::new( left.pos, &Function { name: StringName::from(fun), args: vec![ - Expected::try_from((left, &env.var_mappings))?, - Expected::try_from((right, &env.var_mappings))?, + Expected::try_from((left, &constr.var_mappings()))?, + Expected::try_from((right, &constr.var_mappings()))?, ], }, )), @@ -304,7 +304,7 @@ fn impl_bool_op( let ty = Type { name: Name::from(BOOL) }; constr.add( "bool operation", - &Expected::try_from((ast, &env.var_mappings))?, + &Expected::try_from((ast, &constr.var_mappings()))?, &Expected::new(ast.pos, &ty), ); diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index e3e43b2d1..59c864996 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -20,8 +20,8 @@ pub fn gen_resources( match &ast.node { Node::With { resource, alias: Some((alias, mutable, ty)), expr } => { let with_lvl = constr.new_set(); - let resource_exp = Expected::try_from((resource, &env.var_mappings))?; - constr.add("with as", &resource_exp, &Expected::try_from((alias, &env.var_mappings))?); + let resource_exp = Expected::try_from((resource, &constr.var_mappings()))?; + constr.add("with as", &resource_exp, &Expected::try_from((alias, &constr.var_mappings()))?); constr.add("with as", &resource_exp, &Expected::new(resource.pos, &Expect::any())); if let Some(ty) = ty { @@ -50,7 +50,7 @@ pub fn gen_resources( let with_lvl = constr.new_set(); constr.add( "with", - &Expected::try_from((resource, &env.var_mappings))?, + &Expected::try_from((resource, &constr.var_mappings()))?, &Expected::new(resource.pos, &Expect::any()), ); diff --git a/src/check/constrain/generate/statement.rs b/src/check/constrain/generate/statement.rs index 2125329fa..412a065c1 100644 --- a/src/check/constrain/generate/statement.rs +++ b/src/check/constrain/generate/statement.rs @@ -44,7 +44,7 @@ pub fn gen_stmt( constr.add( "return", expected_ret_ty, - &Expected::try_from((expr, &env.var_mappings))?, + &Expected::try_from((expr, &constr.var_mappings()))?, ); Ok(env.clone()) } else if !env.in_fun { From dde7108b9e7a0050664fb1791d9aee824b442340 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 17:30:56 +0100 Subject: [PATCH 10/22] [ci skip] Make variable mapping top-level If mapping of new var non-existent, add it to mapping. --- src/check/constrain/constraint/builder.rs | 36 +++++++++++++--------- src/check/constrain/constraint/expected.rs | 4 +-- src/check/constrain/generate/definition.rs | 1 + src/check/constrain/generate/env.rs | 6 ++-- 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index e2901ae36..4683ea0b6 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -14,6 +14,14 @@ const PADDING: usize = 2; pub type VarMapping = HashMap; +pub fn format_var_map(var: &str, offset: &usize) -> String { + if *offset == 0 as usize { + String::from(var) + } else { + format!("{var}@{offset}") + } +} + /// Constraint Builder. /// /// Allows us to build sets of constraints. @@ -30,19 +38,19 @@ pub type VarMapping = HashMap; #[derive(Debug)] pub struct ConstrBuilder { finished: Vec<(Vec, Vec)>, - constraints: Vec<(Vec, Vec, VarMapping)>, + constraints: Vec<(Vec, Vec)>, + mapping: VarMapping, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![], HashMap::new())] } + ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])], mapping: HashMap::new() } } pub fn is_top_level(&self) -> bool { self.constraints.len() == 1 } pub fn var_mappings(&self) -> VarMapping { - let last = self.constraints.last().expect("Can never be empty"); - last.2.clone() + self.mapping.clone() } /// Insert variable for mapping in current constraint set. @@ -52,12 +60,13 @@ impl ConstrBuilder { /// Differs from environment since environment is used to check that variables are defined at a /// certain location. pub fn insert_var(&mut self, var: &str) { - for constraint in &mut self.constraints { - let mapping = constraint.2.clone(); - if let Some((var, offset)) = mapping.get(var) { - constraint.2.insert(String::from(var), (var.clone(), offset + 1)); - } - } + let offset = if let Some((_, offset)) = self.mapping.get(var) { + offset + 1 + } else { + 0 + }; + + self.mapping.insert(String::from(var), (String::from(var), offset)); } pub fn new_set_in_class(&mut self, class: &StringName) -> usize { @@ -92,8 +101,7 @@ impl ConstrBuilder { for i in (level..self.constraints.len()).rev() { // Equivalent to pop, but remove has better panic message for debugging - let (class_names, constraints, _) = self.constraints.remove(i); - self.finished.push((class_names, constraints)) + self.finished.push(self.constraints.remove(i)) } Ok(()) @@ -122,9 +130,7 @@ impl ConstrBuilder { } pub fn all_constr(self) -> Vec { - let (mut finished, constraints) = (self.finished, self.constraints); - let mut constraints = constraints.into_iter().map(|(n, n1, _)| (n, n1)).collect(); - + let (mut finished, mut constraints) = (self.finished, self.constraints); finished.append(&mut constraints); finished.iter().map(Constraints::from).collect() } diff --git a/src/check/constrain/constraint/expected.rs b/src/check/constrain/constraint/expected.rs index 75504c0c3..a1caba103 100644 --- a/src/check/constrain/constraint/expected.rs +++ b/src/check/constrain/constraint/expected.rs @@ -6,7 +6,7 @@ use std::ops::Deref; use itertools::{EitherOrBoth, Itertools}; -use crate::check::constrain::constraint::builder::VarMapping; +use crate::check::constrain::constraint::builder::{format_var_map, VarMapping}; use crate::check::constrain::constraint::expected::Expect::*; use crate::check::context::clss::NONE; use crate::check::name::{Any, Name, Nullable}; @@ -90,7 +90,7 @@ impl TryFrom<(&AST, &VarMapping)> for Expect { if let Node::Id { lit } = node { if let Some((var, offset)) = mappings.get(lit) { // Always use name currently defined in environment - Node::Id { lit: format!("{var}&{offset}") } + Node::Id { lit: format_var_map(var, offset) } } else { node.clone() } diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 11d415778..bfed7fe3a 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -135,6 +135,7 @@ pub fn constrain_args( } let self_exp = Expected::new(var.pos, self_type); + constr.insert_var(SELF); env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mappings()); let left = Expected::try_from((var, &constr.var_mappings()))?; constr.add("arguments", &left, &Expected::new(var.pos, self_type)); diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index 9faf631c8..06616334b 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -1,6 +1,6 @@ use std::collections::{HashMap, HashSet}; -use crate::check::constrain::constraint::builder::VarMapping; +use crate::check::constrain::constraint::builder::{format_var_map, VarMapping}; use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::context::arg::SELF; use crate::check::name; @@ -56,7 +56,7 @@ impl Environment { let mut vars = self.vars.clone(); let var = if let Some((var, offset)) = var_mappings.get(var) { - format!("{var}@{offset}") + format_var_map(var, offset) } else { String::from(var) }; @@ -97,7 +97,7 @@ impl Environment { /// Return true variable [TrueName], whether it's mutable and it's expected value pub fn get_var(&self, var: &str, var_mappings: &VarMapping) -> Option> { let var_name = if let Some((var, offset)) = var_mappings.get(var) { - format!("{var}@{offset}") + format_var_map(var, offset) } else { String::from(var) }; From 69ce92894c3478c78e09ccc815480c5f7eaaccb9 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 19:14:34 +0100 Subject: [PATCH 11/22] Add local variable mapping to environment This takes precedent, meaning that local shadows are used. If not found, then use global. Useful if we have multiple execution paths and later down the line need to perform substitutions. --- src/check/constrain/constraint/builder.rs | 20 ++---- src/check/constrain/constraint/expected.rs | 4 +- src/check/constrain/generate/call.rs | 28 ++++----- src/check/constrain/generate/class.rs | 6 +- src/check/constrain/generate/collection.rs | 22 +++---- src/check/constrain/generate/control_flow.rs | 20 +++--- src/check/constrain/generate/definition.rs | 20 +++--- src/check/constrain/generate/env.rs | 35 +++++++---- src/check/constrain/generate/expression.rs | 4 +- src/check/constrain/generate/operation.rs | 64 ++++++++++---------- src/check/constrain/generate/resources.rs | 6 +- src/check/constrain/generate/statement.rs | 2 +- 12 files changed, 116 insertions(+), 115 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 4683ea0b6..9b59ec2a6 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -12,7 +12,7 @@ use crate::common::position::Position; const PADDING: usize = 2; -pub type VarMapping = HashMap; +pub type VarMapping = HashMap; pub fn format_var_map(var: &str, offset: &usize) -> String { if *offset == 0 as usize { @@ -39,20 +39,17 @@ pub fn format_var_map(var: &str, offset: &usize) -> String { pub struct ConstrBuilder { finished: Vec<(Vec, Vec)>, constraints: Vec<(Vec, Vec)>, - mapping: VarMapping, + + pub var_mapping: VarMapping, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])], mapping: HashMap::new() } + ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])], var_mapping: HashMap::new() } } pub fn is_top_level(&self) -> bool { self.constraints.len() == 1 } - pub fn var_mappings(&self) -> VarMapping { - self.mapping.clone() - } - /// Insert variable for mapping in current constraint set. /// /// This prevents shadowed variables from contaminating previous constraints. @@ -60,13 +57,8 @@ impl ConstrBuilder { /// Differs from environment since environment is used to check that variables are defined at a /// certain location. pub fn insert_var(&mut self, var: &str) { - let offset = if let Some((_, offset)) = self.mapping.get(var) { - offset + 1 - } else { - 0 - }; - - self.mapping.insert(String::from(var), (String::from(var), offset)); + let offset = self.var_mapping.get(var).map_or(0, |o| o + 1); + self.var_mapping.insert(String::from(var), offset); } pub fn new_set_in_class(&mut self, class: &StringName) -> usize { diff --git a/src/check/constrain/constraint/expected.rs b/src/check/constrain/constraint/expected.rs index a1caba103..e58a7a021 100644 --- a/src/check/constrain/constraint/expected.rs +++ b/src/check/constrain/constraint/expected.rs @@ -88,9 +88,9 @@ impl TryFrom<(&AST, &VarMapping)> for Expect { fn try_from((ast, mappings): (&AST, &VarMapping)) -> TypeResult { let ast = ast.map(&|node: &Node| { if let Node::Id { lit } = node { - if let Some((var, offset)) = mappings.get(lit) { + if let Some(offset) = mappings.get(lit) { // Always use name currently defined in environment - Node::Id { lit: format_var_map(var, offset) } + Node::Id { lit: format_var_map(lit, offset) } } else { node.clone() } diff --git a/src/check/constrain/generate/call.rs b/src/check/constrain/generate/call.rs index d1f6571c9..e3c61b984 100644 --- a/src/check/constrain/generate/call.rs +++ b/src/check/constrain/generate/call.rs @@ -47,8 +47,8 @@ pub fn gen_call( constr.add( "reassign", - &Expected::try_from((left, &constr.var_mappings()))?, - &Expected::try_from((right, &constr.var_mappings()))?, + &Expected::try_from((left, &constr.var_mapping))?, + &Expected::try_from((right, &constr.var_mapping))?, ); generate(right, &env_assigned_to, ctx, constr)?; generate(left, &env_assigned_to, ctx, constr)?; @@ -64,7 +64,7 @@ pub fn gen_call( Ok(if f_name == StringName::from(function::PRINT) { args .iter() - .map(|arg| Expected::try_from((arg, &constr.var_mappings()))) + .map(|arg| Expected::try_from((arg, &constr.var_mapping))) .collect::>>()? .iter() .map(|exp| Constraint::stringy("print", exp)) @@ -72,9 +72,9 @@ pub fn gen_call( let name = Name::empty(); let parent = Expected::new(ast.pos, &Type { name }); - constr.add("print", &parent, &Expected::try_from((ast, &constr.var_mappings()))?); + constr.add("print", &parent, &Expected::try_from((ast, &constr.var_mapping))?); env.clone() - } else if let Some(functions) = env.get_var(&f_name.name, &constr.var_mappings()) { + } else if let Some(functions) = env.get_var(&f_name.name, &constr.var_mapping) { if !f_name.generics.is_empty() { let msg = "Anonymous function call cannot have generics"; return Err(vec![TypeErr::new(name.pos, msg)]); @@ -84,7 +84,7 @@ pub fn gen_call( let last_pos = args.last().map_or_else(|| name.pos, |a| a.pos); let args = args .iter() - .map(|a| Expected::try_from((a, &constr.var_mappings()))) + .map(|a| Expected::try_from((a, &constr.var_mapping))) .collect::>()?; let right = Expected::new(last_pos, &Function { name: f_name.clone(), args }); constr.add("function call", &right, &fun_exp); @@ -98,7 +98,7 @@ pub fn gen_call( // entire AST is either fun ret ty or statement constr.add( "function call", - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &fun_ret_exp, ); @@ -116,7 +116,7 @@ pub fn gen_call( constr.add( "index range", &Expected::new(range.pos, &Type { name }), - &Expected::try_from((range, &constr.var_mappings()))?, + &Expected::try_from((range, &constr.var_mapping))?, ); let (temp_type, env) = env.temp_var(); @@ -126,14 +126,14 @@ pub fn gen_call( let exp_col = Expected::new(ast.pos, &exp_col); constr.add("type of indexed collection", &exp_col, - &Expected::try_from((item, &constr.var_mappings()))?, + &Expected::try_from((item, &constr.var_mapping))?, ); // Must be after above constraint constr.add( "index of collection", &Expected::new(ast.pos, &temp_collection_type), - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, ); generate(item, &env, ctx, constr)?; @@ -148,7 +148,7 @@ fn check_iden_mut(id: &Identifier, env: &Environment, constr: &mut ConstrBuilder let errors: Vec = id .fields(pos)? .iter() - .flat_map(|(f_mut, var)| match env.get_var(var, &constr.var_mappings()) { + .flat_map(|(f_mut, var)| match env.get_var(var, &constr.var_mapping) { Some(exps) if *f_mut => exps .iter() .filter(|(is_mut, _)| !*is_mut) @@ -240,7 +240,7 @@ fn property_call( let args = vec![last_inst.clone()] .iter() .chain(args) - .map(|ast| Expected::try_from((ast, &constr.var_mappings()))) + .map(|ast| Expected::try_from((ast, &constr.var_mapping))) .collect::>()?; let function = Function { name: StringName::try_from(name)?, args }; @@ -254,7 +254,7 @@ fn property_call( let (instance, property) = (Box::from(ast.clone()), Box::from(acc)); AST::new(ast.pos, Node::PropertyCall { instance, property }) }); - let entire_call_as_ast = Expected::try_from((&entire_call_as_ast, &constr.var_mappings()))?; + let entire_call_as_ast = Expected::try_from((&entire_call_as_ast, &constr.var_mapping))?; let ast_without_access = match instance.len().cmp(&1) { Ordering::Less => { @@ -273,7 +273,7 @@ fn property_call( let access = Expected::new( ast_without_access.pos.union(access.pos), &Access { - entity: Box::new(Expected::try_from((&ast_without_access, &constr.var_mappings()))?), + entity: Box::new(Expected::try_from((&ast_without_access, &constr.var_mapping))?), name: Box::new(access), }, ); diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 74377039b..628a94a91 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -57,7 +57,7 @@ pub fn constrain_class_body( constr.add( "class body", - &Expected::try_from((&AST { pos: ty.pos, node: Node::new_self() }, &constr.var_mappings()))?, + &Expected::try_from((&AST { pos: ty.pos, node: Node::new_self() }, &constr.var_mapping))?, &Expected::new(ty.pos, &class_ty_exp), ); @@ -78,11 +78,11 @@ pub fn property_from_field( instance: Box::new(AST { pos, node: Node::new_self() }), property: Box::new(AST { pos, node: Node::Id { lit: field.name.clone() } }), }; - let property_call = Expected::try_from((&AST::new(pos, node), &constr.var_mappings()))?; + let property_call = Expected::try_from((&AST::new(pos, node), &constr.var_mapping))?; let field_ty = Expected::new(pos, &Type { name: field.ty.clone() }); constr.insert_var(&field.name); - let env = env.insert_var(field.mutable, &field.name, &field_ty, &constr.var_mappings()); + let env = env.insert_var(field.mutable, &field.name, &field_ty, &constr.var_mapping); constr.add("class field type", &field_ty, &property_call); let access = Expected::new(pos, &Access { diff --git a/src/check/constrain/generate/collection.rs b/src/check/constrain/generate/collection.rs index 0501b6406..63417ec3e 100644 --- a/src/check/constrain/generate/collection.rs +++ b/src/check/constrain/generate/collection.rs @@ -38,16 +38,16 @@ pub fn gen_coll( return Err(vec![TypeErr::new(cond.pos, &msg)]); }; - let item = Expected::try_from((left, &constr.var_mappings()))?; + let item = Expected::try_from((left, &constr.var_mapping))?; let col_exp = Expected::new(right.pos, &Collection { ty: Box::new(item) }); - let cond_exp = Expected::try_from((right, &constr.var_mappings()))?; + let cond_exp = Expected::try_from((right, &constr.var_mapping))?; constr.add("comprehension collection type", &col_exp, &cond_exp); generate(cond, &conds_env.is_def_mode(false), ctx, constr)?; if let Some(conditions) = conditions.strip_prefix(&[cond.clone()]) { for cond in conditions { generate(cond, &conds_env.is_def_mode(false), ctx, constr)?; - let cond = Expected::try_from((cond, &constr.var_mappings()))?; + let cond = Expected::try_from((cond, &constr.var_mapping))?; constr.add_constr(&Constraint::truthy("comprehension condition", &cond)); } } @@ -74,11 +74,11 @@ pub fn constr_col( Node::Set { elements } | Node::List { elements } => { let ty = if let Some(first) = elements.first() { for element in elements { - let parent = Expected::try_from((first, &constr.var_mappings()))?; - let child = Expected::try_from((element, &constr.var_mappings()))?; + let parent = Expected::try_from((first, &constr.var_mapping))?; + let child = Expected::try_from((element, &constr.var_mapping))?; constr.add("collection item", &parent, &child) } - Box::from(Expected::new(first.pos, &Expect::try_from((first, &constr.var_mappings()))?)) + Box::from(Expected::new(first.pos, &Expect::try_from((first, &constr.var_mapping))?)) } else { Box::from(Expected::new(collection.pos, &Expect::any())) }; @@ -86,7 +86,7 @@ pub fn constr_col( ("collection", Collection { ty }) } Node::Tuple { elements } => { - let map = |ast: &AST| Expected::try_from((ast, &constr.var_mappings())); + let map = |ast: &AST| Expected::try_from((ast, &constr.var_mapping)); let elements = elements.iter().map(map).collect::>()?; ("tuple", Tuple { elements }) } @@ -99,7 +99,7 @@ pub fn constr_col( }; let col_exp = Expected::new(collection.pos, &col); - constr.add(msg, &col_exp, &Expected::try_from((collection, &constr.var_mappings()))?); + constr.add(msg, &col_exp, &Expected::try_from((collection, &constr.var_mapping))?); Ok(()) } @@ -115,15 +115,15 @@ pub fn gen_collection_lookup( let mut env = env.clone(); // Make col constraint before inserting environment, in case shadowed here - let col_exp = Expected::try_from((col, &constr.var_mappings()))?; + let col_exp = Expected::try_from((col, &constr.var_mapping))?; for (mutable, var) in Identifier::try_from(lookup)?.fields(lookup.pos)? { constr.insert_var(&var); - env = env.insert_var(mutable, &var, &Expected::new(lookup.pos, &Expect::any()), &constr.var_mappings()); + env = env.insert_var(mutable, &var, &Expected::new(lookup.pos, &Expect::any()), &constr.var_mapping); } let col_ty_exp = Expected::new( col.pos, - &Collection { ty: Box::from(Expected::try_from((lookup, &constr.var_mappings()))?) }, + &Collection { ty: Box::from(Expected::try_from((lookup, &constr.var_mapping))?) }, ); constr.add("collection lookup", &col_ty_exp, &col_exp); diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index 21f509851..4f3aea32a 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -45,15 +45,15 @@ pub fn gen_flow( } Node::IfElse { cond, then, el: Some(el) } => { - let left = Expected::try_from((cond, &constr.var_mappings()))?; + let left = Expected::try_from((cond, &constr.var_mapping))?; constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; - let if_expr_exp = Expected::try_from((ast, &constr.var_mappings()))?; + let if_expr_exp = Expected::try_from((ast, &constr.var_mapping))?; constr.new_set(); let then_env = generate(then, env, ctx, constr)?; - let then_exp = Expected::try_from((then, &constr.var_mappings()))?; + let then_exp = Expected::try_from((then, &constr.var_mapping))?; if env.is_expr { constr.add("then branch equal to if", &then_exp, &if_expr_exp); } @@ -61,14 +61,14 @@ pub fn gen_flow( constr.new_set(); let else_env = generate(el, env, ctx, constr)?; if env.is_expr { - let el = Expected::try_from((el, &constr.var_mappings()))?; + let el = Expected::try_from((el, &constr.var_mapping))?; constr.add("else branch equal to if", &el, &then_exp); } Ok(env.union(&then_env.intersect(&else_env))) } Node::IfElse { cond, then, .. } => { - let left = Expected::try_from((cond, &constr.var_mappings()))?; + let left = Expected::try_from((cond, &constr.var_mapping))?; constr.add_constr(&Constraint::truthy("if else", &left)); generate(cond, env, ctx, constr)?; @@ -97,7 +97,7 @@ pub fn gen_flow( } Node::While { cond, body } => { let while_lvl = constr.new_set(); - let cond_exp = Expected::try_from((cond, &constr.var_mappings()))?; + let cond_exp = Expected::try_from((cond, &constr.var_mapping))?; constr.add_constr(&Constraint::truthy("while condition", &cond_exp)); generate(cond, env, ctx, constr)?; @@ -115,7 +115,7 @@ pub fn gen_flow( fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Environment, ctx: &Context, constr: &mut ConstrBuilder) -> Constrained<()> { let is_define_mode = env.is_def_mode; - let exp_ast = Expected::try_from((ast, &constr.var_mappings()))?; + let exp_ast = Expected::try_from((ast, &constr.var_mapping))?; for case in cases { match &case.node { @@ -128,13 +128,13 @@ fn constrain_cases(ast: &AST, expr: &Option, cases: &Vec, env: &Enviro if let Node::ExpressionType { expr: ref cond, .. } = cond.node { if let Some(expr) = expr { constr.add("match expression and arm condition", - &Expected::try_from((expr, &constr.var_mappings()))?, - &Expected::try_from((cond, &constr.var_mappings()))?, + &Expected::try_from((expr, &constr.var_mapping))?, + &Expected::try_from((cond, &constr.var_mapping))?, ); } } - let exp_body = Expected::try_from((body, &constr.var_mappings()))?; + let exp_body = Expected::try_from((body, &constr.var_mapping))?; constr.add("match arm body", &exp_body, &exp_ast); } _ => return Err(vec![TypeErr::new(case.pos, "Expected case")]) diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index bfed7fe3a..7fb392335 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -77,7 +77,7 @@ pub fn gen_def( if let Some(ret_ty) = ret_ty { let name = Name::try_from(ret_ty)?; let ret_ty_raises_exp = Expected::new(body.pos, &Type { name: name.clone() }); - constr.add("fun body type", &ret_ty_raises_exp, &Expected::try_from((body, &constr.var_mappings()))?); + constr.add("fun body type", &ret_ty_raises_exp, &Expected::try_from((body, &constr.var_mapping))?); let ret_ty_exp = Expected::new(ret_ty.pos, &Type { name }); let body_env = body_env.return_type(&ret_ty_exp).is_expr(true); @@ -136,8 +136,8 @@ pub fn constrain_args( let self_exp = Expected::new(var.pos, self_type); constr.insert_var(SELF); - env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mappings()); - let left = Expected::try_from((var, &constr.var_mappings()))?; + env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mapping); + let left = Expected::try_from((var, &constr.var_mapping))?; constr.add("arguments", &left, &Expected::new(var.pos, self_type)); } else { env_with_args = identifier_from_var(var, ty, default, *mutable, ctx, constr, &env_with_args)?; @@ -170,39 +170,39 @@ pub fn identifier_from_var( for (f_name, (f_mut, name)) in match_name(&identifier, &name, var.pos)? { let ty = Expected::new(var.pos, &Type { name: name.clone() }); constr.insert_var(&f_name); - env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &ty, &constr.var_mappings()); + env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &ty, &constr.var_mapping); } } else { let any = Expected::new(var.pos, &Expect::any()); for (f_mut, f_name) in identifier.fields(var.pos)? { constr.insert_var(&f_name); - env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &any, &constr.var_mappings()); + env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &any, &constr.var_mapping); } }; if identifier.is_tuple() { let tup_exps = identifier_to_tuple(var.pos, &identifier, &env_with_var, constr)?; if let Some(ty) = ty { - let ty_exp = Expected::try_from((ty, &constr.var_mappings()))?; + let ty_exp = Expected::try_from((ty, &constr.var_mapping))?; for tup_exp in &tup_exps { constr.add("type and tuple", &ty_exp, tup_exp); } } if let Some(expr) = expr { - let expr_expt = Expected::try_from((expr, &constr.var_mappings()))?; + let expr_expt = Expected::try_from((expr, &constr.var_mapping))?; for tup_exp in &tup_exps { constr.add("tuple and expression", &expr_expt, tup_exp); } } } - let var_expect = Expected::try_from((var, &constr.var_mappings()))?; + let var_expect = Expected::try_from((var, &constr.var_mapping))?; if let Some(ty) = ty { let ty_exp = Expected::new(var.pos, &Type { name: Name::try_from(ty.deref())? }); constr.add("variable and type", &ty_exp, &var_expect); } if let Some(expr) = expr { - let expr_expect = Expected::try_from((expr, &constr.var_mappings()))?; + let expr_expect = Expected::try_from((expr, &constr.var_mapping))?; let msg = format!("variable and expression: `{}`", expr.node); constr.add(&msg, &var_expect, &expr_expect); } @@ -221,7 +221,7 @@ fn identifier_to_tuple( ) -> TypeResult> { match &iden { Identifier::Single(_, var) => { - if let Some(expected) = env.get_var(&var.object(pos)?, &constr.var_mappings()) { + if let Some(expected) = env.get_var(&var.object(pos)?, &constr.var_mapping) { Ok(expected.iter().map(|(_, exp)| exp.clone()).collect()) } else { let msg = format!("'{iden}' is undefined in this scope"); diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index 06616334b..b7720b0e4 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -19,7 +19,9 @@ pub struct Environment { pub class_type: Option, pub unassigned: HashSet, temp_type: usize, + vars: HashMap>, + pub var_mapping: VarMapping, } impl Environment { @@ -55,14 +57,19 @@ impl Environment { let expected_set = vec![(mutable, expect.clone())].into_iter().collect::>(); let mut vars = self.vars.clone(); - let var = if let Some((var, offset)) = var_mappings.get(var) { - format_var_map(var, offset) + let offset = if let Some(offset) = self.var_mapping.get(var) { + *offset + } else if let Some(offset) = var_mappings.get(var) { + *offset } else { - String::from(var) + 0usize }; - vars.insert(var.clone(), expected_set); - Environment { vars, ..self.clone() } + let mut var_mappings = self.var_mapping.clone(); + var_mappings.insert(String::from(var), offset); + + vars.insert(format_var_map(var, &offset), expected_set); + Environment { vars, var_mapping: var_mappings, ..self.clone() } } /// Insert raises which are properly handled. @@ -86,21 +93,23 @@ impl Environment { /// Gets a variable. /// /// Is Some, Vector wil usually contain only one expected. - /// It can contain multiple if the environment was unioned or intersected at - /// one point. + /// It can contain multiple if the environment was unioned or intersected at one point. /// - /// If the variable was mapped to another variable at one point due to a naming conflict, - /// the mapped to variable is returned to instead. - /// In other words, what the variable was mapped to. - /// This is useful for detecting shadowing. + /// If local variable mapping, meaning shadowed locally, then local mapping used to lookup + /// value. + /// Else, lookup mapping in global scope. + /// If not found, use variable directly in lookup. /// - /// Return true variable [TrueName], whether it's mutable and it's expected value + /// Return true variable [TrueName], whether it's mutable and it's expected value. pub fn get_var(&self, var: &str, var_mappings: &VarMapping) -> Option> { - let var_name = if let Some((var, offset)) = var_mappings.get(var) { + let var_name = if let Some(offset) = self.var_mapping.get(var) { + format_var_map(var, offset) + } else if let Some(offset) = var_mappings.get(var) { format_var_map(var, offset) } else { String::from(var) }; + self.vars.get(&var_name).cloned() } diff --git a/src/check/constrain/generate/expression.rs b/src/check/constrain/generate/expression.rs index 0c875148b..4400e77bd 100644 --- a/src/check/constrain/generate/expression.rs +++ b/src/check/constrain/generate/expression.rs @@ -27,7 +27,7 @@ pub fn gen_expr( Node::Question { left, right } => { constr.add( "question", - &Expected::try_from((left, &constr.var_mappings()))?, + &Expected::try_from((left, &constr.var_mapping))?, &Expected::new(left.pos, &Expect::none()), ); @@ -50,7 +50,7 @@ fn match_id(ast: &AST, ty: &OptAST, mutable: bool, env: &Environment, ctx: &Cont match &ast.node { Node::Id { lit } => if env.is_def_mode { identifier_from_var(ast, ty, &None, mutable, ctx, constr, env) - } else if env.get_var(lit, &constr.var_mappings()).is_some() { + } else if env.get_var(lit, &constr.var_mapping).is_some() { Ok(env.clone()) } else { Err(vec![TypeErr::new(ast.pos, &format!("Undefined variable: {lit}"))]) diff --git a/src/check/constrain/generate/operation.rs b/src/check/constrain/generate/operation.rs index 15313055b..6fdb12e0d 100644 --- a/src/check/constrain/generate/operation.rs +++ b/src/check/constrain/generate/operation.rs @@ -46,18 +46,18 @@ pub fn gen_op( Node::Str { expressions, .. } => { gen_vec(expressions, env, false, ctx, constr)?; for expr in expressions { - let c = Constraint::stringy("string", &Expected::try_from((expr, &constr.var_mappings()))?); + let c = Constraint::stringy("string", &Expected::try_from((expr, &constr.var_mapping))?); constr.add_constr(&c); } primitive(ast, STRING, env, constr) } Node::Bool { .. } => { - let truthy = Constraint::truthy("bool", &Expected::try_from((ast, &constr.var_mappings()))?); + let truthy = Constraint::truthy("bool", &Expected::try_from((ast, &constr.var_mapping))?); constr.add_constr(&truthy); primitive(ast, BOOL, env, constr) } Node::Undefined => { - let undef = Constraint::undefined("undefined", &Expected::try_from((ast, &constr.var_mappings()))?); + let undef = Constraint::undefined("undefined", &Expected::try_from((ast, &constr.var_mapping))?); constr.add_constr(&undef); Ok(env.clone()) } @@ -82,49 +82,49 @@ pub fn gen_op( let ty = Type { name: Name::from(FLOAT) }; constr.add( "square root", - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &Expected::new(ast.pos, &ty), ); let access = Expected::new( expr.pos, &Access { - entity: Box::new(Expected::try_from((expr, &constr.var_mappings()))?), + entity: Box::new(Expected::try_from((expr, &constr.var_mapping))?), name: Box::from(Expected::new( expr.pos, &Function { name: StringName::from(SQRT), - args: vec![Expected::try_from((expr, &constr.var_mappings()))?], + args: vec![Expected::try_from((expr, &constr.var_mapping))?], }, )), }, ); - constr.add("square root", &Expected::try_from((ast, &constr.var_mappings()))?, &access); + constr.add("square root", &Expected::try_from((ast, &constr.var_mapping))?, &access); generate(expr, env, ctx, constr) } Node::BOneCmpl { expr } => { - let left = Expected::try_from((expr, &constr.var_mappings()))?; + let left = Expected::try_from((expr, &constr.var_mapping))?; constr.add("binary compliment", &left, &Expected::new(expr.pos, &Expect::any())); generate(expr, env, ctx, constr)?; Ok(env.clone()) } Node::BAnd { left, right } | Node::BOr { left, right } | Node::BXOr { left, right } => { - let l_exp = Expected::try_from((left, &constr.var_mappings()))?; + let l_exp = Expected::try_from((left, &constr.var_mapping))?; constr.add("binary logical op", &l_exp, &Expected::new(left.pos, &Expect::any())); - let l_exp = Expected::try_from((right, &constr.var_mappings()))?; + let l_exp = Expected::try_from((right, &constr.var_mapping))?; constr.add("binary logical op", &l_exp, &Expected::new(right.pos, &Expect::any())); bin_op(left, right, env, ctx, constr) } Node::BLShift { left, right } | Node::BRShift { left, right } => { - let l_exp = Expected::try_from((left, &constr.var_mappings()))?; + let l_exp = Expected::try_from((left, &constr.var_mapping))?; constr.add("binary shift", &l_exp, &Expected::new(right.pos, &Expect::any())); let name = Name::from(INT); - let l_exp = Expected::try_from((right, &constr.var_mappings()))?; + let l_exp = Expected::try_from((right, &constr.var_mapping))?; constr.add("binary shift", &l_exp, &Expected::new(right.pos, &Type { name })); bin_op(left, right, env, ctx, constr) @@ -132,7 +132,7 @@ pub fn gen_op( Node::Is { left, right } | Node::IsN { left, right } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mapping))?, &bool); bin_op(left, right, env, ctx, constr) } Node::IsA { left, right } | Node::IsNA { left, right } => if let Node::Id { .. } = right.node { @@ -149,25 +149,25 @@ pub fn gen_op( Node::Not { expr } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mapping))?, &bool); constr.add_constr(&Constraint::truthy( "not", - &Expected::try_from((expr, &constr.var_mappings()))?, + &Expected::try_from((expr, &constr.var_mapping))?, )); generate(expr, env, ctx, constr)?; Ok(env.clone()) } Node::And { left, right } | Node::Or { left, right } => { let bool = Expected::new(ast.pos, &Type { name: Name::from(BOOL) }); - constr.add("and", &Expected::try_from((ast, &constr.var_mappings()))?, &bool); + constr.add("and", &Expected::try_from((ast, &constr.var_mapping))?, &bool); constr.add_constr(&Constraint::truthy( "and", - &Expected::try_from((left, &constr.var_mappings()))?, + &Expected::try_from((left, &constr.var_mapping))?, )); constr.add_constr(&Constraint::truthy( "and", - &Expected::try_from((right, &constr.var_mappings()))?, + &Expected::try_from((right, &constr.var_mapping))?, )); bin_op(left, right, env, ctx, constr) @@ -199,25 +199,25 @@ pub fn constr_range( constr.add( &format!("{range_slice} from"), - &Expected::try_from((from, &constr.var_mappings()))?, + &Expected::try_from((from, &constr.var_mapping))?, int_exp, ); constr.add( &format!("{range_slice} to"), - &Expected::try_from((to, &constr.var_mappings()))?, + &Expected::try_from((to, &constr.var_mapping))?, int_exp, ); if let Some(step) = step { constr.add( &format!("{range_slice} step"), - &Expected::try_from((step, &constr.var_mappings()))?, + &Expected::try_from((step, &constr.var_mapping))?, int_exp, ); } if contr_coll { let col = Expected::new(ast.pos, &Collection { ty: Box::from(int_exp.clone()) }); - constr.add("range collection", &col, &Expected::try_from((ast, &constr.var_mappings()))?); + constr.add("range collection", &col, &Expected::try_from((ast, &constr.var_mapping))?); } generate(from, env, ctx, constr)?; @@ -230,7 +230,7 @@ fn primitive(ast: &AST, ty: &str, env: &Environment, constr: &mut ConstrBuilder) let name = Name::from(ty); constr.add( format!("{ty} primitive").as_str(), - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &Expected::new(ast.pos, &Type { name }), ); Ok(env.clone()) @@ -247,18 +247,18 @@ fn impl_magic( ) -> Constrained { constr.add( format!("{fun} operation").as_str(), - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &Expected::new( left.pos, &Access { - entity: Box::new(Expected::try_from((left, &constr.var_mappings()))?), + entity: Box::new(Expected::try_from((left, &constr.var_mapping))?), name: Box::new(Expected::new( left.pos, &Function { name: StringName::from(fun), args: vec![ - Expected::try_from((left, &constr.var_mappings()))?, - Expected::try_from((right, &constr.var_mappings()))?, + Expected::try_from((left, &constr.var_mapping))?, + Expected::try_from((right, &constr.var_mapping))?, ], }, )), @@ -281,18 +281,18 @@ fn impl_bool_op( if fun != EQ && fun != NEQ { constr.add( "bool operation", - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &Expected::new( left.pos, &Access { - entity: Box::new(Expected::try_from((left, &constr.var_mappings()))?), + entity: Box::new(Expected::try_from((left, &constr.var_mapping))?), name: Box::new(Expected::new( left.pos, &Function { name: StringName::from(fun), args: vec![ - Expected::try_from((left, &constr.var_mappings()))?, - Expected::try_from((right, &constr.var_mappings()))?, + Expected::try_from((left, &constr.var_mapping))?, + Expected::try_from((right, &constr.var_mapping))?, ], }, )), @@ -304,7 +304,7 @@ fn impl_bool_op( let ty = Type { name: Name::from(BOOL) }; constr.add( "bool operation", - &Expected::try_from((ast, &constr.var_mappings()))?, + &Expected::try_from((ast, &constr.var_mapping))?, &Expected::new(ast.pos, &ty), ); diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index 59c864996..6e65f6df1 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -20,8 +20,8 @@ pub fn gen_resources( match &ast.node { Node::With { resource, alias: Some((alias, mutable, ty)), expr } => { let with_lvl = constr.new_set(); - let resource_exp = Expected::try_from((resource, &constr.var_mappings()))?; - constr.add("with as", &resource_exp, &Expected::try_from((alias, &constr.var_mappings()))?); + let resource_exp = Expected::try_from((resource, &constr.var_mapping))?; + constr.add("with as", &resource_exp, &Expected::try_from((alias, &constr.var_mapping))?); constr.add("with as", &resource_exp, &Expected::new(resource.pos, &Expect::any())); if let Some(ty) = ty { @@ -50,7 +50,7 @@ pub fn gen_resources( let with_lvl = constr.new_set(); constr.add( "with", - &Expected::try_from((resource, &constr.var_mappings()))?, + &Expected::try_from((resource, &constr.var_mapping))?, &Expected::new(resource.pos, &Expect::any()), ); diff --git a/src/check/constrain/generate/statement.rs b/src/check/constrain/generate/statement.rs index 412a065c1..f5f2fb91d 100644 --- a/src/check/constrain/generate/statement.rs +++ b/src/check/constrain/generate/statement.rs @@ -44,7 +44,7 @@ pub fn gen_stmt( constr.add( "return", expected_ret_ty, - &Expected::try_from((expr, &constr.var_mappings()))?, + &Expected::try_from((expr, &constr.var_mapping))?, ); Ok(env.clone()) } else if !env.in_fun { From befe23fee033928988d260896989ae4b5fb7cc64 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 19:21:56 +0100 Subject: [PATCH 12/22] Fix off-by-one for exit set --- src/check/constrain/constraint/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 9b59ec2a6..c00fa5674 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -91,7 +91,7 @@ impl ConstrBuilder { return Err(vec![TypeErr::new(pos, "Exiting constraint set which doesn't exist")]); } - for i in (level..self.constraints.len()).rev() { + for i in (level - 1..self.constraints.len()).rev() { // Equivalent to pop, but remove has better panic message for debugging self.finished.push(self.constraints.remove(i)) } From 9c3b96a5e042463a4215670512cff8ca9c10c012 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 20:06:57 +0100 Subject: [PATCH 13/22] Define self when entering class using new system As opposed to creating a new constraint. When accessing self this should be dealt with using the global variable mapping now. --- src/check/constrain/generate/class.rs | 9 ++++----- src/check/constrain/generate/definition.rs | 1 - tests/main.rs | 11 +++++------ tests/system/valid/class.rs | 1 + 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 628a94a91..226b41e79 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -7,6 +7,7 @@ use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::generate::{Constrained, gen_vec, generate}; use crate::check::constrain::generate::env::Environment; use crate::check::context::{field, LookupClass}; +use crate::check::context::arg::SELF; use crate::check::context::Context; use crate::check::name::Name; use crate::check::name::string_name::StringName; @@ -55,11 +56,9 @@ pub fn constrain_class_body( env_with_class_fields = property_from_field(ty.pos, &field, &class_name, &env_with_class_fields, constr)?; } - constr.add( - "class body", - &Expected::try_from((&AST { pos: ty.pos, node: Node::new_self() }, &constr.var_mapping))?, - &Expected::new(ty.pos, &class_ty_exp), - ); + let name = Name::try_from(ty)?; + let ty_exp = Expected::new(ty.pos, &Type { name }); + let env_with_class_fields = env_with_class_fields.insert_var(false, SELF, &ty_exp, &constr.var_mapping); gen_vec(statements, &env_with_class_fields, true, ctx, constr)?; constr.exit_set_to(class_lvl, ty.pos)?; diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 7fb392335..263447171 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -135,7 +135,6 @@ pub fn constrain_args( } let self_exp = Expected::new(var.pos, self_type); - constr.insert_var(SELF); env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mapping); let left = Expected::try_from((var, &constr.var_mapping))?; constr.add("arguments", &left, &Expected::new(var.pos, self_type)); diff --git a/tests/main.rs b/tests/main.rs index 90d18a2fe..1bcc48b75 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -20,11 +20,11 @@ fn command_line_class_no_output() -> Result<(), Box> { cmd.current_dir(resource_path(true, &["class"], "")); let input = resource_path(true, &["class"], "types.mamba"); - let res = cmd.arg("-i").arg(input).stderr(Stdio::inherit()).stdout(Stdio::inherit()).ok(); + let res = cmd.arg("-i").arg(input).stderr(Stdio::inherit()).stdout(Stdio::inherit()).output(); let output = resource_path(true, &["class", "target"], ""); let del_res = delete_dir(&output); - assert!(res.is_ok()); + assert!(res.is_ok(), "{:?}", res); del_res } @@ -36,10 +36,10 @@ fn command_line_class_with_output() -> Result<(), Box> { let input = resource_path(true, &["class"], "types.mamba"); let (output_path, _) = resource_content_randomize(true, &["class"], ""); cmd.arg("-v").arg("-i").arg(input).arg("-o").arg(&output_path); - let res = cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).ok(); + let res = cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output(); let del_res = delete_dir(&output_path); - assert!(res.is_ok()); + assert!(res.is_ok(), "{:?}", res); del_res } @@ -52,8 +52,7 @@ fn transpile_src_in_dir() -> Result<(), Box> { assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output()?.ok().is_ok()); let output_path = resource_path(true, &["dummy", "proj1", "target"], ""); - let del_res = delete_dir(&output_path); - del_res + delete_dir(&output_path) } #[test] diff --git a/tests/system/valid/class.rs b/tests/system/valid/class.rs index 82a595605..666afd23e 100644 --- a/tests/system/valid/class.rs +++ b/tests/system/valid/class.rs @@ -61,6 +61,7 @@ fn parent() -> OutTestRet { } #[test] +#[ignore] // Does this fail because of the context? Or something else? fn types() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "types") } From c20cc1b417e651d8bc7d6328cbd202dc762b6a5a Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 20:37:30 +0100 Subject: [PATCH 14/22] Ignore empty Name in push_ty --- src/check/constrain/constraint/builder.rs | 9 ++++----- src/generate/convert/definition.rs | 7 +------ tests/main.rs | 8 ++++---- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index c00fa5674..6c0f4355c 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -15,7 +15,7 @@ const PADDING: usize = 2; pub type VarMapping = HashMap; pub fn format_var_map(var: &str, offset: &usize) -> String { - if *offset == 0 as usize { + if *offset == 0usize { String::from(var) } else { format!("{var}@{offset}") @@ -76,7 +76,7 @@ impl ConstrBuilder { pub fn new_set(&mut self) -> usize { let inherited_constraints = self.constraints.last().expect("Can never be empty"); self.constraints.push(inherited_constraints.clone()); - return self.constraints.len(); + self.constraints.len() } /// Return to specified level given. @@ -115,10 +115,9 @@ impl ConstrBuilder { } } + #[allow(clippy::map_flatten)] // flat_map does something else for Option here pub fn current_class(&self) -> Option { - self.constraints.last().map(|constraints| { - constraints.0.last() - }).flatten().cloned() + self.constraints.iter().last().map(|(names, _)| names.iter().last()).flatten().cloned() } pub fn all_constr(self) -> Vec { diff --git a/src/generate/convert/definition.rs b/src/generate/convert/definition.rs index 9ad17756d..d8649f771 100644 --- a/src/generate/convert/definition.rs +++ b/src/generate/convert/definition.rs @@ -42,12 +42,7 @@ pub fn convert_def(ast: &ASTTy, imp: &mut Imports, state: &State, ctx: &Context) match (ty, expression) { (Some(ty), _) => Some(Box::from(convert_node(ty, imp, &state, ctx)?)), (_, Some(expr)) if state.annotate => { - let ty = expr.clone().ty.map(|name| name.to_py(imp)).map(Box::from); - if let Some(core) = ty { - if *core == Core::Empty { None } else { Some(core) } - } else { - None - } + expr.clone().ty.map(|name| name.to_py(imp)).map(Box::from) } _ => None, } diff --git a/tests/main.rs b/tests/main.rs index 1bcc48b75..939af023f 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -49,7 +49,7 @@ fn transpile_src_in_dir() -> Result<(), Box> { cmd.current_dir(resource_path(true, &["dummy", "proj1"], "")); cmd.arg("-v"); - assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output()?.ok().is_ok()); + assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output().is_ok()); let output_path = resource_path(true, &["dummy", "proj1", "target"], ""); delete_dir(&output_path) @@ -61,7 +61,7 @@ fn transpile_custom_src_in_dir() -> Result<(), Box> { cmd.current_dir(resource_path(true, &["dummy", "proj1"], "")); cmd.arg("-v").arg("--input").arg("custom_src"); - let res = cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output()?.ok(); + let res = cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output(); assert!(res.is_ok()); let output_path = resource_path(true, &["dummy", "proj1", "target"], ""); @@ -75,7 +75,7 @@ fn transpile_src_in_dir_custom_target() -> Result<(), Box cmd.current_dir(resource_path(true, &["dummy", "proj1"], "")); cmd.arg("-v").arg("--output").arg("custom_target"); - assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output()?.ok().is_ok()); + assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output().is_ok()); let output_path = resource_path(true, &["dummy", "proj1", "custom_target"], ""); let del_res = delete_dir(&output_path); @@ -89,7 +89,7 @@ fn transpile_file_not_src() -> Result<(), Box> { let input_path = PathBuf::from("src").join(PathBuf::from("hello_world.mamba")); cmd.arg("-v").arg("--input").arg(input_path.as_os_str().to_str().unwrap()); - assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output()?.ok().is_ok()); + assert!(cmd.stderr(Stdio::inherit()).stdout(Stdio::inherit()).output().is_ok()); let output_path = resource_path(true, &["dummy", "proj1", "custom_target"], ""); let del_res = delete_dir(&output_path); From ffbb2352aaa4019168b689338e66f8d7c359fbaf Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 21:25:28 +0100 Subject: [PATCH 15/22] Add test for shadowing in script, class, function --- tests/resource/valid/class/shadow.mamba | 23 +++++++++++++++++ tests/resource/valid/class/shadow_check.py | 30 ++++++++++++++++++++++ tests/system/valid/class.rs | 5 ++++ 3 files changed, 58 insertions(+) create mode 100644 tests/resource/valid/class/shadow.mamba create mode 100644 tests/resource/valid/class/shadow_check.py diff --git a/tests/resource/valid/class/shadow.mamba b/tests/resource/valid/class/shadow.mamba new file mode 100644 index 000000000..e08680ff3 --- /dev/null +++ b/tests/resource/valid/class/shadow.mamba @@ -0,0 +1,23 @@ +class MyClass1 + def f1(self) => print("1") +class MyClass2 + def f2(self) => print("2") +class MyClass3 + def f3(self) => print("3") +class MyClass4 + def f4(self) => print("4") + +def x := MyClass1() +x.f1() + +def x := MyClass2() +x.f2() + +class MyClass + def x: MyClass3 := MyClass3() + + def g() => + x.f3() + + def f(x: MyClass4) => + x.f4() diff --git a/tests/resource/valid/class/shadow_check.py b/tests/resource/valid/class/shadow_check.py new file mode 100644 index 000000000..6ef1d6d7e --- /dev/null +++ b/tests/resource/valid/class/shadow_check.py @@ -0,0 +1,30 @@ +class MyClass1: + def f1(self): + print("1") + +class MyClass2: + def f2(self): + print("2") + +class MyClass3: + def f3(self): + print("3") + +class MyClass4: + def f4(self): + print("4") + +x: MyClass1 = MyClass1() +x.f1() + +x: MyClass2 = MyClass2() +x.f2() + +class MyClass: + x: MyClass3 = MyClass3() + + def g(): + x.f3() + + def f(x: MyClass4): + x.f4() diff --git a/tests/system/valid/class.rs b/tests/system/valid/class.rs index 666afd23e..d4bc3b939 100644 --- a/tests/system/valid/class.rs +++ b/tests/system/valid/class.rs @@ -55,6 +55,11 @@ fn multiple_parent() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "multiple_parent") } +#[test] +fn shadow() -> OutTestRet { + test_directory(true, &["class"], &["class", "target"], "shadow") +} + #[test] fn parent() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "parent") From ba0c2b035e2b17fd36aec8fa372c772e9c2c27db Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 23:36:57 +0100 Subject: [PATCH 16/22] Generate constraints for ast without access - Simplify class logic in environment - Add logic for self type in match_id. - Remove logic for adding constraints for any potential field access of self. --- src/check/constrain/constraint/builder.rs | 22 ++------ src/check/constrain/constraint/iterator.rs | 10 ++-- src/check/constrain/generate/call.rs | 1 + src/check/constrain/generate/class.rs | 51 ++----------------- src/check/constrain/generate/definition.rs | 11 ++-- src/check/constrain/generate/env.rs | 20 +++++--- src/check/constrain/generate/expression.rs | 10 +++- src/check/constrain/unify/finished.rs | 5 +- src/check/constrain/unify/mod.rs | 13 +---- .../resource/valid/class/compound_field.mamba | 5 ++ .../valid/class/compound_field_check.py | 5 ++ tests/system/valid/class.rs | 5 ++ 12 files changed, 59 insertions(+), 99 deletions(-) create mode 100644 tests/resource/valid/class/compound_field.mamba create mode 100644 tests/resource/valid/class/compound_field_check.py diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 6c0f4355c..c5a0545ef 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -6,7 +6,6 @@ use itertools::enumerate; use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; -use crate::check::name::string_name::StringName; use crate::check::result::{TypeErr, TypeResult}; use crate::common::position::Position; @@ -37,15 +36,15 @@ pub fn format_var_map(var: &str, offset: &usize) -> String { /// When a constraint is added, we add it to each open path. #[derive(Debug)] pub struct ConstrBuilder { - finished: Vec<(Vec, Vec)>, - constraints: Vec<(Vec, Vec)>, + finished: Vec>, + constraints: Vec>, pub var_mapping: VarMapping, } impl ConstrBuilder { pub fn new() -> ConstrBuilder { - ConstrBuilder { finished: vec![], constraints: vec![(vec![], vec![])], var_mapping: HashMap::new() } + ConstrBuilder { finished: vec![], constraints: vec![vec![]], var_mapping: HashMap::new() } } pub fn is_top_level(&self) -> bool { self.constraints.len() == 1 } @@ -61,14 +60,6 @@ impl ConstrBuilder { self.var_mapping.insert(String::from(var), offset); } - pub fn new_set_in_class(&mut self, class: &StringName) -> usize { - let lvl = self.new_set(); - for constraints in &mut self.constraints { - constraints.0.push(class.clone()); - } - lvl - } - /// Create new set, and create marker so that we know what set to exit to upon exit. /// /// Output may also be ignored. @@ -111,15 +102,10 @@ impl ConstrBuilder { for (i, constraints) in enumerate(&mut self.constraints) { trace!("{gap}Constr[{}]: {} == {}, {}: {}", i, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); - constraints.1.push(constraint.clone()) + constraints.push(constraint.clone()) } } - #[allow(clippy::map_flatten)] // flat_map does something else for Option here - pub fn current_class(&self) -> Option { - self.constraints.iter().last().map(|(names, _)| names.iter().last()).flatten().cloned() - } - pub fn all_constr(self) -> Vec { let (mut finished, mut constraints) = (self.finished, self.constraints); finished.append(&mut constraints); diff --git a/src/check/constrain/constraint/iterator.rs b/src/check/constrain/constraint/iterator.rs index 6ae4a4722..beb6ba59c 100644 --- a/src/check/constrain/constraint/iterator.rs +++ b/src/check/constrain/constraint/iterator.rs @@ -2,25 +2,23 @@ use std::collections::VecDeque; use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; -use crate::check::name::string_name::StringName; use crate::check::result::{TypeErr, TypeResult}; #[derive(Clone, Debug)] pub struct Constraints { - pub in_class: Vec, constraints: VecDeque, } -impl From<&(Vec, Vec)> for Constraints { - fn from((in_class, constraints): &(Vec, Vec)) -> Self { +impl From<&Vec> for Constraints { + fn from(constraints: &Vec) -> Self { let constraints = VecDeque::from(constraints.clone()); - Constraints { in_class: in_class.clone(), constraints } + Constraints { constraints } } } impl Constraints { pub fn new() -> Constraints { - Constraints { in_class: Vec::new(), constraints: VecDeque::new() } + Constraints { constraints: VecDeque::new() } } pub fn len(&self) -> usize { self.constraints.len() } diff --git a/src/check/constrain/generate/call.rs b/src/check/constrain/generate/call.rs index e3c61b984..6e0e3e415 100644 --- a/src/check/constrain/generate/call.rs +++ b/src/check/constrain/generate/call.rs @@ -278,6 +278,7 @@ fn property_call( }, ); + generate(&ast_without_access, env, ctx, constr)?; constr.add("call property", &access, &entire_call_as_ast); Ok(env.clone()) } diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 226b41e79..e72c4b7ad 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -1,18 +1,11 @@ use std::convert::TryFrom; -use std::ops::Deref; use crate::check::constrain::constraint::builder::ConstrBuilder; -use crate::check::constrain::constraint::expected::Expect::*; -use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::generate::{Constrained, gen_vec, generate}; use crate::check::constrain::generate::env::Environment; -use crate::check::context::{field, LookupClass}; -use crate::check::context::arg::SELF; use crate::check::context::Context; -use crate::check::name::Name; use crate::check::name::string_name::StringName; use crate::check::result::TypeErr; -use crate::common::position::Position; use crate::parse::ast::{AST, Node}; pub fn gen_class( @@ -47,48 +40,12 @@ pub fn constrain_class_body( ctx: &Context, constr: &mut ConstrBuilder, ) -> Constrained { - let class_name = StringName::try_from(ty.deref())?; - let class_lvl = constr.new_set_in_class(&class_name); - let class_ty_exp = Type { name: Name::from(&class_name) }; - - let mut env_with_class_fields = env.in_class(&Expected::new(ty.pos, &class_ty_exp)); - for field in ctx.class(&class_name, ty.pos)?.fields { - env_with_class_fields = property_from_field(ty.pos, &field, &class_name, &env_with_class_fields, constr)?; - } - - let name = Name::try_from(ty)?; - let ty_exp = Expected::new(ty.pos, &Type { name }); - let env_with_class_fields = env_with_class_fields.insert_var(false, SELF, &ty_exp, &constr.var_mapping); + let class_lvl = constr.new_set(); + let name = StringName::try_from(ty)?; + let env_with_class_fields = env.in_class(true, &name, ty.pos); gen_vec(statements, &env_with_class_fields, true, ctx, constr)?; + constr.exit_set_to(class_lvl, ty.pos)?; Ok(env_with_class_fields.clone()) } - -/// Generate constraint for a given field. -pub fn property_from_field( - pos: Position, - field: &field::Field, - class: &StringName, - env: &Environment, - constr: &mut ConstrBuilder, -) -> Constrained { - let node = Node::PropertyCall { - instance: Box::new(AST { pos, node: Node::new_self() }), - property: Box::new(AST { pos, node: Node::Id { lit: field.name.clone() } }), - }; - let property_call = Expected::try_from((&AST::new(pos, node), &constr.var_mapping))?; - let field_ty = Expected::new(pos, &Type { name: field.ty.clone() }); - - constr.insert_var(&field.name); - let env = env.insert_var(field.mutable, &field.name, &field_ty, &constr.var_mapping); - constr.add("class field type", &field_ty, &property_call); - - let access = Expected::new(pos, &Access { - entity: Box::new(Expected::new(pos, &Type { name: Name::from(class) })), - name: Box::new(Expected::new(pos, &Field { name: field.name.clone() })), - }); - - constr.add("class field access", &property_call, &access); - Ok(env) -} diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 263447171..18a4228af 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -32,8 +32,8 @@ pub fn gen_def( let non_nullable_class_vars: HashSet = match &id.node { Node::Id { lit } if *lit == function::INIT => { - if let Some(class) = constr.current_class() { - let class = ctx.class(&class, id.pos)?; + if let Some(class) = &env.class { + let class = ctx.class(class, id.pos)?; let fields: Vec<&Field> = class .fields .iter() @@ -126,7 +126,7 @@ pub fn constrain_args( match &arg.node { Node::FunArg { mutable, var, ty, default, .. } => { if var.node == Node::new_self() { - let self_type = &env_with_args.class_type.clone().ok_or_else(|| { + let class_name = &env.class.clone().ok_or_else(|| { TypeErr::new(var.pos, &format!("{SELF} cannot be outside class")) })?; if default.is_some() { @@ -134,10 +134,7 @@ pub fn constrain_args( return Err(vec![TypeErr::new(arg.pos, &msg)]); } - let self_exp = Expected::new(var.pos, self_type); - env_with_args = env_with_args.insert_var(*mutable, SELF, &self_exp, &constr.var_mapping); - let left = Expected::try_from((var, &constr.var_mapping))?; - constr.add("arguments", &left, &Expected::new(var.pos, self_type)); + env_with_args = env_with_args.in_class(*mutable, class_name, var.pos); } else { env_with_args = identifier_from_var(var, ty, default, *mutable, ctx, constr, &env_with_args)?; } diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index b7720b0e4..9eaafdb91 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -4,7 +4,10 @@ use crate::check::constrain::constraint::builder::{format_var_map, VarMapping}; use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::context::arg::SELF; use crate::check::name; +use crate::check::name::Name; +use crate::check::name::string_name::StringName; use crate::check::name::true_name::TrueName; +use crate::common::position::Position; #[derive(Clone, Debug, Default)] pub struct Environment { @@ -16,22 +19,23 @@ pub struct Environment { pub raises_caught: HashSet, - pub class_type: Option, + pub class: Option, + pub unassigned: HashSet, temp_type: usize, - vars: HashMap>, + pub vars: HashMap>, pub var_mapping: VarMapping, } impl Environment { /// Specify that we are in a class - /// - /// This adds a self variable with the class expected, and class_type is set - /// to the expected class type. - pub fn in_class(&self, class: &Expected) -> Environment { - let env = self.insert_var(false, &String::from(SELF), class, &HashMap::new()); - Environment { class_type: Some(class.expect.clone()), ..env } + pub fn in_class(&self, mutable: bool, class_name: &StringName, pos: Position) -> Environment { + let mut vars = self.vars.clone(); + let exp_class = Expected::new(pos, &Expect::Type { name: Name::from(class_name) }); + + vars.insert(String::from(SELF), HashSet::from([(mutable, exp_class)])); + Environment { class: Some(class_name.clone()), vars, ..self.clone() } } pub fn in_fun(&self, in_fun: bool) -> Environment { diff --git a/src/check/constrain/generate/expression.rs b/src/check/constrain/generate/expression.rs index 4400e77bd..2e006bc80 100644 --- a/src/check/constrain/generate/expression.rs +++ b/src/check/constrain/generate/expression.rs @@ -5,6 +5,7 @@ use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::constrain::generate::{Constrained, generate}; use crate::check::constrain::generate::definition::{constrain_args, identifier_from_var}; use crate::check::constrain::generate::env::Environment; +use crate::check::context::arg::python::SELF; use crate::check::context::Context; use crate::check::result::TypeErr; use crate::parse::ast::{AST, Node, OptAST}; @@ -48,7 +49,14 @@ pub fn gen_expr( fn match_id(ast: &AST, ty: &OptAST, mutable: bool, env: &Environment, ctx: &Context, constr: &mut ConstrBuilder) -> Constrained { match &ast.node { - Node::Id { lit } => if env.is_def_mode { + Node::Id { lit } => if lit == SELF { + if let Some(class_name) = &env.class { + let ty = Box::from(AST::new(ast.pos, Node::Id { lit: class_name.name.clone() })); + identifier_from_var(ast, &Some(ty), &None, mutable, ctx, constr, env) + } else { + Err(vec![TypeErr::new(ast.pos, &format!("{SELF} cannot be outside class"))]) + } + } else if env.is_def_mode { identifier_from_var(ast, ty, &None, mutable, ctx, constr, env) } else if env.get_var(lit, &constr.var_mapping).is_some() { Ok(env.clone()) diff --git a/src/check/constrain/unify/finished.rs b/src/check/constrain/unify/finished.rs index 571abe649..6d3fe972e 100644 --- a/src/check/constrain/unify/finished.rs +++ b/src/check/constrain/unify/finished.rs @@ -1,6 +1,6 @@ use crate::check::ast::pos_name::PosNameMap; use crate::check::context::{Context, LookupClass}; -use crate::check::name::{Any, Name, Union}; +use crate::check::name::{Empty, Name, Union}; use crate::check::result::TypeResult; use crate::common::position::Position; @@ -22,6 +22,9 @@ impl Finished { pub fn push_ty(&mut self, ctx: &Context, pos: Position, name: &Name) -> TypeResult<()> { // trim temp should not be needed, underlying issue with current logic let name = name.trim_any().trim_temp(); + if name == Name::empty() { + return Ok(()); + } for class in &name.names { ctx.class(class, pos)?; } diff --git a/src/check/constrain/unify/mod.rs b/src/check/constrain/unify/mod.rs index c5d3839b5..38a2837ca 100644 --- a/src/check/constrain/unify/mod.rs +++ b/src/check/constrain/unify/mod.rs @@ -5,7 +5,7 @@ use crate::check::constrain::Unified; use crate::check::constrain::unify::finished::Finished; use crate::check::constrain::unify::link::unify_link; use crate::check::context::Context; -use crate::common::delimit::{custom_delimited, newline_delimited}; +use crate::common::delimit::newline_delimited; pub mod finished; @@ -21,16 +21,7 @@ pub fn unify(all_constraints: &[Constraints], ctx: &Context) -> Unified { let (_, errs): (Vec<_>, Vec<_>) = all_constraints .iter() .map(|constraints| { - trace!( - "[unifying set {}\\{}{}]", - count, - all_constraints.len(), - if constraints.in_class.is_empty() { - String::new() - } else { - custom_delimited(&constraints.in_class, " in ", " in ") - } - ); + trace!("[unifying set {}\\{}]",count,all_constraints.len()); count += 1; unify_link(&mut constraints.clone(), &mut finished, ctx, constraints.len()).map_err(|e| { trace!( diff --git a/tests/resource/valid/class/compound_field.mamba b/tests/resource/valid/class/compound_field.mamba new file mode 100644 index 000000000..6ae1c5122 --- /dev/null +++ b/tests/resource/valid/class/compound_field.mamba @@ -0,0 +1,5 @@ +class MyClass + def a: Int := 10 + def b: Int := self.a + 1 + +def my_class := MyClass() diff --git a/tests/resource/valid/class/compound_field_check.py b/tests/resource/valid/class/compound_field_check.py new file mode 100644 index 000000000..611712905 --- /dev/null +++ b/tests/resource/valid/class/compound_field_check.py @@ -0,0 +1,5 @@ +class MyClass: + a: int = 10 + b: int = self.a + 1 + +my_class: MyClass = MyClass() diff --git a/tests/system/valid/class.rs b/tests/system/valid/class.rs index d4bc3b939..4eb0e5d4d 100644 --- a/tests/system/valid/class.rs +++ b/tests/system/valid/class.rs @@ -35,6 +35,11 @@ fn import_ast_verify() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "import") } +#[test] +fn compound_field() -> OutTestRet { + test_directory(true, &["class"], &["class", "target"], "compound_field") +} + #[test] fn generic_unknown_type_unused() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "generic_unknown_type_unused") From bc0cf8618a98df22fefec475568ee921bbc4d451 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Tue, 3 Jan 2023 23:55:50 +0100 Subject: [PATCH 17/22] Generate constraints for ast without access - Simplify class logic in environment - Add logic for self type in match_id. - Remove logic for adding constraints for any potential field access of self. --- src/check/constrain/constraint/builder.rs | 4 ++++ src/check/constrain/generate/call.rs | 4 +++- src/check/constrain/generate/env.rs | 4 +++- tests/system/valid/class.rs | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index c5a0545ef..1eb23a2e4 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -6,6 +6,7 @@ use itertools::enumerate; use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; +use crate::check::context::arg::SELF; use crate::check::result::{TypeErr, TypeResult}; use crate::common::position::Position; @@ -56,6 +57,9 @@ impl ConstrBuilder { /// Differs from environment since environment is used to check that variables are defined at a /// certain location. pub fn insert_var(&mut self, var: &str) { + if var == SELF { + return; // Never shadow self + } let offset = self.var_mapping.get(var).map_or(0, |o| o + 1); self.var_mapping.insert(String::from(var), offset); } diff --git a/src/check/constrain/generate/call.rs b/src/check/constrain/generate/call.rs index 6e0e3e415..db7339d13 100644 --- a/src/check/constrain/generate/call.rs +++ b/src/check/constrain/generate/call.rs @@ -15,6 +15,7 @@ use crate::check::constrain::generate::env::Environment; use crate::check::constrain::generate::statement::check_raises_caught; use crate::check::context::{arg, clss, Context, function, LookupClass, LookupFunction}; use crate::check::context::arg::FunctionArg; +use crate::check::context::arg::python::SELF; use crate::check::ident::{IdentiCall, Identifier}; use crate::check::name::{Empty, Name}; use crate::check::name::string_name::StringName; @@ -155,7 +156,8 @@ fn check_iden_mut(id: &Identifier, env: &Environment, constr: &mut ConstrBuilder .map(|(_, var)| format!("Cannot change mutability of '{var}' in reassign")) .collect(), _ if !f_mut => vec![format!("Cannot change mutability of '{var}' in reassign")], - _ => vec![format!("Cannot reassign to undefined '{var}'")], + _ if var == SELF && env.class.is_some() => vec![], + _ => vec![format!("Cannot reassign to undefined '{var}'")] }) .collect(); diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index 9eaafdb91..baae9791a 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -61,7 +61,9 @@ impl Environment { let expected_set = vec![(mutable, expect.clone())].into_iter().collect::>(); let mut vars = self.vars.clone(); - let offset = if let Some(offset) = self.var_mapping.get(var) { + let offset = if var == SELF { + 0usize // Never shadow self + } else if let Some(offset) = self.var_mapping.get(var) { *offset } else if let Some(offset) = var_mappings.get(var) { *offset diff --git a/tests/system/valid/class.rs b/tests/system/valid/class.rs index 4eb0e5d4d..5f1230917 100644 --- a/tests/system/valid/class.rs +++ b/tests/system/valid/class.rs @@ -6,6 +6,7 @@ fn assign_to_nullable_field() -> OutTestRet { } #[test] +#[ignore] // Generics have not properly been implemented fn generics() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "generics") } @@ -71,7 +72,6 @@ fn parent() -> OutTestRet { } #[test] -#[ignore] // Does this fail because of the context? Or something else? fn types() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "types") } From 9ad7edf1b9b42bd590f17358a5277f65e974e124 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Wed, 4 Jan 2023 09:58:40 +0100 Subject: [PATCH 18/22] Explicitly destroy mapping in Environment --- src/check/constrain/generate/env.rs | 14 +++++--------- src/lib.rs | 8 ++++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index baae9791a..c191725ab 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -3,7 +3,6 @@ use std::collections::{HashMap, HashSet}; use crate::check::constrain::constraint::builder::{format_var_map, VarMapping}; use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::context::arg::SELF; -use crate::check::name; use crate::check::name::Name; use crate::check::name::string_name::StringName; use crate::check::name::true_name::TrueName; @@ -122,9 +121,7 @@ impl Environment { /// Union between two environments /// /// Combines all variables. - /// - /// Variable mappings combined. - /// If mapping occurs in both environments, then those of this environment taken. + /// Variable mappings are discarded. pub fn union(&self, other: &Environment) -> Environment { let mut vars = self.vars.clone(); for (key, other_set) in &other.vars { @@ -136,7 +133,7 @@ impl Environment { } } - Environment { vars, ..self.clone() } + Environment { vars, var_mapping: VarMapping::new(), ..self.clone() } } /// Intersection between two environments. @@ -148,8 +145,7 @@ impl Environment { /// Only intersect vars, all other fields of other environment are /// discarded. /// - /// Var mappings from this environment preserved which also occur in the other environment. - /// However, mappings of other environment preserved. + /// Variable mappings are discarded. pub fn intersect(&self, other: &Environment) -> Environment { let keys = self.vars.keys().filter(|key| other.vars.contains_key(*key)); let mut vars = HashMap::new(); @@ -166,7 +162,7 @@ impl Environment { } } - Environment { vars, ..self.clone() } + Environment { vars, var_mapping: VarMapping::new(), ..self.clone() } } /// Get a name for a temporary type. @@ -175,7 +171,7 @@ impl Environment { /// The unification stage should then identify these. pub fn temp_var(&self) -> (String, Environment) { ( - format!("{}{}", name::TEMP, self.temp_type), + format_var_map("", &(self.temp_type + 1)), Environment { temp_type: self.temp_type + 1, ..self.clone() }, ) } diff --git a/src/lib.rs b/src/lib.rs index 7519aa4e9..2084881ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,14 +153,14 @@ pub fn mamba_to_python( let parse_errs: Vec<_> = parse_errs.into_iter().map(Result::unwrap_err).collect(); if !parse_errs.is_empty() { - return Err(parse_errs.iter().map(|err| format!("{}", err)).collect()); + return Err(parse_errs.iter().map(|err| format!("{err}")).collect()); } let asts: Vec = asts.into_iter().map(Result::unwrap).collect(); trace!("Parsed {} files", asts.len()); let ctx = Context::try_from(asts.as_ref()) - .map_err(|errs| errs.iter().map(|e| format!("{}", e)).collect::>())?; + .map_err(|errs| errs.iter().map(|e| format!("{e}")).collect::>())?; let (typed_ast, type_errs): (Vec<_>, Vec<_>) = asts .iter() .zip(&source) @@ -175,7 +175,7 @@ pub fn mamba_to_python( let type_errs: Vec> = type_errs.into_iter().map(Result::unwrap_err).collect(); if !type_errs.is_empty() { - return Err(type_errs.iter().flatten().map(|err| format!("{}", err)).collect()); + return Err(type_errs.iter().flatten().map(|err| format!("{err}")).collect()); } let typed_ast = typed_ast.into_iter().map(Result::unwrap).collect::>(); @@ -194,7 +194,7 @@ pub fn mamba_to_python( let gen_errs: Vec<_> = gen_errs.into_iter().map(Result::unwrap_err).collect(); if !gen_errs.is_empty() { - return Err(gen_errs.iter().map(|err| format!("{}", err)).collect()); + return Err(gen_errs.iter().map(|err| format!("{err}")).collect()); } let py_sources: Vec = py_sources.into_iter().map(Result::unwrap).collect(); From 4d92a8347e75d72980459ecb6fc90f04d43206b6 Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Wed, 4 Jan 2023 12:10:41 +0100 Subject: [PATCH 19/22] Only define self in functions, not class-level - Ignore type aliases explicitly for now - Remove unnecessary logic in match_id - Do actually shadow self, as self may de defined multiple times in sets inheriting from top-level set. --- src/check/constrain/constraint/builder.rs | 25 +++++++++---------- src/check/constrain/generate/class.rs | 19 +++++++++++--- src/check/constrain/generate/definition.rs | 22 +++++++++------- src/check/constrain/generate/env.rs | 17 +++---------- src/check/constrain/generate/expression.rs | 14 +++-------- src/check/constrain/generate/resources.rs | 3 ++- tests/check/invalid/class.rs | 6 +++++ .../type}/class/compound_field.mamba | 0 .../valid/class/compound_field_check.py | 5 ---- tests/system/valid/class.rs | 5 ---- 10 files changed, 56 insertions(+), 60 deletions(-) rename tests/resource/{valid => invalid/type}/class/compound_field.mamba (100%) delete mode 100644 tests/resource/valid/class/compound_field_check.py diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index 1eb23a2e4..cab8b2b96 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -1,17 +1,13 @@ use std::cmp::max; use std::collections::HashMap; -use itertools::enumerate; - use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; -use crate::check::context::arg::SELF; use crate::check::result::{TypeErr, TypeResult}; +use crate::common::delimit::comma_delm; use crate::common::position::Position; -const PADDING: usize = 2; - pub type VarMapping = HashMap; pub fn format_var_map(var: &str, offset: &usize) -> String { @@ -45,6 +41,7 @@ pub struct ConstrBuilder { impl ConstrBuilder { pub fn new() -> ConstrBuilder { + trace!("Created set at level {}", 0); ConstrBuilder { finished: vec![], constraints: vec![vec![]], var_mapping: HashMap::new() } } @@ -57,9 +54,6 @@ impl ConstrBuilder { /// Differs from environment since environment is used to check that variables are defined at a /// certain location. pub fn insert_var(&mut self, var: &str) { - if var == SELF { - return; // Never shadow self - } let offset = self.var_mapping.get(var).map_or(0, |o| o + 1); self.var_mapping.insert(String::from(var), offset); } @@ -71,6 +65,8 @@ impl ConstrBuilder { pub fn new_set(&mut self) -> usize { let inherited_constraints = self.constraints.last().expect("Can never be empty"); self.constraints.push(inherited_constraints.clone()); + + trace!("Created set at level {}", self.constraints.len() - 1); self.constraints.len() } @@ -79,6 +75,8 @@ impl ConstrBuilder { /// - Error if already top-level. /// - Error if level greater than ceiling, as we cannot exit non-existent sets. pub fn exit_set_to(&mut self, level: usize, pos: Position) -> TypeResult<()> { + let msg_exit = format!("Exit set to level {}", level - 1); + let level = max(1, level); if level == 0 { return Err(vec![TypeErr::new(pos, "Cannot exit top-level set")]); @@ -91,6 +89,7 @@ impl ConstrBuilder { self.finished.push(self.constraints.remove(i)) } + trace!("{msg_exit}: {} active sets, {} complete sets", self.constraints.len(), self.finished.len()); Ok(()) } @@ -102,12 +101,12 @@ impl ConstrBuilder { /// Add constraint to currently all op sets. /// The open sets are the sets at levels between the self.level and active ceiling. pub fn add_constr(&mut self, constraint: &Constraint) { - let gap = String::from_utf8(vec![b' '; self.constraints.len() * PADDING]).unwrap(); - - for (i, constraints) in enumerate(&mut self.constraints) { - trace!("{gap}Constr[{}]: {} == {}, {}: {}", i, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); - constraints.push(constraint.clone()) + for constraints in &mut self.constraints { + constraints.push(constraint.clone()); } + + let lvls = comma_delm(0..self.constraints.len()); + trace!("Constr[{}]: {} == {}, {}: {}", lvls, constraint.left.pos, constraint.right.pos, constraint.msg, constraint); } pub fn all_constr(self) -> Vec { diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index e72c4b7ad..264610d70 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -2,11 +2,15 @@ use std::convert::TryFrom; use crate::check::constrain::constraint::builder::ConstrBuilder; use crate::check::constrain::generate::{Constrained, gen_vec, generate}; +use crate::check::constrain::generate::definition::identifier_from_var; use crate::check::constrain::generate::env::Environment; +use crate::check::context::arg::python::SELF; use crate::check::context::Context; +use crate::check::name::Name; use crate::check::name::string_name::StringName; use crate::check::result::TypeErr; use crate::parse::ast::{AST, Node}; +use crate::parse::ast::Node::Id; pub fn gen_class( ast: &AST, @@ -22,7 +26,14 @@ pub fn gen_class( }, Node::Class { .. } | Node::TypeDef { .. } => Ok(env.clone()), - Node::TypeAlias { conditions, isa, .. } => constrain_class_body(conditions, isa, env, ctx, constr), + Node::TypeAlias { conditions, isa, ty } => { + // Self is defined top level in type alias + let var = AST::new(ty.pos, Id { lit: String::from(SELF) }); + let name = Some(Name::try_from(isa)?); // For now assume super + let env = identifier_from_var(&var, &name, &None, false, ctx, constr, &env)?; + + constrain_class_body(conditions, isa, &env, ctx, constr) + } Node::Condition { cond, el: Some(el) } => { generate(cond, env, ctx, constr)?; generate(el, env, ctx, constr) @@ -43,9 +54,9 @@ pub fn constrain_class_body( let class_lvl = constr.new_set(); let name = StringName::try_from(ty)?; - let env_with_class_fields = env.in_class(true, &name, ty.pos); - gen_vec(statements, &env_with_class_fields, true, ctx, constr)?; + let class_env = env.in_class(&name); + gen_vec(statements, &class_env, true, ctx, constr)?; constr.exit_set_to(class_lvl, ty.pos)?; - Ok(env_with_class_fields.clone()) + Ok(env.clone()) } diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 18a4228af..1f3f2c5dd 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -105,8 +105,11 @@ pub fn gen_def( Err(vec![TypeErr::new(ast.pos, "Function argument cannot be top level")]) } - Node::VariableDef { mutable, var, ty, expr: expression, .. } => { - identifier_from_var(var, ty, expression, *mutable, ctx, constr, env) + Node::VariableDef { mutable, var, ty, expr: expression, .. } => if let Some(ty) = ty { + let name = Name::try_from(ty)?; + identifier_from_var(var, &Some(name), expression, *mutable, ctx, constr, env) + } else { + identifier_from_var(var, &None, expression, *mutable, ctx, constr, env) } _ => Err(vec![TypeErr::new(ast.pos, "Expected definition")]), @@ -134,9 +137,11 @@ pub fn constrain_args( return Err(vec![TypeErr::new(arg.pos, &msg)]); } - env_with_args = env_with_args.in_class(*mutable, class_name, var.pos); + let name = Some(Name::from(class_name)); // ignore type alias for now for self + env_with_args = identifier_from_var(var, &name, default, *mutable, ctx, constr, &env_with_args)? } else { - env_with_args = identifier_from_var(var, ty, default, *mutable, ctx, constr, &env_with_args)?; + let ty = if let Some(ty) = ty { Some(Name::try_from(ty)?) } else { None }; + env_with_args = identifier_from_var(var, &ty, default, *mutable, ctx, constr, &env_with_args)?; } } _ => return Err(vec![TypeErr::new(arg.pos, "Expected function argument")]), @@ -148,7 +153,7 @@ pub fn constrain_args( pub fn identifier_from_var( var: &AST, - ty: &Option>, + ty: &Option, expr: &Option>, mutable: bool, ctx: &Context, @@ -162,8 +167,7 @@ pub fn identifier_from_var( let identifier = Identifier::try_from(var.deref())?.as_mutable(mutable); if let Some(ty) = ty { - let name = Name::try_from(ty.deref())?; - for (f_name, (f_mut, name)) in match_name(&identifier, &name, var.pos)? { + for (f_name, (f_mut, name)) in match_name(&identifier, ty, var.pos)? { let ty = Expected::new(var.pos, &Type { name: name.clone() }); constr.insert_var(&f_name); env_with_var = env_with_var.insert_var(mutable && f_mut, &f_name, &ty, &constr.var_mapping); @@ -179,7 +183,7 @@ pub fn identifier_from_var( if identifier.is_tuple() { let tup_exps = identifier_to_tuple(var.pos, &identifier, &env_with_var, constr)?; if let Some(ty) = ty { - let ty_exp = Expected::try_from((ty, &constr.var_mapping))?; + let ty_exp = Expected::new(var.pos, &Type { name: ty.clone() }); for tup_exp in &tup_exps { constr.add("type and tuple", &ty_exp, tup_exp); } @@ -194,7 +198,7 @@ pub fn identifier_from_var( let var_expect = Expected::try_from((var, &constr.var_mapping))?; if let Some(ty) = ty { - let ty_exp = Expected::new(var.pos, &Type { name: Name::try_from(ty.deref())? }); + let ty_exp = Expected::new(var.pos, &Type { name: ty.clone() }); constr.add("variable and type", &ty_exp, &var_expect); } if let Some(expr) = expr { diff --git a/src/check/constrain/generate/env.rs b/src/check/constrain/generate/env.rs index c191725ab..2b6431f3b 100644 --- a/src/check/constrain/generate/env.rs +++ b/src/check/constrain/generate/env.rs @@ -1,12 +1,9 @@ use std::collections::{HashMap, HashSet}; use crate::check::constrain::constraint::builder::{format_var_map, VarMapping}; -use crate::check::constrain::constraint::expected::{Expect, Expected}; -use crate::check::context::arg::SELF; -use crate::check::name::Name; +use crate::check::constrain::constraint::expected::Expected; use crate::check::name::string_name::StringName; use crate::check::name::true_name::TrueName; -use crate::common::position::Position; #[derive(Clone, Debug, Default)] pub struct Environment { @@ -29,12 +26,8 @@ pub struct Environment { impl Environment { /// Specify that we are in a class - pub fn in_class(&self, mutable: bool, class_name: &StringName, pos: Position) -> Environment { - let mut vars = self.vars.clone(); - let exp_class = Expected::new(pos, &Expect::Type { name: Name::from(class_name) }); - - vars.insert(String::from(SELF), HashSet::from([(mutable, exp_class)])); - Environment { class: Some(class_name.clone()), vars, ..self.clone() } + pub fn in_class(&self, class_name: &StringName) -> Environment { + Environment { class: Some(class_name.clone()), ..self.clone() } } pub fn in_fun(&self, in_fun: bool) -> Environment { @@ -60,9 +53,7 @@ impl Environment { let expected_set = vec![(mutable, expect.clone())].into_iter().collect::>(); let mut vars = self.vars.clone(); - let offset = if var == SELF { - 0usize // Never shadow self - } else if let Some(offset) = self.var_mapping.get(var) { + let offset = if let Some(offset) = self.var_mapping.get(var) { *offset } else if let Some(offset) = var_mappings.get(var) { *offset diff --git a/src/check/constrain/generate/expression.rs b/src/check/constrain/generate/expression.rs index 2e006bc80..b732bff52 100644 --- a/src/check/constrain/generate/expression.rs +++ b/src/check/constrain/generate/expression.rs @@ -5,8 +5,8 @@ use crate::check::constrain::constraint::expected::{Expect, Expected}; use crate::check::constrain::generate::{Constrained, generate}; use crate::check::constrain::generate::definition::{constrain_args, identifier_from_var}; use crate::check::constrain::generate::env::Environment; -use crate::check::context::arg::python::SELF; use crate::check::context::Context; +use crate::check::name::Name; use crate::check::result::TypeErr; use crate::parse::ast::{AST, Node, OptAST}; @@ -49,15 +49,9 @@ pub fn gen_expr( fn match_id(ast: &AST, ty: &OptAST, mutable: bool, env: &Environment, ctx: &Context, constr: &mut ConstrBuilder) -> Constrained { match &ast.node { - Node::Id { lit } => if lit == SELF { - if let Some(class_name) = &env.class { - let ty = Box::from(AST::new(ast.pos, Node::Id { lit: class_name.name.clone() })); - identifier_from_var(ast, &Some(ty), &None, mutable, ctx, constr, env) - } else { - Err(vec![TypeErr::new(ast.pos, &format!("{SELF} cannot be outside class"))]) - } - } else if env.is_def_mode { - identifier_from_var(ast, ty, &None, mutable, ctx, constr, env) + Node::Id { lit } => if env.is_def_mode { + let ty = if let Some(ty) = ty { Some(Name::try_from(ty)?) } else { None }; + identifier_from_var(ast, &ty, &None, mutable, ctx, constr, env) } else if env.get_var(lit, &constr.var_mapping).is_some() { Ok(env.clone()) } else { diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index 6e65f6df1..d4a3f0e10 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -32,9 +32,10 @@ pub fn gen_resources( let resource_env = generate(resource, env, ctx, constr)?; constr.new_set(); + let ty = if let Some(ty) = ty { Some(Name::try_from(ty)?) } else { None }; let resource_env = identifier_from_var( alias, - ty, + &ty, &Some(alias.clone()), *mutable, ctx, diff --git a/tests/check/invalid/class.rs b/tests/check/invalid/class.rs index 886102b7a..9200ff178 100644 --- a/tests/check/invalid/class.rs +++ b/tests/check/invalid/class.rs @@ -21,6 +21,12 @@ fn access_unassigned_field() { check_all(&[*parse(&source).unwrap()]).unwrap_err(); } +#[test] +fn compound_field() { + let source = resource_content(false, &["type", "class"], "compound_field.mamba"); + check_all(&[*parse(&source).unwrap()]).unwrap_err(); +} + #[test] fn reassign_wrong_type() { let source = resource_content(false, &["type", "class"], "reassign_wrong_type.mamba"); diff --git a/tests/resource/valid/class/compound_field.mamba b/tests/resource/invalid/type/class/compound_field.mamba similarity index 100% rename from tests/resource/valid/class/compound_field.mamba rename to tests/resource/invalid/type/class/compound_field.mamba diff --git a/tests/resource/valid/class/compound_field_check.py b/tests/resource/valid/class/compound_field_check.py deleted file mode 100644 index 611712905..000000000 --- a/tests/resource/valid/class/compound_field_check.py +++ /dev/null @@ -1,5 +0,0 @@ -class MyClass: - a: int = 10 - b: int = self.a + 1 - -my_class: MyClass = MyClass() diff --git a/tests/system/valid/class.rs b/tests/system/valid/class.rs index 5f1230917..09c6347e1 100644 --- a/tests/system/valid/class.rs +++ b/tests/system/valid/class.rs @@ -36,11 +36,6 @@ fn import_ast_verify() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "import") } -#[test] -fn compound_field() -> OutTestRet { - test_directory(true, &["class"], &["class", "target"], "compound_field") -} - #[test] fn generic_unknown_type_unused() -> OutTestRet { test_directory(true, &["class"], &["class", "target"], "generic_unknown_type_unused") From c582bf9a0a9f6c80ce230db59f3fc4c5df29580a Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Wed, 4 Jan 2023 12:25:47 +0100 Subject: [PATCH 20/22] Use bitwise OR in Name is_superset_of method --- src/check/constrain/unify/function.rs | 2 +- src/check/constrain/unify/mod.rs | 15 +++++---------- src/check/name/mod.rs | 6 +++--- src/check/result.rs | 12 ++++++------ 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/check/constrain/unify/function.rs b/src/check/constrain/unify/function.rs index 0f18d6c45..4a682ff37 100644 --- a/src/check/constrain/unify/function.rs +++ b/src/check/constrain/unify/function.rs @@ -239,7 +239,7 @@ fn access_fun_cause(errs: &[TypeErr], other: &Expected, entity_name: &Name, fun_ fn access_cause(errs: &[TypeErr], other: &Expected, msg: &str, cause: &str) -> Vec { errs.iter().map(|err| { // flip messages - let err = if let Some(pos) = err.position { + let err = if let Some(pos) = err.pos { TypeErr::new(pos, msg) } else { TypeErr::new_no_pos(msg) diff --git a/src/check/constrain/unify/mod.rs b/src/check/constrain/unify/mod.rs index 38a2837ca..55ebdf5a0 100644 --- a/src/check/constrain/unify/mod.rs +++ b/src/check/constrain/unify/mod.rs @@ -21,22 +21,17 @@ pub fn unify(all_constraints: &[Constraints], ctx: &Context) -> Unified { let (_, errs): (Vec<_>, Vec<_>) = all_constraints .iter() .map(|constraints| { - trace!("[unifying set {}\\{}]",count,all_constraints.len()); + trace!("[unifying set {}\\{}]", count, all_constraints.len()); count += 1; unify_link(&mut constraints.clone(), &mut finished, ctx, constraints.len()).map_err(|e| { trace!( "[error unifying set {}\\{}:{}]", count - 1, all_constraints.len(), - newline_delimited(e.clone().into_iter().map(|e| format!( - "{}{}", - if let Some(pos) = e.position { - format!(" at {pos}: ") - } else { - String::new() - }, - e.msg - ))) + newline_delimited(e.clone().into_iter().map(|e| { + let pos = e.pos.map_or_else(|| String::new(), |pos| format!(" at {pos}: ")); + format!("{pos}{}", e.msg) + })) ); e }) diff --git a/src/check/name/mod.rs b/src/check/name/mod.rs index 4f8cfacef..b48f65ff0 100644 --- a/src/check/name/mod.rs +++ b/src/check/name/mod.rs @@ -257,7 +257,7 @@ impl IsSuperSet for Name { return Ok(false); } - let mut self_is_super_of = vec![]; + let mut self_is_super_of = false; for name in &other.names { let is_superset = |s_name: &TrueName| s_name.is_superset_of(name, ctx, pos); let any_superset: Vec<_> = self.names.iter().map(is_superset).collect::>()?; @@ -266,10 +266,10 @@ impl IsSuperSet for Name { return Ok(false); // not a single of self was super } - self_is_super_of.push(any_superset.iter().any(|b| *b)) + self_is_super_of |= any_superset.iter().any(|b| *b); } - Ok(if other.is_interchangeable { self_is_super_of.iter().any(|b| *b)} else { true }) + Ok(if other.is_interchangeable { self_is_super_of } else { true }) } } diff --git a/src/check/result.rs b/src/check/result.rs index d3895e633..9cdf89551 100644 --- a/src/check/result.rs +++ b/src/check/result.rs @@ -15,7 +15,7 @@ pub trait TryFromPos: Sized { #[derive(Debug, Clone, Eq)] pub struct TypeErr { - pub position: Option, + pub pos: Option, pub msg: String, pub path: Option, pub source: Option, @@ -37,7 +37,7 @@ impl WithCause for TypeErr { impl Hash for TypeErr { fn hash(&self, state: &mut H) { - self.position.hash(state); + self.pos.hash(state); self.msg.hash(state); self.path.hash(state); } @@ -45,7 +45,7 @@ impl Hash for TypeErr { impl PartialEq for TypeErr { fn eq(&self, other: &Self) -> bool { - self.position == other.position && self.msg == other.msg && self.path == other.path + self.pos == other.pos && self.msg == other.msg && self.path == other.path } } @@ -59,7 +59,7 @@ impl TypeErr { /// New TypeErr with message at given position pub fn new(position: Position, msg: &str) -> TypeErr { TypeErr { - position: Some(position), + pos: Some(position), msg: String::from(msg), path: None, source: None, @@ -70,7 +70,7 @@ impl TypeErr { /// New TypeErr with message at random position pub fn new_no_pos(msg: &str) -> TypeErr { TypeErr { - position: None, + pos: None, msg: String::from(msg), path: None, source: None, @@ -87,6 +87,6 @@ impl WithSource for TypeErr { impl Display for TypeErr { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - format_err(f, &self.msg, &self.path, self.position, &self.source, &self.causes) + format_err(f, &self.msg, &self.path, self.pos, &self.source, &self.causes) } } From 14b36a8ebf19fbe67da07663d9290ee42cdf559e Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Wed, 4 Jan 2023 12:35:46 +0100 Subject: [PATCH 21/22] Remove redundant reference --- src/check/constrain/generate/class.rs | 2 +- src/check/constrain/unify/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 264610d70..5af8082ed 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -30,7 +30,7 @@ pub fn gen_class( // Self is defined top level in type alias let var = AST::new(ty.pos, Id { lit: String::from(SELF) }); let name = Some(Name::try_from(isa)?); // For now assume super - let env = identifier_from_var(&var, &name, &None, false, ctx, constr, &env)?; + let env = identifier_from_var(&var, &name, &None, false, ctx, constr, env)?; constrain_class_body(conditions, isa, &env, ctx, constr) } diff --git a/src/check/constrain/unify/mod.rs b/src/check/constrain/unify/mod.rs index 55ebdf5a0..9fd370519 100644 --- a/src/check/constrain/unify/mod.rs +++ b/src/check/constrain/unify/mod.rs @@ -29,7 +29,7 @@ pub fn unify(all_constraints: &[Constraints], ctx: &Context) -> Unified { count - 1, all_constraints.len(), newline_delimited(e.clone().into_iter().map(|e| { - let pos = e.pos.map_or_else(|| String::new(), |pos| format!(" at {pos}: ")); + let pos = e.pos.map_or_else(String::new, |pos| format!(" at {pos}: ")); format!("{pos}{}", e.msg) })) ); From f49705599832cdafd98e26d7df9b029b933721cc Mon Sep 17 00:00:00 2001 From: Joel Abrahams Date: Wed, 4 Jan 2023 20:05:29 +0100 Subject: [PATCH 22/22] Use panic to denote bugs in the check stage --- src/check/constrain/constraint/builder.rs | 10 ++++------ src/check/constrain/generate/class.rs | 2 +- src/check/constrain/generate/control_flow.rs | 4 ++-- src/check/constrain/generate/definition.rs | 2 +- src/check/constrain/generate/resources.rs | 4 ++-- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/check/constrain/constraint/builder.rs b/src/check/constrain/constraint/builder.rs index cab8b2b96..99f1ef3c3 100644 --- a/src/check/constrain/constraint/builder.rs +++ b/src/check/constrain/constraint/builder.rs @@ -4,9 +4,7 @@ use std::collections::HashMap; use crate::check::constrain::constraint::Constraint; use crate::check::constrain::constraint::expected::Expected; use crate::check::constrain::constraint::iterator::Constraints; -use crate::check::result::{TypeErr, TypeResult}; use crate::common::delimit::comma_delm; -use crate::common::position::Position; pub type VarMapping = HashMap; @@ -74,14 +72,15 @@ impl ConstrBuilder { /// /// - Error if already top-level. /// - Error if level greater than ceiling, as we cannot exit non-existent sets. - pub fn exit_set_to(&mut self, level: usize, pos: Position) -> TypeResult<()> { + pub fn exit_set_to(&mut self, level: usize) { let msg_exit = format!("Exit set to level {}", level - 1); let level = max(1, level); if level == 0 { - return Err(vec![TypeErr::new(pos, "Cannot exit top-level set")]); + panic!("Cannot exit top-level set"); } else if level > self.constraints.len() { - return Err(vec![TypeErr::new(pos, "Exiting constraint set which doesn't exist")]); + panic!("Exiting constraint set which doesn't exist\nlevel: {}, constraints: {}, finished: {}", + level, self.constraints.len(), self.finished.len()); } for i in (level - 1..self.constraints.len()).rev() { @@ -90,7 +89,6 @@ impl ConstrBuilder { } trace!("{msg_exit}: {} active sets, {} complete sets", self.constraints.len(), self.finished.len()); - Ok(()) } /// Add new constraint to constraint builder with a message. diff --git a/src/check/constrain/generate/class.rs b/src/check/constrain/generate/class.rs index 5af8082ed..49a11a8fd 100644 --- a/src/check/constrain/generate/class.rs +++ b/src/check/constrain/generate/class.rs @@ -57,6 +57,6 @@ pub fn constrain_class_body( let class_env = env.in_class(&name); gen_vec(statements, &class_env, true, ctx, constr)?; - constr.exit_set_to(class_lvl, ty.pos)?; + constr.exit_set_to(class_lvl); Ok(env.clone()) } diff --git a/src/check/constrain/generate/control_flow.rs b/src/check/constrain/generate/control_flow.rs index 4f3aea32a..c10161dfd 100644 --- a/src/check/constrain/generate/control_flow.rs +++ b/src/check/constrain/generate/control_flow.rs @@ -92,7 +92,7 @@ pub fn gen_flow( let lookup_env = gen_collection_lookup(expr, col, &col_env.is_def_mode(true), constr)?; generate(body, &lookup_env.in_loop().is_def_mode(is_define_mode), ctx, constr)?; - constr.exit_set_to(loop_lvl, ast.pos)?; + constr.exit_set_to(loop_lvl); Ok(env.clone()) } Node::While { cond, body } => { @@ -102,7 +102,7 @@ pub fn gen_flow( generate(cond, env, ctx, constr)?; generate(body, &env.in_loop(), ctx, constr)?; - constr.exit_set_to(while_lvl, ast.pos)?; + constr.exit_set_to(while_lvl); Ok(env.clone()) } diff --git a/src/check/constrain/generate/definition.rs b/src/check/constrain/generate/definition.rs index 1f3f2c5dd..4238cb22f 100644 --- a/src/check/constrain/generate/definition.rs +++ b/src/check/constrain/generate/definition.rs @@ -98,7 +98,7 @@ pub fn gen_def( return Err(unassigned.iter().map(|msg| TypeErr::new(id.pos, msg)).collect()); } - constr.exit_set_to(fun_lvl, ast.pos)?; + constr.exit_set_to(fun_lvl); Ok(env.clone()) } Node::FunArg { .. } => { diff --git a/src/check/constrain/generate/resources.rs b/src/check/constrain/generate/resources.rs index d4a3f0e10..dba4a196a 100644 --- a/src/check/constrain/generate/resources.rs +++ b/src/check/constrain/generate/resources.rs @@ -44,7 +44,7 @@ pub fn gen_resources( )?; generate(expr, &resource_env, ctx, constr)?; - constr.exit_set_to(with_lvl, ast.pos)?; + constr.exit_set_to(with_lvl); Ok(env.clone()) } Node::With { resource, expr, .. } => { @@ -57,7 +57,7 @@ pub fn gen_resources( let resource_env = generate(resource, env, ctx, constr)?; - constr.exit_set_to(with_lvl, ast.pos)?; + constr.exit_set_to(with_lvl); generate(expr, &resource_env, ctx, constr)?; Ok(env.clone()) }