Skip to content

Commit

Permalink
Handle exceptions (#382)
Browse files Browse the repository at this point in the history
* Update logic for raises in handle

We now check in the generate stage whether a
raise is covered correctly.
Looks like we might not even need it in the
unification stage.

* Add expr type to covered expressions in handle

- Fix generate stage to insert return (and assign)
  in proper locations within try except.

* Check whether parents of exception covered

Which is also valid.

Remove `Raises` `Expect`, which gets rid of a lot
of unnecessary logic in the unification stage.

* Cleanup

* Restore logic for same value of two types

* Move shared logic match, handle to constrain_cases

* Dynamically add raises to Environment

Useful if exceptions in Mamba are not dealt with
locally, but say in the function signature, which
is allowed.

* Add return in nested_exception_check.py

The fact that the test still passed means that
something is going wrong here under the hood.
An issue has been made: #386
  • Loading branch information
JSAbrahams authored Dec 27, 2022
1 parent 1b1ccce commit dfe3b63
Show file tree
Hide file tree
Showing 25 changed files with 241 additions and 160 deletions.
8 changes: 2 additions & 6 deletions src/check/constrain/constraint/expected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ pub enum Expect {
Expression { ast: AST },
Collection { ty: Box<Expected> },
Tuple { elements: Vec<Expected> },
Raises { name: Name },
Function { name: StringName, args: Vec<Expected> },
Field { name: String },
Access { entity: Box<Expected>, name: Box<Expected> },
Expand Down Expand Up @@ -106,7 +105,6 @@ impl TryFrom<(&AST, &HashMap<String, String>)> for Expect {
Node::Bool { .. } => Type { name: Name::from(BOOL) },
Node::Str { .. } => Type { name: Name::from(STRING) },
Node::Undefined => Expect::none(),
Node::Raise { error } => Raises { name: Name::try_from(error)? },
_ => Expression { ast },
})
}
Expand All @@ -130,7 +128,6 @@ impl Display for Expect {
let elements: Vec<Expected> = elements.iter().map(|a| a.and_or_a(false)).collect();
write!(f, "({})", comma_delm(elements))
}
Raises { name: ty } => write!(f, "Raises {ty}"),
Access { entity, name } => write!(f, "{}.{}", entity.and_or_a(false), name.and_or_a(false)),
Function { name, args } => {
let args: Vec<Expected> = args.iter().map(|a| a.and_or_a(false)).collect();
Expand All @@ -151,12 +148,11 @@ impl Expect {
match (self, other) {
(Collection { ty: l }, Collection { ty: r }) => l.expect.same_value(&r.expect),
(Field { name: l }, Field { name: r }) => l == r,
(Raises { name: l }, Raises { name: r }) | (Type { name: l }, Type { name: r }) => {
l == r
}
(Access { entity: le, name: ln }, Access { entity: re, name: rn }) => {
le == re && ln == rn
}
(Type { name: l }, Type { name: r }) => l == r,

(Function { name: l, args: la }, Function { name: r, args: ra }) => {
l == r
&& la.iter().zip_longest(ra.iter()).all(|pair| {
Expand Down
19 changes: 6 additions & 13 deletions src/check/constrain/generate/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::check::constrain::constraint::expected::Expect::*;
use crate::check::constrain::generate::{Constrained, gen_vec, generate};
use crate::check::constrain::generate::collection::constr_col;
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::ident::{IdentiCall, Identifier};
Expand Down Expand Up @@ -73,7 +74,7 @@ pub fn gen_call(
});

let name = Name::empty();
let parent = Expected::new(ast.pos, &Expect::Type { name });
let parent = Expected::new(ast.pos, &Type { name });
constr.add("print", &parent, &Expected::try_from((ast, &env.var_mappings))?);
return Ok((constr, env));
} else if let Some(functions) = env.get_var(&f_name.name) {
Expand Down Expand Up @@ -103,15 +104,7 @@ pub fn gen_call(
&fun_ret_exp,
);

if !fun.raises.is_empty() {
if let Some(raises) = &env.raises {
let raises_exp = Expected::new(ast.pos, &Raises { name: fun.raises });
constr.add("function call", raises, &raises_exp);
} else if !constr.is_top_level() {
let msg = format!("Exceptions not covered: {}", &fun.raises);
return Err(vec![TypeErr::new(ast.pos, &msg)]);
}
}
check_raises_caught(&constr, &fun.raises.names, &env, ctx, ast.pos)?;
}

Ok((constr, env))
Expand All @@ -125,15 +118,15 @@ pub fn gen_call(
let name = Name::from(&HashSet::from([clss::INT, clss::SLICE]));
constr.add(
"index range",
&Expected::new(range.pos, &Expect::Type { name }),
&Expected::new(range.pos, &Type { name }),
&Expected::try_from((range, &env.var_mappings))?,
);

let (temp_type, env) = env.temp_var();
let name = Name::from(temp_type.as_str());
constr.add(
"index of collection",
&Expected::new(ast.pos, &Expect::Type { name: name.clone() }),
&Expected::new(ast.pos, &Type { name: name.clone() }),
&Expected::try_from((ast, &env.var_mappings))?,
);

Expand Down Expand Up @@ -274,7 +267,7 @@ fn property_call(

let access = Expected::new(
ast_without_access.pos.union(access.pos),
&Expect::Access {
&Access {
entity: Box::new(Expected::try_from((&ast_without_access, &env.var_mappings))?),
name: Box::new(access),
},
Expand Down
72 changes: 45 additions & 27 deletions src/check/constrain/generate/control_flow.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashSet;
use std::convert::TryFrom;

use crate::check::constrain::constraint::{Constraint, ConstrVariant};
Expand All @@ -7,6 +8,7 @@ use crate::check::constrain::generate::{Constrained, generate};
use crate::check::constrain::generate::collection::gen_collection_lookup;
use crate::check::constrain::generate::env::Environment;
use crate::check::context::Context;
use crate::check::name::true_name::TrueName;
use crate::check::result::TypeErr;
use crate::parse::ast::{AST, Node};

Expand All @@ -17,10 +19,27 @@ pub fn gen_flow(
constr: &mut ConstrBuilder,
) -> Constrained {
match &ast.node {
Node::Handle { expr_or_stmt, .. } => {
let mut res = (constr.clone(), env.clone());
Node::Handle { expr_or_stmt, cases } => {
let raises: HashSet<TrueName> = cases.iter().flat_map(|c| match &c.node {
Node::Case { cond, .. } => {
match &cond.node {
Node::ExpressionType { ty: Some(ty), .. } => if let Node::Type { .. } = &ty.node {
if let Ok(name) = TrueName::try_from(ty) {
Some(name)
} else { None }
} else { None }
_ => None
}
}
_ => None
}).collect();

let raises_before = env.raises_caught.clone();
let (mut constr, outer_env) = generate(expr_or_stmt, &env.raises_caught(&raises), ctx, constr)?;
let outer_env = outer_env.raises_caught(&raises_before);

generate(expr_or_stmt, &res.1, ctx, &mut res.0)
constrain_cases(ast, cases, &outer_env, ctx, &mut constr)?;
Ok((constr, outer_env))
}

Node::IfElse { cond, then, el: Some(el) } => {
Expand Down Expand Up @@ -65,30 +84,7 @@ pub fn gen_flow(
Node::Match { cond, cases } => {
let (mut constr, outer_env) = generate(cond, &env, ctx, constr)?;

for case in cases {
match &case.node {
Node::Case { cond, body } => {
let define_mode = outer_env.is_define_mode;
constr.new_set(true);

let (mut inner_constr, cond_env) = generate(cond, &outer_env.define_mode(true), ctx, &mut constr)?;
let cond_env = cond_env.define_mode(define_mode);

inner_constr.add(
"match body",
&Expected::try_from((body, &cond_env.var_mappings))?,
&Expected::try_from((ast, &outer_env.var_mappings))?,
);

let (inner_constr, _) = generate(body, &cond_env, ctx, &mut inner_constr)?;
constr = inner_constr;

constr.exit_set(case.pos)?;
}
_ => return Err(vec![TypeErr::new(case.pos, "Expected case")])
}
}

constrain_cases(ast, cases, &outer_env, ctx, &mut constr)?;
Ok((constr, outer_env))
}

Expand Down Expand Up @@ -126,6 +122,28 @@ pub fn gen_flow(
}
}

fn constrain_cases(ast: &AST, cases: &Vec<AST>, env: &Environment, ctx: &Context, constr: &mut ConstrBuilder) -> Constrained<()> {
let is_define_mode = env.is_define_mode;
let exp_ast = Expected::try_from((ast, &env.var_mappings))?;

for case in cases {
match &case.node {
Node::Case { cond, body } => {
constr.new_set(true);

let (_, cond_env) = generate(cond, &env.define_mode(true), ctx, constr)?;
let (_, body_env) = generate(body, &cond_env.define_mode(is_define_mode), ctx, constr)?;

let exp_body = Expected::try_from((body, &body_env.var_mappings))?;
constr.add("handle arm body", &exp_body, &exp_ast);
constr.exit_set(case.pos)?;
}
_ => return Err(vec![TypeErr::new(case.pos, "Expected case")])
}
}
Ok(())
}

#[cfg(test)]
mod tests {
use crate::check::constrain::constraint::builder::ConstrBuilder;
Expand Down
34 changes: 21 additions & 13 deletions src/check/constrain/generate/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use crate::check::context::arg::SELF;
use crate::check::context::clss::HasParent;
use crate::check::context::field::Field;
use crate::check::ident::Identifier;
use crate::check::name::{Any, Empty, match_name, Name, Nullable, Union};
use crate::check::name::{Any, match_name, Name, Nullable};
use crate::check::name::true_name::TrueName;
use crate::check::result::{TypeErr, TypeResult};
use crate::common::position::Position;
use crate::parse::ast::{AST, Node};
Expand Down Expand Up @@ -49,21 +50,28 @@ pub fn gen_def(
let (mut constr, inner_env) = constrain_args(fun_args, env, ctx, constr)?;
let inner_env = inner_env.with_unassigned(non_nullable_class_vars);

let (mut constr, inner_env) = if let Some(body) = body {
let r_tys: Vec<_> = raises.iter().map(|r| (r.pos, Name::try_from(r))).collect();
let mut r_res = Name::empty();
let (raises, errs): (Vec<(Position, _)>, Vec<_>) = raises
.iter()
.map(|r| (r.pos, TrueName::try_from(r)))
.partition(|(_, res)| res.is_ok());
if !errs.is_empty() {
let errs = errs.into_iter().flat_map(|(_, e)| e.unwrap_err());
return Err(errs.collect());
}

let exception_name = Name::from(clss::EXCEPTION);
for (pos, raise) in r_tys {
let raise = raise?;
if !ctx.class(&raise, pos)?.has_parent(&exception_name, ctx, pos)? {
let msg = format!("`{}` is not an `{}`", raise, exception_name);
return Err(vec![TypeErr::new(pos, &msg)]);
}
r_res = r_res.union(&raise)
let exception_name = Name::from(clss::EXCEPTION);
for (pos, raise) in &raises {
let raise = raise.clone()?;
if !ctx.class(&raise, *pos)?.has_parent(&exception_name, ctx, *pos)? {
let msg = format!("`{raise}` is not an `{exception_name}`");
return Err(vec![TypeErr::new(*pos, &msg)]);
}
}

let raises = raises.into_iter().map(|(_, r)| r.unwrap()).collect();
let inner_env = inner_env.raises_caught(&raises);

let inner_env = inner_env.insert_raises(&r_res, ast.pos);
let (mut constr, inner_env) = if let Some(body) = body {
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() });
Expand Down
22 changes: 10 additions & 12 deletions src/check/constrain/generate/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ use std::collections::{HashMap, HashSet};
use std::iter::FromIterator;

use crate::check::constrain::constraint::expected::{Expect, Expected};
use crate::check::constrain::constraint::expected::Expect::Raises;
use crate::check::context::arg::SELF;
use crate::check::name;
use crate::check::name::{Empty, Name};
use crate::common::position::Position;
use crate::check::name::true_name::TrueName;

#[derive(Clone, Debug, Default)]
pub struct Environment {
Expand All @@ -15,7 +13,9 @@ pub struct Environment {
pub last_stmt_in_function: bool,
pub is_define_mode: bool,
pub return_type: Option<Expected>,
pub raises: Option<Expected>,

pub raises_caught: HashSet<TrueName>,

pub class_type: Option<Expect>,
pub var_mappings: HashMap<String, String>,
pub unassigned: HashSet<String>,
Expand Down Expand Up @@ -71,14 +71,12 @@ impl Environment {
Environment { vars, ..self.clone() }
}

/// Insert raises.
pub fn insert_raises(&self, raises: &Name, pos: Position) -> Environment {
if raises.is_empty() {
self.clone()
} else {
let raises = Expected::new(pos, &Raises { name: raises.clone() });
Environment { raises: Some(raises), ..self.clone() }
}
/// Insert raises which are properly handled.
///
/// Appends to current set.
pub fn raises_caught(&self, raises: &HashSet<TrueName>) -> Environment {
let raises_caught = self.raises_caught.union(raises).cloned().collect();
Environment { raises_caught, ..self.clone() }
}

/// Specify that we are in a loop.
Expand Down
36 changes: 2 additions & 34 deletions src/check/constrain/generate/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use std::convert::TryFrom;

use crate::check::constrain::constraint::builder::ConstrBuilder;
use crate::check::constrain::constraint::expected::{Expect, Expected};
use crate::check::constrain::constraint::expected::Expect::{Raises, Type};
use crate::check::constrain::constraint::expected::Expect::Type;
use crate::check::constrain::generate::{Constrained, generate};
use crate::check::constrain::generate::definition::identifier_from_var;
use crate::check::constrain::generate::env::Environment;
use crate::check::context::Context;
use crate::check::name::{Any, Name};
use crate::check::result::{TypeErr, TypeResult};
use crate::check::result::TypeErr;
use crate::parse::ast::{AST, Node};

pub fn gen_resources(
Expand All @@ -18,16 +18,6 @@ pub fn gen_resources(
constr: &mut ConstrBuilder,
) -> Constrained {
match &ast.node {
Node::Raises { expr_or_stmt, errors } => {
let mut constr = constr.clone();
for error in errors {
let exp = Expected::new(error.pos, &Raises { name: Name::try_from(error)? });
constr = constrain_raises(&exp, &env.raises, &mut constr)?;
}
// raises expression has type of contained expression
constr.add("raises", &Expected::try_from((ast, &env.var_mappings))?, &Expected::try_from((expr_or_stmt, &env.var_mappings))?);
generate(expr_or_stmt, env, ctx, &mut constr)
}
Node::With { resource, alias: Some((alias, mutable, ty)), expr } => {
constr.new_set(true);
let resource_exp = Expected::try_from((resource, &env.var_mappings))?;
Expand Down Expand Up @@ -73,25 +63,3 @@ pub fn gen_resources(
_ => Err(vec![TypeErr::new(ast.pos, "Expected resources")])
}
}

/// Constrain expected to raises
///
/// This indicates that the type should be contained within the set of the
/// raises constraint. Does not constrain if top-level in constr builder,
/// meaning that we do not constrain raises in top-level scripts.
pub fn constrain_raises(
raises: &Expected,
env_raises: &Option<Expected>,
constr: &mut ConstrBuilder,
) -> TypeResult<ConstrBuilder> {
if constr.level == 0 {
return Ok(constr.clone());
}

if let Some(env_raises) = env_raises {
constr.add("raises", env_raises, raises);
Ok(constr.clone())
} else {
Err(vec![TypeErr::new(raises.pos, "Unexpected raise")])
}
}
Loading

0 comments on commit dfe3b63

Please sign in to comment.