From 2662376941367d88687b3ebc4e4b941b266cee42 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Mon, 22 Aug 2022 23:20:50 +0300 Subject: feat(tvix/eval): carry optional SyntaxNode in error type This starts paving the way for nicer, source-code based error reporting. Right now the code paths in the VM do not emit annotated errors, as we do not yet preserve that structure from the compiler. However, error emitting code paths in the compiler have been amended to include known nodes. Change-Id: I1b74410ffd891c40cd913361bd73c4336ec8aa5b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6235 Tested-by: BuildkiteCI Reviewed-by: grfn --- tvix/eval/src/compiler.rs | 14 +++++++------- tvix/eval/src/errors.rs | 16 ++++++++++++++-- tvix/eval/src/eval.rs | 4 ++-- tvix/eval/src/value/attrs.rs | 12 +++++++----- tvix/eval/src/value/mod.rs | 27 ++++++++++++++++----------- tvix/eval/src/vm.rs | 29 ++++++++++++++++------------- 6 files changed, 62 insertions(+), 40 deletions(-) (limited to 'tvix') diff --git a/tvix/eval/src/compiler.rs b/tvix/eval/src/compiler.rs index 0718f45ba7..92f04c84dc 100644 --- a/tvix/eval/src/compiler.rs +++ b/tvix/eval/src/compiler.rs @@ -19,7 +19,7 @@ use rowan::ast::AstNode; use std::path::{Path, PathBuf}; use crate::chunk::Chunk; -use crate::errors::{Error, EvalResult}; +use crate::errors::{ErrorKind, EvalResult}; use crate::opcode::{CodeIdx, OpCode}; use crate::value::Value; use crate::warnings::{EvalWarning, WarningKind}; @@ -155,7 +155,7 @@ impl Compiler { Path::new(&raw_path).to_owned() } else if raw_path.starts_with('~') { let mut buf = dirs::home_dir().ok_or_else(|| { - Error::PathResolution("failed to determine home directory".into()) + ErrorKind::PathResolution("failed to determine home directory".into()) })?; buf.push(&raw_path); @@ -435,7 +435,7 @@ impl Compiler { match self.resolve_local(ident.ident_token().unwrap().text()) { Some(idx) => self.chunk.push_op(OpCode::OpGetLocal(idx)), - None => return Err(Error::UnknownStaticVariable(ident)), + None => return Err(ErrorKind::UnknownStaticVariable(ident).into()), }; } } @@ -663,7 +663,7 @@ impl Compiler { Some(idx) => self.chunk.push_op(OpCode::OpGetLocal(idx)), None => { if self.scope.with_stack.is_empty() { - return Err(Error::UnknownStaticVariable(node)); + return Err(ErrorKind::UnknownStaticVariable(node).into()); } // Variable needs to be dynamically resolved @@ -836,7 +836,7 @@ fn expr_str_to_string(expr: ast::Str) -> EvalResult { } } - return Err(Error::DynamicKeyInLet(expr.syntax().clone())); + return Err(ErrorKind::DynamicKeyInLet(expr.syntax().clone()).into()); } /// Convert a single identifier path fragment to a string if possible, @@ -852,7 +852,7 @@ fn attr_to_string(node: ast::Attr) -> EvalResult { // inside (i.e. `let ${"a"} = 1; in a` is valid). ast::Attr::Dynamic(ref dynamic) => match dynamic.expr().unwrap() { ast::Expr::Str(s) => expr_str_to_string(s), - _ => Err(Error::DynamicKeyInLet(node.syntax().clone())), + _ => Err(ErrorKind::DynamicKeyInLet(node.syntax().clone()).into()), }, } } @@ -868,7 +868,7 @@ pub fn compile(expr: ast::Expr, location: Option) -> EvalResult Ok(dir), None => std::env::current_dir().map_err(|e| { - Error::PathResolution(format!("could not determine current directory: {}", e)) + ErrorKind::PathResolution(format!("could not determine current directory: {}", e)) }), }?; diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs index 4aba877f49..01a5e81449 100644 --- a/tvix/eval/src/errors.rs +++ b/tvix/eval/src/errors.rs @@ -1,7 +1,7 @@ use std::fmt::Display; #[derive(Debug)] -pub enum Error { +pub enum ErrorKind { DuplicateAttrsKey { key: String, }, @@ -37,9 +37,21 @@ pub enum Error { AssertionFailed, } +#[derive(Debug)] +pub struct Error { + pub node: Option, + pub kind: ErrorKind, +} + +impl From for Error { + fn from(kind: ErrorKind) -> Self { + Error { node: None, kind } + } +} + impl Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{:?}", self) + writeln!(f, "{:?}", self.kind) } } diff --git a/tvix/eval/src/eval.rs b/tvix/eval/src/eval.rs index cd11d3289b..8f2946e6cc 100644 --- a/tvix/eval/src/eval.rs +++ b/tvix/eval/src/eval.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use rnix; use crate::{ - errors::{Error, EvalResult}, + errors::{ErrorKind, EvalResult}, value::Value, }; @@ -15,7 +15,7 @@ pub fn interpret(code: &str, location: Option) -> EvalResult { for err in errors { eprintln!("parse error: {}", err); } - return Err(Error::ParseErrors(errors.to_vec())); + return Err(ErrorKind::ParseErrors(errors.to_vec()).into()); } // If we've reached this point, there are no errors. diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index f614128550..319f6bdfa9 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -10,7 +10,7 @@ use std::collections::BTreeMap; use std::fmt::Display; use std::rc::Rc; -use crate::errors::{Error, EvalResult}; +use crate::errors::{ErrorKind, EvalResult}; use super::string::NixString; use super::Value; @@ -304,9 +304,10 @@ fn attempt_optimise_kv(slice: &mut [Value]) -> Option { // checking against duplicate keys. fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> EvalResult<()> { match attrs.0.map_mut().entry(key) { - btree_map::Entry::Occupied(entry) => Err(Error::DuplicateAttrsKey { + btree_map::Entry::Occupied(entry) => Err(ErrorKind::DuplicateAttrsKey { key: entry.key().as_str().to_string(), - }), + } + .into()), btree_map::Entry::Vacant(entry) => { entry.insert(value); @@ -365,9 +366,10 @@ fn set_nested_attr( } _ => { - return Err(Error::DuplicateAttrsKey { + return Err(ErrorKind::DuplicateAttrsKey { key: entry.key().as_str().to_string(), - }) + } + .into()) } }, } diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 6c0473fa66..f054191f67 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -7,7 +7,7 @@ mod attrs; mod list; mod string; -use crate::errors::{Error, EvalResult}; +use crate::errors::{ErrorKind, EvalResult}; pub use attrs::NixAttrs; pub use list::NixList; pub use string::NixString; @@ -55,50 +55,55 @@ impl Value { pub fn as_bool(&self) -> EvalResult { match self { Value::Bool(b) => Ok(*b), - other => Err(Error::TypeError { + other => Err(ErrorKind::TypeError { expected: "bool", actual: other.type_of(), - }), + } + .into()), } } pub fn as_attrs(&self) -> EvalResult<&NixAttrs> { match self { Value::Attrs(attrs) => Ok(attrs), - other => Err(Error::TypeError { + other => Err(ErrorKind::TypeError { expected: "set", actual: other.type_of(), - }), + } + .into()), } } pub fn to_string(self) -> EvalResult { match self { Value::String(s) => Ok(s), - other => Err(Error::TypeError { + other => Err(ErrorKind::TypeError { expected: "string", actual: other.type_of(), - }), + } + .into()), } } pub fn to_attrs(self) -> EvalResult> { match self { Value::Attrs(s) => Ok(s), - other => Err(Error::TypeError { + other => Err(ErrorKind::TypeError { expected: "set", actual: other.type_of(), - }), + } + .into()), } } pub fn to_list(self) -> EvalResult { match self { Value::List(l) => Ok(l), - other => Err(Error::TypeError { + other => Err(ErrorKind::TypeError { expected: "list", actual: other.type_of(), - }), + } + .into()), } } diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index cf70fda307..7a6a6454ea 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -5,7 +5,7 @@ use std::rc::Rc; use crate::{ chunk::Chunk, - errors::{Error, EvalResult}, + errors::{ErrorKind, EvalResult}, opcode::OpCode, value::{NixAttrs, NixList, Value}, }; @@ -38,14 +38,14 @@ macro_rules! arithmetic_op { (Value::Integer(i1), Value::Float(f2)) => Value::Float(i1 as f64 $op f2), (Value::Float(f1), Value::Integer(i2)) => Value::Float(f1 $op i2 as f64), - (v1, v2) => return Err(Error::TypeError { + (v1, v2) => return Err(ErrorKind::TypeError { expected: "number (either int or float)", actual: if v1.is_number() { v2.type_of() } else { v1.type_of() }, - }), + }.into()), } }}; } @@ -65,10 +65,10 @@ macro_rules! cmp_op { (Value::Float(f1), Value::Integer(i2)) => f1 $op (i2 as f64), (Value::String(s1), Value::String(s2)) => s1 $op s2, - (lhs, rhs) => return Err(Error::Incomparable { + (lhs, rhs) => return Err(ErrorKind::Incomparable { lhs: lhs.type_of(), rhs: rhs.type_of(), - }), + }.into()), }; $self.push(Value::Bool(result)); @@ -136,10 +136,11 @@ impl VM { Value::Integer(i) => self.push(Value::Integer(-i)), Value::Float(f) => self.push(Value::Float(-f)), v => { - return Err(Error::TypeError { + return Err(ErrorKind::TypeError { expected: "number (either int or float)", actual: v.type_of(), - }) + } + .into()) } }, @@ -177,9 +178,10 @@ impl VM { Some(value) => self.push(value.clone()), None => { - return Err(Error::AttributeNotFound { + return Err(ErrorKind::AttributeNotFound { name: key.as_str().to_string(), - }) + } + .into()) } } } @@ -255,10 +257,11 @@ impl VM { OpCode::OpAssertBool => { let val = self.peek(0); if !val.is_bool() { - return Err(Error::TypeError { + return Err(ErrorKind::TypeError { expected: "bool", actual: val.type_of(), - }); + } + .into()); } } @@ -302,12 +305,12 @@ impl VM { } } - return Err(Error::UnknownDynamicVariable(ident.to_string())); + return Err(ErrorKind::UnknownDynamicVariable(ident.to_string()).into()); } OpCode::OpAssert => { if !self.pop().as_bool()? { - return Err(Error::AssertionFailed); + return Err(ErrorKind::AssertionFailed.into()); } } } -- cgit 1.4.1