DeCarabas/oden
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

[fine] All error sentinels carry diagnostics

Browse source 
The system has an invariant that if you ever return an error
sentinel (error environment, error type) then that sentinel is caused
by an error that was reported to the user. We have had too many bugs
over the last little while where that was not the case!

(An example is if we mis-interpret the tree by calling `nth_tree` with
the wrong index or something, and get `None`, and think "oh must be a
syntax error", but it was really just the wrong index. Then there's an
error sentinel with no error diagnostic and we don't discover the
mistake until much farther along.)

Now we enforce this by requiring that whoever constructs the error
sentinel *prove* that they can do so by providing a diagnostic. It's
less efficient but prevents the problem.

This actually uncovered a couple of latent bugs where we were
generating error sentinels instead of a more appropriate type! Whoops!
This commit is contained in:
John Doty 2024-03-25 08:07:18 -07:00
parent 8779aade24
commit 85ffc0c7dd
6 changed files with 276 additions and 170 deletions
Download patch file Download diff file Expand all files Collapse all files

22
fine/src/compiler.rs
View file

@ -3,7 +3,7 @@ use std::rc::Rc;
use crate::{ use crate::{
parser::{Child, SyntaxTree, Tree, TreeKind, TreeRef}, parser::{Child, SyntaxTree, Tree, TreeKind, TreeRef},
semantics::{string_constant_to_string, Declaration, Environment, Location, Semantics, Type}, semantics::{string_constant_to_string, Declaration, Location, Semantics, Type},
tokens::TokenKind, tokens::TokenKind,
}; };
@ -366,7 +366,7 @@ fn compile_literal(c: &mut Compiler, t: TreeRef, tr: &Tree) -> CR {
let index = c.add_string(result); let index = c.add_string(result);
c.push(Instruction::PushString(index)) c.push(Instruction::PushString(index))
} }
Type::Error => c.push_panic(format!("compiling literal {:?}", tr)), Type::Error(e) => c.push_panic(format!("compiling literal {:?}: {e}", tr)),
_ => ice!(c, t, "unsupported literal type: {t:?}"), _ => ice!(c, t, "unsupported literal type: {t:?}"),
}; };
OK OK
@ -556,22 +556,30 @@ fn compile_binary_expression(c: &mut Compiler, t: TreeRef, tr: &Tree) -> CR {
let ltree = &c.syntax[lvalue]; let ltree = &c.syntax[lvalue];
#[allow(unused_assignments)] #[allow(unused_assignments)]
let mut environment = Environment::error("??"); let mut environment = None;
let declaration = match ltree.kind { let declaration = match ltree.kind {
// TODO: Assign to list access // TODO: Assign to list access
TreeKind::Identifier => { TreeKind::Identifier => {
let id = ltree.nth_token(0).ok_or("no id")?.as_str(&c.source); let id = ltree.nth_token(0).ok_or("no id")?.as_str(&c.source);
environment = c.semantics.environment_of(lvalue); environment = Some(c.semantics.environment_of(lvalue));
environment.bind(id).ok_or("cannot bind destination")? environment
.as_ref()
.unwrap()
.bind(id)
.ok_or("cannot bind destination")?
} }
TreeKind::MemberAccess => { TreeKind::MemberAccess => {
let id = ltree.nth_token(2).ok_or("no member")?.as_str(&c.source); let id = ltree.nth_token(2).ok_or("no member")?.as_str(&c.source);
let t = ltree.nth_tree(0).ok_or("no lhs exp")?; let t = ltree.nth_tree(0).ok_or("no lhs exp")?;
let typ = c.semantics.type_of(t); let typ = c.semantics.type_of(t);
environment = c.semantics.member_environment(t, &typ); environment = Some(c.semantics.member_environment(t, &typ));
environment.bind(id).ok_or("cannot bind field")? environment
.as_ref()
.unwrap()
.bind(id)
.ok_or("cannot bind field")?
} }
_ => return Err("unsupported lval expression"), _ => return Err("unsupported lval expression"),
}; };

5
fine/src/lib.rs
View file

@ -86,7 +86,10 @@ impl Runtime {
} }
} }
pub fn load_module(&mut self, name: &str) -> Result<(Vec<Error>, Rc<Module>), ModuleLoadError> { pub fn load_module(
&mut self,
name: &str,
) -> Result<(Vec<Rc<Error>>, Rc<Module>), ModuleLoadError> {
let mut init_pending = HashMap::new(); let mut init_pending = HashMap::new();
let mut names = Vec::new(); let mut names = Vec::new();
let name = self.loader.normalize_module_name("", name.to_string()); let name = self.loader.normalize_module_name("", name.to_string());

3
fine/src/parser.rs
View file

@ -26,6 +26,7 @@ impl SyntaxTree {
assert!(t.parent.is_none()); assert!(t.parent.is_none());
let tr = TreeRef::from_index(self.trees.len()); let tr = TreeRef::from_index(self.trees.len());
t.self_ref = tr;
t.start_pos = t t.start_pos = t
.children .children
.first() .first()
@ -172,6 +173,7 @@ pub enum TreeKind {
} }
pub struct Tree { pub struct Tree {
pub self_ref: TreeRef,
pub kind: TreeKind, pub kind: TreeKind,
pub parent: Option<TreeRef>, // TODO: Do we actually need this? pub parent: Option<TreeRef>, // TODO: Do we actually need this?
pub start_pos: usize, pub start_pos: usize,
@ -538,6 +540,7 @@ impl<'a> CParser<'a> {
match event { match event {
ParseEvent::Start { kind } => stack.push(Tree { ParseEvent::Start { kind } => stack.push(Tree {
kind, kind,
self_ref: TreeRef::from_index(0),
parent: None, parent: None,
start_pos: 0, start_pos: 0,
end_pos: 0, end_pos: 0,

397
fine/src/semantics.rs
View file

@ -26,11 +26,12 @@ pub struct Error {
pub file: Rc<str>, pub file: Rc<str>,
pub start: (usize, usize), pub start: (usize, usize),
pub end: (usize, usize), pub end: (usize, usize),
pub span: (usize, usize),
pub message: String, pub message: String,
} }
impl Error { impl Error {
pub fn new<T>(file: Rc<str>, line: usize, column: usize, message: T) -> Self pub fn new<T>(file: Rc<str>, line: usize, column: usize, pos: usize, message: T) -> Self
where where
T: ToString, T: ToString,
{ {
@ -38,6 +39,7 @@ impl Error {
file, file,
start: (line, column), start: (line, column),
end: (line, column), end: (line, column),
span: (pos, pos),
message: message.to_string(), message: message.to_string(),
} }
} }
@ -46,6 +48,7 @@ impl Error {
file: Rc<str>, file: Rc<str>,
start: (usize, usize), start: (usize, usize),
end: (usize, usize), end: (usize, usize),
span: (usize, usize),
message: T, message: T,
) -> Self ) -> Self
where where
@ -55,6 +58,7 @@ impl Error {
file, file,
start, start,
end, end,
span,
message: message.to_string(), message: message.to_string(),
} }
} }
@ -137,7 +141,7 @@ pub enum Type {
// Signals a type error. If you receive this then you know that an error // Signals a type error. If you receive this then you know that an error
// has already been reported; if you produce this be sure to also note // has already been reported; if you produce this be sure to also note
// the error in the errors collection. // the error in the errors collection.
Error, Error(Rc<Error>),
// Signals that the expression has a control-flow side-effect and that no // Signals that the expression has a control-flow side-effect and that no
// value will ever result from this expression. Usually this means // value will ever result from this expression. Usually this means
@ -190,14 +194,14 @@ pub enum Type {
impl Type { impl Type {
pub fn is_error(&self) -> bool { pub fn is_error(&self) -> bool {
match self { match self {
Type::Error => true, Type::Error(..) => true,
_ => false, _ => false,
} }
} }
fn discriminant_number(&self) -> i8 { fn discriminant_number(&self) -> i8 {
match self { match self {
Type::Error => 0, Type::Error(..) => 0,
Type::Unreachable => 1, Type::Unreachable => 1,
Type::Assignment(..) => 2, Type::Assignment(..) => 2,
Type::TypeVariable(..) => 3, Type::TypeVariable(..) => 3,
@ -227,7 +231,7 @@ impl fmt::Display for Type {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use Type::*; use Type::*;
match self { match self {
Error => write!(f, "<< INTERNAL ERROR >>"), Error(e) => write!(f, "<< INTERNAL ERROR ({e}) >>"),
Unreachable => write!(f, "<< UNREACHABLE >>"), Unreachable => write!(f, "<< UNREACHABLE >>"),
Assignment(_) => write!(f, "assignment"), Assignment(_) => write!(f, "assignment"),
Nothing => write!(f, "nothing"), Nothing => write!(f, "nothing"),
@ -431,7 +435,7 @@ pub struct Environment {
pub location: Location, pub location: Location,
pub next_index: usize, pub next_index: usize,
pub declarations: HashMap<Box<str>, Declaration>, pub declarations: HashMap<Box<str>, Declaration>,
pub error: Option<&'static str>, pub error: Option<Rc<Error>>,
} }
impl Environment { impl Environment {
@ -465,7 +469,7 @@ impl Environment {
self.error.is_some() self.error.is_some()
} }
pub fn error(why: &'static str) -> EnvironmentRef { pub fn error(why: Rc<Error>) -> EnvironmentRef {
// TODO: Exactly once? // TODO: Exactly once?
EnvironmentRef::new(Environment { EnvironmentRef::new(Environment {
parent: None, parent: None,
@ -709,7 +713,7 @@ pub struct Semantics {
logical_parents: Vec<Option<TreeRef>>, logical_parents: Vec<Option<TreeRef>>,
// TODO: State should be externalized instead of this refcell nonsense. // TODO: State should be externalized instead of this refcell nonsense.
errors: RefCell<Vec<Error>>, errors: RefCell<Vec<Rc<Error>>>,
types: RefCell<Vec<Incremental<Type>>>, types: RefCell<Vec<Incremental<Type>>>,
environments: RefCell<Vec<Incremental<EnvironmentRef>>>, environments: RefCell<Vec<Incremental<EnvironmentRef>>>,
root_environment: EnvironmentRef, root_environment: EnvironmentRef,
@ -791,12 +795,9 @@ impl Semantics {
.unwrap_or(Vec::new()) .unwrap_or(Vec::new())
} }
pub fn snapshot_errors(&self) -> Vec<Error> { pub fn snapshot_errors(&self) -> Vec<Rc<Error>> {
let mut result = (*self.errors.borrow()).clone(); let mut result = (*self.errors.borrow()).clone();
result.sort_by(|a, b| match a.start.0.cmp(&b.start.0) { result.sort_by_key(|a| a.span.0);
std::cmp::Ordering::Equal => a.start.1.cmp(&b.start.1),
o => o,
});
result result
} }
@ -808,28 +809,31 @@ impl Semantics {
} }
} }
fn report_error_span<T>(&self, start: usize, end: usize, error: T) fn report_error_span<T>(&self, start_pos: usize, end_pos: usize, error: T) -> Rc<Error>
where where
T: ToString, T: ToString,
{ {
let start = self.lines.position(start); let start = self.lines.position(start_pos);
let end = self.lines.position(end); let end = self.lines.position(end_pos);
self.errors.borrow_mut().push(Error::new_spanned( let error = Rc::new(Error::new_spanned(
self.file.clone(), self.file.clone(),
start, start,
end, end,
(start_pos, end_pos),
error.to_string(), error.to_string(),
)); ));
self.errors.borrow_mut().push(error.clone());
error
} }
fn report_error_tree<T>(&self, tree: &Tree, error: T) fn report_error_tree<T>(&self, tree: &Tree, error: T) -> Rc<Error>
where where
T: ToString, T: ToString,
{ {
self.report_error_span(tree.start_pos, tree.end_pos, error) self.report_error_span(tree.start_pos, tree.end_pos, error)
} }
fn report_error_tree_ref<T>(&self, tree: TreeRef, error: T) fn report_error_tree_ref<T>(&self, tree: TreeRef, error: T) -> Rc<Error>
where where
T: ToString, T: ToString,
{ {
@ -854,6 +858,52 @@ impl Semantics {
} }
} }
// TODO: Here we're just looking for *an* error, not the most specific
// error.
fn find_error(&self, tree: &Tree) -> Option<Rc<Error>> {
let mut result = (*self.errors.borrow()).clone();
result.sort_by_key(|a| a.span.0);
let mut error = None;
for candidate in result.into_iter() {
let (candiate_start, candidate_end) = candidate.span;
if candidate_end < tree.start_pos {
continue;
}
if candiate_start > tree.end_pos {
break;
}
// End is after our point, Start is before our point, we are
// inside. This error at least affects us somehow.
error = Some(candidate);
}
error
}
fn type_error_for(&self, tree: &Tree) -> Type {
let Some(error) = self.find_error(&tree) else {
self.internal_compiler_error(
Some(tree.self_ref),
"Unable to find a diagnostic that encompasses the tree generating an error type",
);
};
Type::Error(error)
}
fn environment_error_for(&self, tree: &Tree) -> EnvironmentRef {
let Some(error) = self.find_error(&tree) else {
self.internal_compiler_error(
Some(tree.self_ref),
"Unable to find a diagnostic that encompasses the tree generating an error environment",
);
};
Environment::error(error)
}
pub fn environment_of(&self, t: TreeRef) -> EnvironmentRef { pub fn environment_of(&self, t: TreeRef) -> EnvironmentRef {
{ {
// I want to make sure that this borrow is dropped after this block. // I want to make sure that this borrow is dropped after this block.
@ -1141,12 +1191,12 @@ impl Semantics {
let Some(pattern) = tree.child_tree_of_kind(&self.syntax_tree, TreeKind::Pattern) else { let Some(pattern) = tree.child_tree_of_kind(&self.syntax_tree, TreeKind::Pattern) else {
// Should really have a pattern in there; otherwise there was a // Should really have a pattern in there; otherwise there was a
// parse error, don't make more trouble. // parse error, don't make more trouble.
return Environment::error("no rhs (pattern)"); return self.environment_error_for(tree);
}; };
// The left hand side of the `is` expression is used for wildcard types. // The left hand side of the `is` expression is used for wildcard types.
let Some(lhs) = tree.nth_tree(0) else { let Some(lhs) = tree.nth_tree(0) else {
return Environment::error("no lhs (value)"); return self.environment_error_for(tree);
}; };
self.environment_of_pattern(parent, pattern, lhs) self.environment_of_pattern(parent, pattern, lhs)
} }
@ -1163,7 +1213,7 @@ impl Semantics {
let Some(pattern) = tree.child_tree_of_kind(&self.syntax_tree, TreeKind::Pattern) else { let Some(pattern) = tree.child_tree_of_kind(&self.syntax_tree, TreeKind::Pattern) else {
// Should really have a pattern in there; otherwise there was a // Should really have a pattern in there; otherwise there was a
// parse error, don't make more trouble. // parse error, don't make more trouble.
return Environment::error("arm: no lhs (pattern)"); return self.environment_error_for(tree);
}; };
// The expression in the match expression is the binding for the wildcard pattern. // The expression in the match expression is the binding for the wildcard pattern.
@ -1185,7 +1235,7 @@ impl Semantics {
// The expression is the first tree child of match expression. // The expression is the first tree child of match expression.
let Some(lhs) = tree.nth_tree(2) else { let Some(lhs) = tree.nth_tree(2) else {
return Environment::error("arm: no rhs (expression)"); return self.environment_error_for(tree);
}; };
self.environment_of_pattern(parent, pattern, lhs) self.environment_of_pattern(parent, pattern, lhs)
} }
@ -1205,7 +1255,7 @@ impl Semantics {
return parent; return parent;
}; };
let Some(variable) = binding.nth_token(0) else { let Some(variable) = binding.nth_token(0) else {
return Environment::error("no variable"); return self.environment_error_for(binding);
}; };
let is_wildcard = tree let is_wildcard = tree
@ -1222,7 +1272,7 @@ impl Semantics {
// match for the variable to have a value. // match for the variable to have a value.
let Some(type_expr) = tree.child_of_kind(&self.syntax_tree, TreeKind::TypeExpression) let Some(type_expr) = tree.child_of_kind(&self.syntax_tree, TreeKind::TypeExpression)
else { else {
return Environment::error("no type expression"); return self.environment_error_for(tree);
}; };
type_expr type_expr
}; };
@ -1283,7 +1333,7 @@ impl Semantics {
let field_type = f let field_type = f
.nth_tree(2) .nth_tree(2)
.map(|t| self.type_of(t)) .map(|t| self.type_of(t))
.unwrap_or(Type::Error); .unwrap_or_else(|| self.type_error_for(f));
fields.push(FieldDecl { fields.push(FieldDecl {
name: field_name.as_str(&self.source).into(), name: field_name.as_str(&self.source).into(),
declaration: field, declaration: field,
@ -1430,8 +1480,8 @@ impl Semantics {
} }
// Avoid introducing more errors // Avoid introducing more errors
(Type::Error, _) => true, (Type::Error(_), _) => true,
(_, Type::Error) => true, (_, Type::Error(_)) => true,
// Can... I... convert unreachable always? Is this sound? // Can... I... convert unreachable always? Is this sound?
(Type::Unreachable, _) => true, (Type::Unreachable, _) => true,
@ -1468,9 +1518,11 @@ impl Semantics {
Incremental::Complete(existing) => return existing.clone(), Incremental::Complete(existing) => return existing.clone(),
Incremental::InProgress => { Incremental::InProgress => {
// eprintln!("type_of circular => {t:?}"); // eprintln!("type_of circular => {t:?}");
self.report_error_tree_ref(t, "The type of this expression depends on itself"); let error = self
*state = Incremental::Complete(Type::Error); .report_error_tree_ref(t, "The type of this expression depends on itself");
return Type::Error; let e_type = Type::Error(error);
*state = Incremental::Complete(e_type.clone());
return e_type;
} }
} }
*state = Incremental::InProgress; *state = Incremental::InProgress;
@ -1480,7 +1532,7 @@ impl Semantics {
// eprintln!(">>> type_of => {tree:?}"); // eprintln!(">>> type_of => {tree:?}");
let result = match tree.kind { let result = match tree.kind {
TreeKind::Error => Some(Type::Error), TreeKind::Error => Some(self.type_error_for(tree)),
TreeKind::AlternateType => self.type_of_alternate_type(tree), TreeKind::AlternateType => self.type_of_alternate_type(tree),
TreeKind::Argument => self.type_of_argument(tree), TreeKind::Argument => self.type_of_argument(tree),
@ -1525,7 +1577,7 @@ impl Semantics {
}; };
// NOTE: These return `None` if they encounter some problem. // NOTE: These return `None` if they encounter some problem.
let result = result.unwrap_or(Type::Error); let result = result.unwrap_or_else(|| self.type_error_for(tree));
self.types.borrow_mut()[t.index()] = Incremental::Complete(result.clone()); self.types.borrow_mut()[t.index()] = Incremental::Complete(result.clone());
// eprintln!("<<< type_of => {tree:?}"); // eprintln!("<<< type_of => {tree:?}");
@ -1546,19 +1598,19 @@ impl Semantics {
// This is dumb and should be punished, probably. // This is dumb and should be punished, probably.
(_, Type::Unreachable) => { (_, Type::Unreachable) => {
self.report_error_span( let err = self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
"cannot apply a unary operator to something that doesn't yield a value", "cannot apply a unary operator to something that doesn't yield a value",
); );
Some(Type::Error) Some(Type::Error(err))
} }
// Propagate existing errors without additional complaint. // Propagate existing errors without additional complaint.
(_, Type::Error) => Some(Type::Error), (_, Type::Error(e)) => Some(Type::Error(e)),
(_, arg_type) => { (_, arg_type) => {
self.report_error_span( let err = self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
format!( format!(
@ -1567,7 +1619,7 @@ impl Semantics {
arg_type arg_type
), ),
); );
Some(Type::Error) Some(Type::Error(err))
} }
} }
} }
@ -1607,7 +1659,7 @@ impl Semantics {
// This is dumb and should be punished, probably. // This is dumb and should be punished, probably.
(_, _, Type::Unreachable) => { (_, _, Type::Unreachable) => {
self.report_error_span( let err = self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
format!( format!(
@ -1615,10 +1667,10 @@ impl Semantics {
op.as_str(&self.source) op.as_str(&self.source)
), ),
); );
Some(Type::Error) Some(Type::Error(err))
} }
(_, Type::Unreachable, _) => { (_, Type::Unreachable, _) => {
self.report_error_span( let err = self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
format!( format!(
@ -1626,19 +1678,19 @@ impl Semantics {
op.as_str(&self.source) op.as_str(&self.source)
), ),
); );
Some(Type::Error) Some(Type::Error(err))
} }
// Propagate existing errors without additional complaint. // Propagate existing errors without additional complaint.
(_, Type::Error, _) => Some(Type::Error), (_, Type::Error(e), _) => Some(Type::Error(e)),
(_, _, Type::Error) => Some(Type::Error), (_, _, Type::Error(e)) => Some(Type::Error(e)),
// Assignments are fun. // Assignments are fun.
(TokenKind::Equal, a, b) => self.type_of_assignment(left_tree, a, b, op), (TokenKind::Equal, a, b) => self.type_of_assignment(left_tree, a, b, op),
// Missed the whole table, it must be an error. // Missed the whole table, it must be an error.
(_, left_type, right_type) => { (_, left_type, right_type) => {
self.report_error_span( let err =self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
format!( format!(
@ -1646,7 +1698,7 @@ impl Semantics {
op.as_str(&self.source) op.as_str(&self.source)
), ),
); );
Some(Type::Error) Some(Type::Error(err))
} }
} }
} }
@ -1662,75 +1714,79 @@ impl Semantics {
let tree = &self.syntax_tree[left_tree]; let tree = &self.syntax_tree[left_tree];
#[allow(unused_assignments)] #[allow(unused_assignments)]
let mut environment = Environment::error("?"); let mut environment = None;
let declaration = match tree.kind { let declaration = match tree.kind {
// TODO: Assign to list access // TODO: Assign to list access
TreeKind::Identifier => { TreeKind::Identifier => {
let id = tree.nth_token(0)?.as_str(&self.source); let id = tree.nth_token(0)?.as_str(&self.source);
environment = self.environment_of(left_tree); environment = Some(self.environment_of(left_tree));
match environment.bind(id) { match environment.as_ref().unwrap().bind(id) {
Some(decl) => decl, Some(decl) => decl,
None => { None => {
if !environment.is_error() { let error = if let Some(e) = &environment.as_ref().unwrap().error {
self.report_error_tree(tree, format!("cannot find value {id} here")); e.clone()
} } else {
return Some(Type::Error); self.report_error_tree(tree, format!("cannot find value {id} here"))
};
return Some(Type::Error(error));
} }
} }
} }
TreeKind::MemberAccess => { TreeKind::MemberAccess => {
let id = tree.nth_token(2)?.as_str(&self.source); let id = tree.nth_token(2)?.as_str(&self.source);
let typ = self.type_of(tree.nth_tree(0)?); let typ = self.type_of(tree.nth_tree(0)?);
environment = self.member_environment(left_tree, &typ); environment = Some(self.member_environment(left_tree, &typ));
match environment.bind(id) { match environment.as_ref().unwrap().bind(id) {
Some(decl) => decl, Some(decl) => decl,
None => { None => {
if !environment.is_error() { let error = if let Some(e) = &environment.as_ref().unwrap().error {
self.report_error_tree(tree, format!("'{typ}' has no member {id}")); e.clone()
} } else {
return Some(Type::Error); self.report_error_tree(tree, format!("'{typ}' has no member {id}"))
};
return Some(Type::Error(error));
} }
} }
} }
_ => { _ => {
self.report_error_tree_ref( let error = self.report_error_tree_ref(
left_tree, left_tree,
"cannot assign a value to this expression, it is not a place you can store things", "cannot assign a value to this expression, it is not a place you can store things",
); );
return Some(Type::Error); return Some(Type::Error(error));
} }
}; };
match declaration { match declaration {
Declaration::Variable { .. } => (), Declaration::Variable { .. } => (),
Declaration::ExternFunction { .. } | Declaration::Function { .. } => { Declaration::ExternFunction { .. } | Declaration::Function { .. } => {
self.report_error_tree_ref( let error = self.report_error_tree_ref(
left_tree, left_tree,
"cannot assign a new value to a function declaration", "cannot assign a new value to a function declaration",
); );
return Some(Type::Error); return Some(Type::Error(error));
} }
Declaration::Class { .. } => { Declaration::Class { .. } => {
self.report_error_tree_ref( let error = self.report_error_tree_ref(
left_tree, left_tree,
"cannot assign a new value to a class declaration", "cannot assign a new value to a class declaration",
); );
return Some(Type::Error); return Some(Type::Error(error));
} }
Declaration::ImportedModule { .. } => { Declaration::ImportedModule { .. } => {
self.report_error_tree_ref( let error = self.report_error_tree_ref(
left_tree, left_tree,
"cannot assign a new value to an imported module", "cannot assign a new value to an imported module",
); );
return Some(Type::Error); return Some(Type::Error(error));
} }
Declaration::ImportedDeclaration { .. } => { Declaration::ImportedDeclaration { .. } => {
self.report_error_tree_ref( let error = self.report_error_tree_ref(
left_tree, left_tree,
"cannot assign a new value to a member of an imported module", "cannot assign a new value to a member of an imported module",
); );
return Some(Type::Error); return Some(Type::Error(error));
} }
} }
@ -1745,17 +1801,19 @@ impl Semantics {
t => t, t => t,
}; };
if left_type.is_error() || right_type.is_error() { if let Type::Error(e) = left_type {
Some(Type::Error) Some(Type::Error(e))
} else if let Type::Error(e) = right_type {
Some(Type::Error(e))
} else if self.can_convert(&right_type, &left_type) { } else if self.can_convert(&right_type, &left_type) {
Some(Type::Assignment(Box::new(left_type))) Some(Type::Assignment(Box::new(left_type)))
} else { } else {
self.report_error_span( let error = self.report_error_span(
op.start(), op.start(),
op.end(), op.end(),
format!("cannot assign a value of type '{right_type}' to type '{left_type}'"), format!("cannot assign a value of type '{right_type}' to type '{left_type}'"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
} }
@ -1780,8 +1838,8 @@ impl Semantics {
let mut arg_types: Vec<_> = args.child_trees().map(|t| self.type_of(t)).collect(); let mut arg_types: Vec<_> = args.child_trees().map(|t| self.type_of(t)).collect();
if arg_types.len() != 1 { if arg_types.len() != 1 {
self.report_error_tree(tree, "list takes a single type argument"); let error = self.report_error_tree(tree, "list takes a single type argument");
Some(Type::Error) Some(Type::Error(error))
} else { } else {
Some(Type::List(Box::new(arg_types.pop().unwrap()))) Some(Type::List(Box::new(arg_types.pop().unwrap())))
} }
@ -1793,25 +1851,25 @@ impl Semantics {
Some(self.type_of(*declaration)) Some(self.type_of(*declaration))
} }
Some(Declaration::Variable { .. }) => { Some(Declaration::Variable { .. }) => {
self.report_error_tree( let error = self.report_error_tree(
tree, tree,
format!("'{token}' is a variable and cannot be used as a type"), format!("'{token}' is a variable and cannot be used as a type"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
Some(Declaration::Function { .. } | Declaration::ExternFunction { .. }) => { Some(Declaration::Function { .. } | Declaration::ExternFunction { .. }) => {
self.report_error_tree( let error = self.report_error_tree(
tree, tree,
format!("'{token}' is a function and cannot be used as a type"), format!("'{token}' is a function and cannot be used as a type"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
Some(Declaration::ImportedModule { .. }) => { Some(Declaration::ImportedModule { .. }) => {
self.report_error_tree( let error = self.report_error_tree(
tree, tree,
format!("'{token}' is an imported module and cannot be used as a type"), format!("'{token}' is an imported module and cannot be used as a type"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
Some(Declaration::ImportedDeclaration { Some(Declaration::ImportedDeclaration {
semantics, semantics,
@ -1824,10 +1882,12 @@ impl Semantics {
.type_of_declaration(*tree, declaration), .type_of_declaration(*tree, declaration),
), ),
None => { None => {
if !environment.is_error() { let error = if let Some(e) = &environment.error {
self.report_error_tree(tree, format!("Unrecognized type: '{token}'")); e.clone()
} } else {
Some(Type::Error) self.report_error_tree(tree, format!("Unrecognized type: '{token}'"))
};
Some(Type::Error(error))
} }
} }
} }
@ -1918,8 +1978,8 @@ impl Semantics {
}; };
match (then_type, else_type) { match (then_type, else_type) {
(Type::Error, _) => Some(Type::Error), (Type::Error(e), _) => Some(Type::Error(e)),
(_, Some(Type::Error)) => Some(Type::Error), (_, Some(Type::Error(e))) => Some(Type::Error(e)),
(Type::Unreachable, None) => Some(Type::Nothing), (Type::Unreachable, None) => Some(Type::Nothing),
(Type::Unreachable, Some(t)) => Some(t), (Type::Unreachable, Some(t)) => Some(t),
@ -1956,76 +2016,88 @@ impl Semantics {
}) })
.collect(); .collect();
if f.is_error() || arg_types.iter().any(|(_, t)| t.is_error()) { // Propagate type errors if there are any.
return Some(Type::Error); let type_error = if let Type::Error(e) = &f {
Some(e.clone())
} else {
arg_types.iter().find_map(|(_, t)| match t {
Type::Error(e) => Some(e.clone()),
_ => None,
})
};
if let Some(error) = type_error {
return Some(Type::Error(error));
} }
match f { match f {
Type::Function(params, ret) => { Type::Function(params, ret) => {
let mut any_errors = false; let mut param_error = None;
if params.len() != arg_types.len() { if params.len() != arg_types.len() {
// TODO: Augment with function name if known // TODO: Augment with function name if known
self.report_error_tree(tree, format!("expected {} parameters", params.len())); let err = self
any_errors = true; .report_error_tree(tree, format!("expected {} parameters", params.len()));
param_error = Some(err);
} }
for (i, ((t, a), p)) in arg_types.iter().zip(params.iter()).enumerate() { for (i, ((t, a), p)) in arg_types.iter().zip(params.iter()).enumerate() {
// a here is the type of the argument expression; p is // a here is the type of the argument expression; p is
// the declared type of the parameter. // the declared type of the parameter.
if !self.can_convert(&a, p) { if !self.can_convert(&a, p) {
self.report_error_tree_ref( let err = self.report_error_tree_ref(
*t, *t,
format!( format!(
"parameter {i} has an incompatible type: expected {} but got {}", "parameter {i} has an incompatible type: expected {} but got {}",
p, a p, a
), ),
); );
any_errors = true; param_error = Some(err);
} }
} }
if any_errors { if let Some(param_error) = param_error {
return Some(Type::Error); return Some(Type::Error(param_error));
} }
Some(*ret.clone()) Some(*ret.clone())
} }
Type::Method(_, params, ret) => { Type::Method(_, params, ret) => {
let mut any_errors = false; let mut param_error = None;
// For the purposes of type checking ignore the self type. // For the purposes of type checking ignore the self type.
if params.len() != arg_types.len() { if params.len() != arg_types.len() {
// TODO: Augment with function name if known // TODO: Augment with function name if known
self.report_error_tree(tree, format!("expected {} parameters", params.len())); let err = self
any_errors = true; .report_error_tree(tree, format!("expected {} parameters", params.len()));
param_error = Some(err);
} }
for (i, ((t, a), p)) in arg_types.iter().zip(params.iter()).enumerate() { for (i, ((t, a), p)) in arg_types.iter().zip(params.iter()).enumerate() {
// a here is the type of the argument expression; p is // a here is the type of the argument expression; p is
// the declared type of the parameter. // the declared type of the parameter.
if !self.can_convert(&a, p) { if !self.can_convert(&a, p) {
self.report_error_tree_ref( let err = self.report_error_tree_ref(
*t, *t,
format!( format!(
"parameter {i} has an incompatible type: expected {} but got {}", "parameter {i} has an incompatible type: expected {} but got {}",
p, a p, a
), ),
); );
any_errors = true; param_error = Some(err);
} }
} }
if any_errors { if let Some(param_error) = param_error {
return Some(Type::Error); return Some(Type::Error(param_error));
} }
Some(*ret.clone()) Some(*ret.clone())
} }
_ => { _ => {
self.report_error_tree_ref(f_ref, format!("expected a function type, got: {f}")); let err = self
Some(Type::Error) .report_error_tree_ref(f_ref, format!("expected a function type, got: {f}"));
Some(Type::Error(err))
} }
} }
} }
@ -2045,19 +2117,21 @@ impl Semantics {
let env = self.member_environment(lhs, &typ); let env = self.member_environment(lhs, &typ);
let id = tree.nth_token(2)?; let id = tree.nth_token(2)?;
if id.kind != TokenKind::Identifier { if id.kind != TokenKind::Identifier {
return Some(Type::Error); return Some(self.type_error_for(tree));
} }
let id_str = id.as_str(&self.source); let id_str = id.as_str(&self.source);
let Some(declaration) = env.bind(id_str) else { let Some(declaration) = env.bind(id_str) else {
if !env.is_error() { let error = if let Some(e) = &env.error {
e.clone()
} else {
self.report_error_span( self.report_error_span(
id.start(), id.start(),
id.end(), id.end(),
format!("'{typ}' has no member {id_str}"), format!("'{typ}' has no member {id_str}"),
); )
} };
return Some(Type::Error); return Some(Type::Error(error));
}; };
Some(self.type_of_declaration(Some(t), declaration)) Some(self.type_of_declaration(Some(t), declaration))
@ -2104,10 +2178,11 @@ impl Semantics {
} }
EnvironmentRef::new(result) EnvironmentRef::new(result)
} }
Type::Error => return Environment::error("error type has no members"), Type::Error(e) => return Environment::error(e.clone()),
_ => { _ => {
self.report_error_tree_ref(t, format!("cannot access members of '{typ}'")); let error =
return Environment::error("type has no members"); self.report_error_tree_ref(t, format!("cannot access members of '{typ}'"));
return Environment::error(error);
} }
} }
} }
@ -2119,20 +2194,20 @@ impl Semantics {
.map(|t| t.kind == TokenKind::Semicolon) .map(|t| t.kind == TokenKind::Semicolon)
.unwrap_or(false); .unwrap_or(false);
let expression_type = self.type_of(tree.nth_tree(0)?); let expression_type = tree.nth_tree(0).map(|t| self.type_of(t));
Some(match expression_type { match expression_type {
Type::Unreachable => Type::Unreachable, Some(Type::Unreachable) => Some(Type::Unreachable),
_ => { _ => {
// A semicolon at the end of an expression statement discards // A semicolon at the end of an expression statement discards
// the value, leaving us with nothing. (Even if the // the value, leaving us with nothing. (Even if the
// expression otherwise generated a type error!) // expression otherwise generated a type error!)
if last_is_semicolon { if last_is_semicolon {
Type::Nothing Some(Type::Nothing)
} else { } else {
expression_type expression_type
} }
} }
}) }
} }
fn type_of_identifier(&self, t: TreeRef, tree: &Tree) -> Option<Type> { fn type_of_identifier(&self, t: TreeRef, tree: &Tree) -> Option<Type> {
@ -2144,10 +2219,12 @@ impl Semantics {
return Some(self.type_of_declaration(Some(t), declaration)); return Some(self.type_of_declaration(Some(t), declaration));
} }
if !environment.is_error() { let error = if let Some(e) = &environment.error {
self.report_error_tree(tree, format!("cannot find value {id} here")); e.clone()
} } else {
Some(Type::Error) self.report_error_tree(tree, format!("cannot find value {id} here"))
};
Some(Type::Error(error))
} }
fn type_of_declaration(&self, t: Option<TreeRef>, declaration: &Declaration) -> Type { fn type_of_declaration(&self, t: Option<TreeRef>, declaration: &Declaration) -> Type {
@ -2184,8 +2261,8 @@ impl Semantics {
let class_decl = &self.syntax_tree[cd]; let class_decl = &self.syntax_tree[cd];
if class_decl.kind != TreeKind::ClassDecl { if class_decl.kind != TreeKind::ClassDecl {
self.report_error_tree(tree, "self parameter only allowed in methods"); let error = self.report_error_tree(tree, "self parameter only allowed in methods");
Some(Type::Error) Some(Type::Error(error))
} else { } else {
Some(self.type_of(cd)) Some(self.type_of(cd))
} }
@ -2206,10 +2283,12 @@ impl Semantics {
}); });
} }
if !environment.is_error() { let error = if let Some(e) = &environment.error {
self.report_error_tree(tree, "`self` is only valid in methods"); e.clone()
} } else {
Some(Type::Error) self.report_error_tree(tree, "`self` is only valid in methods")
};
Some(Type::Error(error))
} }
fn type_of_function_decl(&self, tree: &Tree) -> Option<Type> { fn type_of_function_decl(&self, tree: &Tree) -> Option<Type> {
@ -2249,8 +2328,9 @@ impl Semantics {
match tree.child_of_kind(&self.syntax_tree, TreeKind::TypeExpression) { match tree.child_of_kind(&self.syntax_tree, TreeKind::TypeExpression) {
Some(t) => Some(self.type_of(t)), Some(t) => Some(self.type_of(t)),
None => { None => {
self.report_error_tree(tree, format!("the parameter is missing a type")); let error =
Some(Type::Error) self.report_error_tree(tree, format!("the parameter is missing a type"));
Some(Type::Error(error))
} }
} }
} }
@ -2262,11 +2342,12 @@ impl Semantics {
let enumerable = parent.nth_tree(3)?; let enumerable = parent.nth_tree(3)?;
let item_type = match self.type_of(enumerable) { let item_type = match self.type_of(enumerable) {
Type::Error => Type::Error, Type::Error(e) => Type::Error(e),
Type::List(x) => (&*x).clone(), Type::List(x) => (&*x).clone(),
_ => { _ => {
self.report_error_tree_ref(enumerable, "this expression is not enumerable"); let error =
Type::Error self.report_error_tree_ref(enumerable, "this expression is not enumerable");
Type::Error(error)
} }
}; };
@ -2344,10 +2425,12 @@ impl Semantics {
let declaration = match environment.bind(id) { let declaration = match environment.bind(id) {
Some(d) => d, Some(d) => d,
None => { None => {
if !environment.is_error() { let error = if let Some(e) = &environment.error {
self.report_error_tree(tree, format!("cannot find value {id} here")); e.clone()
} } else {
return Some(Type::Error); self.report_error_tree(tree, format!("cannot find value {id} here"))
};
return Some(Type::Error(error));
} }
}; };
match declaration { match declaration {
@ -2359,18 +2442,18 @@ impl Semantics {
} => Some(declaration_type.clone()), } => Some(declaration_type.clone()),
Declaration::Class { .. } => { Declaration::Class { .. } => {
self.report_error_tree( let error = self.report_error_tree(
tree, tree,
format!("'{id}' is a class, and cannot be the value of a field"), format!("'{id}' is a class, and cannot be the value of a field"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
Declaration::ImportedModule { .. } => { Declaration::ImportedModule { .. } => {
self.report_error_tree( let error = self.report_error_tree(
tree, tree,
format!("'{id}' is an imported module, and cannot be the value of a field"), format!("'{id}' is an imported module, and cannot be the value of a field"),
); );
Some(Type::Error) Some(Type::Error(error))
} }
Declaration::ImportedDeclaration { Declaration::ImportedDeclaration {
semantics, semantics,
@ -2416,8 +2499,9 @@ impl Semantics {
.collect(); .collect();
if arms.len() == 0 { if arms.len() == 0 {
self.report_error_tree(tree, "a match expression must have at least one arm"); let error =
Some(Type::Error) self.report_error_tree(tree, "a match expression must have at least one arm");
Some(Type::Error(error))
} else { } else {
let mut actual_type = self.type_of(arms[0]); let mut actual_type = self.type_of(arms[0]);
for arm in &arms[1..] { for arm in &arms[1..] {
@ -2456,7 +2540,7 @@ impl Semantics {
fn type_of_import(&self, tree: &Tree) -> Option<Type> { fn type_of_import(&self, tree: &Tree) -> Option<Type> {
let tok = tree.nth_token(1)?; let tok = tree.nth_token(1)?;
if tok.kind != TokenKind::String { if tok.kind != TokenKind::String {
return Some(Type::Error); // Already reported as syntax error return Some(self.type_error_for(tree));
} }
// do we bind it here? it's not normalized.... // do we bind it here? it's not normalized....
@ -2468,8 +2552,9 @@ impl Semantics {
match import_map.get(&name) { match import_map.get(&name) {
Some(import) => Some(Type::Module(name.into(), import.clone())), Some(import) => Some(Type::Module(name.into(), import.clone())),
None => { None => {
self.report_error_tree(tree, format!("unable to resolve module import {name}")); let error =
Some(Type::Error) self.report_error_tree(tree, format!("unable to resolve module import {name}"));
Some(Type::Error(error))
} }
} }
} }
@ -2651,7 +2736,7 @@ impl Semantics {
eprintln!("\nThe environment of the tree was:"); eprintln!("\nThe environment of the tree was:");
let mut environment = Some(self.environment_of(tr)); let mut environment = Some(self.environment_of(tr));
while let Some(env) = environment { while let Some(env) = environment {
if let Some(error) = env.error { if let Some(error) = &env.error {
eprint!(" *** ERROR: {error}"); eprint!(" *** ERROR: {error}");
} }
for (k, v) in env.declarations.iter() { for (k, v) in env.declarations.iter() {
@ -2865,14 +2950,14 @@ fn check_return_statement(s: &Semantics, tree: &Tree) {
let actual_type = if let Some(expr) = tree.nth_tree(1) { let actual_type = if let Some(expr) = tree.nth_tree(1) {
s.type_of(expr) s.type_of(expr)
} else { } else {
Type::Error Type::Nothing
}; };
if !s.can_convert(&actual_type, &expected_type) { if !s.can_convert(&actual_type, &expected_type) {
s.report_error_tree(tree, format!("callers of this function expect a value of type '{expected_type}' but this statement returns a value of type '{actual_type}'")); s.report_error_tree(tree, format!("callers of this function expect a value of type '{expected_type}' but this statement returns a value of type '{actual_type}'"));
} }
} }
Type::Error => (), Type::Error(_) => (),
_ => s.internal_compiler_error( _ => s.internal_compiler_error(
Some(enclosing_function), Some(enclosing_function),
"a return statement in here expected this to yield a function type", "a return statement in here expected this to yield a function type",
@ -2942,11 +3027,13 @@ fn check_new_object_expression(s: &Semantics, tree: &Tree) {
} }
} }
} }
Type::Error => (), Type::Error(_) => (),
ct => s.report_error_tree_ref( ct => {
type_expression, s.report_error_tree_ref(
format!("expected this to be a class type, but it is {ct}"), type_expression,
), format!("expected this to be a class type, but it is {ct}"),
);
}
} }
} }
@ -2981,7 +3068,7 @@ fn check_pattern(s: &Semantics, tree: &Tree) {
s.report_error_tree_ref( s.report_error_tree_ref(
pred, pred,
format!("this predicate produces '{predicate_type}', but must produce bool"), format!("this predicate produces '{predicate_type}', but must produce bool"),
) );
} }
} }
} }

12
fine/tests/example_tests.rs
View file

@ -198,7 +198,7 @@ fn assert_type_at(module: Rc<fine::Module>, pos: usize, expected: &str, _source_
fn assert_type_error_at( fn assert_type_error_at(
module: Rc<fine::Module>, module: Rc<fine::Module>,
errors: &[Error], errors: &[Rc<Error>],
pos: usize, pos: usize,
expected: &str, expected: &str,
_source_path: &str, _source_path: &str,
@ -219,7 +219,7 @@ fn assert_type_error_at(
semantic_assert!( semantic_assert!(
&semantics, &semantics,
Some(tree_ref), Some(tree_ref),
matches!(tree_type, Type::Error), matches!(tree_type, Type::Error(_)),
"The type of the {:?} tree at position {pos} was '{tree_type:?}', not an error", "The type of the {:?} tree at position {pos} was '{tree_type:?}', not an error",
tree[tree_ref].kind tree[tree_ref].kind
); );
@ -286,10 +286,10 @@ fn assert_compiles_to(module: Rc<fine::Module>, expected: &str, source_path: &st
} }
} }
fn assert_no_errors(module: Rc<fine::Module>, errors: &[Error]) { fn assert_no_errors(module: Rc<fine::Module>, errors: &[Rc<Error>]) {
let semantics = module.semantics(); let semantics = module.semantics();
let expected_errors: &[Error] = &[]; let expected_errors: &[Rc<Error>] = &[];
semantic_assert_eq!( semantic_assert_eq!(
&semantics, &semantics,
None, None,
@ -341,7 +341,7 @@ fn assert_eval_ok(module: Rc<fine::Module>, expected: &str) {
} }
} }
fn assert_errors(module: Rc<fine::Module>, errors: &[Error], expected_errors: Vec<&str>) { fn assert_errors(module: Rc<fine::Module>, errors: &[Rc<Error>], expected_errors: Vec<&str>) {
let semantics = module.semantics(); let semantics = module.semantics();
let errors: Vec<String> = errors.iter().map(|e| format!("{}", e)).collect(); let errors: Vec<String> = errors.iter().map(|e| format!("{}", e)).collect();
@ -355,7 +355,7 @@ fn assert_errors(module: Rc<fine::Module>, errors: &[Error], expected_errors: Ve
); );
} }
fn assert_check_error(module: Rc<fine::Module>, errors: &[Error], expected: &str) { fn assert_check_error(module: Rc<fine::Module>, errors: &[Rc<Error>], expected: &str) {
let semantics = module.semantics(); let semantics = module.semantics();
semantic_assert!( semantic_assert!(

7
fine/tests/expression/errors/return_statement_mismatch.fine
View file

@ -1,3 +1,7 @@
fun something() -> f64 {
return
}
fun test() -> f64 { fun test() -> f64 {
if false { if false {
return "no way!"; return "no way!";
@ -6,4 +10,5 @@ fun test() -> f64 {
} }
// @expect-errors: // @expect-errors:
// | __test__:3:4: callers of this function expect a value of type 'f64' but this statement returns a value of type 'string' // | __test__:2:2: callers of this function expect a value of type 'f64' but this statement returns a value of type 'nothing'
// | __test__:7:4: callers of this function expect a value of type 'f64' but this statement returns a value of type 'string'