mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(tvix/eval): enrich errors with VM's frame stack information

Browse source 
When emitting an error at runtime, the VM will now use the new
`NativeError` and `BytecodeError` error kinds (which just wrap inner
errors) to create a set of diagnostics to emit.

The primary diagnostic is emitted last, with `error` type (so it will
be coloured red in terminals), the other ones will be emitted with
`note` type, highlighting the causal chain.

Example:
https://gist.github.com/tazjin/25feba7d211702453c9ebd5f8fd378e4

This is currently quite verbose, and we can cut down on this further,
but the purpose of this commit is to surface more information first of
all before worrying about the exact display.

Change-Id: I058104a178c37031c0db6b4b3e4f4170cf76087d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8266
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
Vincent Ambo 2023-03-12 18:06:11 +03:00 committed by clbot
parent b78ae941a4
commit ea80e0d3f8
4 changed files with 243 additions and 130 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
tvix/eval/src/compiler/mod.rs
View file

@ -1339,7 +1339,8 @@ fn compile_src_builtin(
file.clone(),
weak.upgrade().unwrap(),
&mut crate::observer::NoOpObserver {},
)?;
)
.map_err(|e| ErrorKind::NativeError(Box::new(e)))?;
if !result.errors.is_empty() {
return Err(ErrorKind::ImportCompilerError {

129
tvix/eval/src/errors.rs
View file

@ -81,9 +81,13 @@ pub enum ErrorKind {
ParseErrors(Vec<rnix::parser::ParseError>),
/// An error occured while forcing a thunk, and needs to be
/// chained up.
ThunkForce(Box<Error>),
/// An error occured while executing some native code (e.g. a
/// builtin), and needs to be chained up.
NativeError(Box<Error>),
/// An error occured while executing Tvix bytecode, but needs to
/// be chained up.
BytecodeError(Box<Error>),
/// Given type can't be coerced to a string in the respective context
NotCoercibleToString {
@ -175,7 +179,7 @@ pub enum ErrorKind {
impl error::Error for Error {
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
match &self.kind {
ErrorKind::ThunkForce(err) => err.source(),
ErrorKind::NativeError(err) | ErrorKind::BytecodeError(err) => err.source(),
ErrorKind::ParseErrors(err) => err.first().map(|e| e as &dyn error::Error),
ErrorKind::ParseIntError(err) => Some(err),
ErrorKind::ImportParseError { errors, .. } => {
@ -216,15 +220,6 @@ impl From<XmlError> for ErrorKind {
}
}
/// Implementation used if errors occur while forcing thunks (which
/// can potentially be threaded through a few contexts, i.e. nested
/// thunks).
impl From<Error> for ErrorKind {
fn from(e: Error) -> Self {
Self::ThunkForce(Box::new(e))
}
}
impl From<io::Error> for ErrorKind {
fn from(e: io::Error) -> Self {
ErrorKind::IO {
@ -239,7 +234,7 @@ impl ErrorKind {
pub fn is_catchable(&self) -> bool {
match self {
Self::Throw(_) | Self::AssertionFailed | Self::NixPathResolution(_) => true,
Self::ThunkForce(err) => err.kind.is_catchable(),
Self::NativeError(err) | Self::BytecodeError(err) => err.kind.is_catchable(),
_ => false,
}
}
@ -361,10 +356,8 @@ to a missing value in the attribute set(s) included via `with`."#,
// Errors themselves ignored here & handled in Self::spans instead
ErrorKind::ParseErrors(_) => write!(f, "failed to parse Nix code:"),
// TODO(tazjin): trace through the whole chain of thunk
// forcing errors with secondary spans, instead of just
// delegating to the inner error
ErrorKind::ThunkForce(err) => write!(f, "{err}"),
ErrorKind::NativeError(_) => write!(f, "while evaluating this native code"),
ErrorKind::BytecodeError(_) => write!(f, "while evaluating this Nix code"),
ErrorKind::NotCoercibleToString { kind, from } => {
let kindly = match kind {
@ -757,7 +750,8 @@ impl Error {
| ErrorKind::NotCallable(_)
| ErrorKind::InfiniteRecursion
| ErrorKind::ParseErrors(_)
| ErrorKind::ThunkForce(_)
| ErrorKind::NativeError(_)
| ErrorKind::BytecodeError(_)
| ErrorKind::NotCoercibleToString { .. }
| ErrorKind::NotAnAbsolutePath(_)
| ErrorKind::ParseIntError(_)
@ -831,12 +825,9 @@ impl Error {
// Placeholder error while Tvix is under construction.
ErrorKind::NotImplemented(_) => "E999",
// TODO: thunk force errors should yield a chained
// diagnostic, but until then we just forward the error
// code from the inner error.
//
// The error code for thunk forces is E017.
ErrorKind::ThunkForce(ref err) => err.code(),
// Chained errors should yield the code of the innermost
// error.
ErrorKind::NativeError(ref err) | ErrorKind::BytecodeError(ref err) => err.code(),
ErrorKind::WithContext { .. } => {
panic!("internal ErrorKind::WithContext variant leaked")
@ -855,27 +846,6 @@ impl Error {
spans_for_parse_errors(&file, errors)
}
// Unwrap thunk errors to the innermost one
// TODO: limit the number of intermediates!
ErrorKind::ThunkForce(err) => {
let mut labels = err.spans(source);
// Only add this thunk to the "cause chain" if it span isn't
// exactly identical to the next-higher level, which is very
// common for the last thunk in a chain.
if let Some(label) = labels.last() {
if label.span != self.span {
labels.push(SpanLabel {
label: Some("while evaluating this".into()),
span: self.span,
style: SpanStyle::Secondary,
});
}
}
labels
}
ErrorKind::UnexpectedArgument { formals_span, .. } => {
vec![
SpanLabel {
@ -926,19 +896,82 @@ impl Error {
/// any) of an error.
fn diagnostics(&self, source: &SourceCode) -> Vec<Diagnostic> {
match &self.kind {
ErrorKind::ThunkForce(err) => err.diagnostics(source),
ErrorKind::ImportCompilerError { errors, .. } => {
let mut out = vec![self.diagnostic(source)];
out.extend(errors.iter().map(|e| e.diagnostic(source)));
out
}
// When encountering either of these error kinds, we are dealing
// with the top of an error chain.
//
// An error chain creates a list of diagnostics which provide trace
// information.
//
// We don't know how deep this chain is, so we avoid recursing in
// this function while unrolling the chain.
ErrorKind::NativeError(next) | ErrorKind::BytecodeError(next) => {
// Accumulated diagnostics to return.
let mut diagnostics: Vec<Diagnostic> = vec![];
// The next (inner) error to add to the diagnostics, after this
// one.
let mut next = *next.clone();
// Diagnostic message for *this* error.
let mut this_message = self.to_string();
// Primary span for *this* error.
let mut this_span = self.span;
// Diagnostic spans for *this* error.
let mut this_spans = self.spans(source);
loop {
if is_new_span(
this_span,
diagnostics.last().and_then(|last| last.spans.last()),
) {
diagnostics.push(Diagnostic {
level: Level::Note,
message: this_message,
spans: this_spans,
code: None, // only the top-level error has one
});
}
this_message = next.to_string();
this_span = next.span;
this_spans = next.spans(source);
match next.kind {
ErrorKind::NativeError(inner) | ErrorKind::BytecodeError(inner) => {
next = *inner;
continue;
}
_ => {
diagnostics.extend(next.diagnostics(source));
break;
}
}
}
diagnostics
}
_ => vec![self.diagnostic(source)],
}
}
}
// Check if this error is in a different span from its immediate ancestor.
fn is_new_span(this_span: Span, parent: Option<&SpanLabel>) -> bool {
match parent {
None => true,
Some(parent) => parent.span != this_span,
}
}
// Convenience methods to add context on other types.
pub trait AddContext {
/// Add context to the error-carrying type.

22
tvix/eval/src/vm/generators.rs
View file

@ -400,19 +400,15 @@ impl<'o> VM<'o> {
}
VMRequest::PathImport(path) => {
let imported = self
.io_handle
.import_path(&path)
.map_err(|kind| Error::new(kind, span.span()))?;
let imported =
self.io_handle.import_path(&path).with_span(&span, self)?;
message = VMResponse::Path(imported);
}
VMRequest::ReadToString(path) => {
let content = self
.io_handle
.read_to_string(path)
.map_err(|kind| Error::new(kind, span.span()))?;
let content =
self.io_handle.read_to_string(path).with_span(&span, self)?;
message = VMResponse::Value(Value::String(content.into()))
}
@ -422,17 +418,13 @@ impl<'o> VM<'o> {
.io_handle
.path_exists(path)
.map(Value::Bool)
.map_err(|kind| Error::new(kind, span.span()))?;
.with_span(&span, self)?;
message = VMResponse::Value(exists);
}
VMRequest::ReadDir(path) => {
let dir = self
.io_handle
.read_dir(path)
.map_err(|kind| Error::new(kind, span.span()))?;
let dir = self.io_handle.read_dir(path).with_span(&span, self)?;
message = VMResponse::Directory(dir);
}
@ -466,7 +458,7 @@ impl<'o> VM<'o> {
// Generator has completed, and its result value should
// be left on the stack.
genawaiter::GeneratorState::Complete(result) => {
let value = result.map_err(|kind| Error::new(kind, span.span()))?;
let value = result.with_span(&span, self)?;
self.stack.push(value);
return Ok(true);
}

219
tvix/eval/src/vm/mod.rs
View file

@ -12,6 +12,7 @@
pub mod generators;
mod macros;
use codemap::Span;
use serde_json::json;
use std::{cmp::Ordering, collections::HashMap, ops::DerefMut, path::PathBuf, rc::Rc};
@ -39,15 +40,75 @@ use generators::{call_functor, Generator, GeneratorState};
use self::generators::{VMRequest, VMResponse};
/// Internal helper trait for ergonomically converting from a `Result<T,
/// ErrorKind>` to a `Result<T, Error>` using the current span of a call frame.
trait WithSpan<T> {
fn with_span(self, frame: &CallFrame) -> Result<T, Error>;
/// Internal helper trait for taking a span from a variety of types, to make use
/// of `WithSpan` (defined below) more ergonomic at call sites.
trait GetSpan {
fn get_span(self) -> Span;
}
impl<T> WithSpan<T> for Result<T, ErrorKind> {
fn with_span(self, frame: &CallFrame) -> Result<T, Error> {
self.map_err(|kind| frame.error(kind))
impl<'o> GetSpan for &VM<'o> {
fn get_span(self) -> Span {
self.reasonable_span.span()
}
}
impl GetSpan for &CallFrame {
fn get_span(self) -> Span {
self.current_span()
}
}
impl GetSpan for &LightSpan {
fn get_span(self) -> Span {
self.span()
}
}
impl GetSpan for Span {
fn get_span(self) -> Span {
self
}
}
/// Internal helper trait for ergonomically converting from a `Result<T,
/// ErrorKind>` to a `Result<T, Error>` using the current span of a call frame,
/// and chaining the VM's frame stack around it for printing a cause chain.
trait WithSpan<T, S: GetSpan> {
fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error>;
}
impl<T, S: GetSpan> WithSpan<T, S> for Result<T, ErrorKind> {
fn with_span(self, top_span: S, vm: &VM) -> Result<T, Error> {
match self {
Ok(something) => Ok(something),
Err(kind) => {
let mut error = Error::new(kind, top_span.get_span());
// Short-circuit the wrapping if we're dealing with tryEval, in
// which case the error is hidden and does not need to be
// exhaustive.
if !vm.try_eval_frames.is_empty() && error.kind.is_catchable() {
return Err(error);
}
// Wrap the top-level error in chaining errors for each element
// of the frame stack.
for frame in vm.frames.iter().rev() {
match frame {
Frame::CallFrame { span, .. } => {
error =
Error::new(ErrorKind::BytecodeError(Box::new(error)), span.span());
}
Frame::Generator { span, .. } => {
error =
Error::new(ErrorKind::NativeError(Box::new(error)), span.span());
}
}
}
Err(error)
}
}
}
}
@ -86,10 +147,16 @@ impl CallFrame {
op
}
/// Construct an error from the given ErrorKind and the source span of the
/// current instruction.
pub fn error(&self, kind: ErrorKind) -> Error {
Error::new(kind, self.chunk().get_span(self.ip - 1))
/// Construct an error result from the given ErrorKind and the source span
/// of the current instruction.
pub fn error<T>(&self, vm: &VM, kind: ErrorKind) -> Result<T, Error> {
Err(kind).with_span(self, vm)
}
/// Returns the current span. This is potentially expensive and should only
/// be used when actually constructing an error or warning.
pub fn current_span(&self) -> Span {
self.chunk().get_span(self.ip - 1)
}
/// Returns the information needed to calculate the current span,
@ -475,23 +542,26 @@ impl<'o> VM<'o> {
}
OpCode::OpAttrsSelect => {
let key = self.stack_pop().to_str().with_span(&frame)?;
let attrs = self.stack_pop().to_attrs().with_span(&frame)?;
let key = self.stack_pop().to_str().with_span(&frame, self)?;
let attrs = self.stack_pop().to_attrs().with_span(&frame, self)?;
match attrs.select(key.as_str()) {
Some(value) => self.stack.push(value.clone()),
None => {
return Err(frame.error(ErrorKind::AttributeNotFound {
name: key.as_str().to_string(),
}))
return frame.error(
self,
ErrorKind::AttributeNotFound {
name: key.as_str().to_string(),
},
);
}
}
}
OpCode::OpJumpIfFalse(JumpOffset(offset)) => {
debug_assert!(offset != 0);
if !self.stack_peek(0).as_bool().with_span(&frame)? {
if !self.stack_peek(0).as_bool().with_span(&frame, self)? {
frame.ip += offset;
}
}
@ -501,7 +571,7 @@ impl<'o> VM<'o> {
}
OpCode::OpAttrsTrySelect => {
let key = self.stack_pop().to_str().with_span(&frame)?;
let key = self.stack_pop().to_str().with_span(&frame, self)?;
let value = match self.stack_pop() {
Value::Attrs(attrs) => match attrs.select(key.as_str()) {
Some(value) => value.clone(),
@ -550,24 +620,27 @@ impl<'o> VM<'o> {
OpCode::OpAssertBool => {
let val = self.stack_peek(0);
if !val.is_bool() {
return Err(frame.error(ErrorKind::TypeError {
expected: "bool",
actual: val.type_of(),
}));
return frame.error(
self,
ErrorKind::TypeError {
expected: "bool",
actual: val.type_of(),
},
);
}
}
OpCode::OpAttrs(Count(count)) => self.run_attrset(&frame, count)?,
OpCode::OpAttrsUpdate => {
let rhs = self.stack_pop().to_attrs().with_span(&frame)?;
let lhs = self.stack_pop().to_attrs().with_span(&frame)?;
let rhs = self.stack_pop().to_attrs().with_span(&frame, self)?;
let lhs = self.stack_pop().to_attrs().with_span(&frame, self)?;
self.stack.push(Value::attrs(lhs.update(*rhs)))
}
OpCode::OpInvert => {
let v = self.stack_pop().as_bool().with_span(&frame)?;
let v = self.stack_pop().as_bool().with_span(&frame, self)?;
self.stack.push(Value::Bool(!v));
}
@ -580,13 +653,13 @@ impl<'o> VM<'o> {
OpCode::OpJumpIfTrue(JumpOffset(offset)) => {
debug_assert!(offset != 0);
if self.stack_peek(0).as_bool().with_span(&frame)? {
if self.stack_peek(0).as_bool().with_span(&frame, self)? {
frame.ip += offset;
}
}
OpCode::OpHasAttr => {
let key = self.stack_pop().to_str().with_span(&frame)?;
let key = self.stack_pop().to_str().with_span(&frame, self)?;
let result = match self.stack_pop() {
Value::Attrs(attrs) => attrs.contains(key.as_str()),
@ -599,13 +672,21 @@ impl<'o> VM<'o> {
}
OpCode::OpConcat => {
let rhs = self.stack_pop().to_list().with_span(&frame)?.into_inner();
let lhs = self.stack_pop().to_list().with_span(&frame)?.into_inner();
let rhs = self
.stack_pop()
.to_list()
.with_span(&frame, self)?
.into_inner();
let lhs = self
.stack_pop()
.to_list()
.with_span(&frame, self)?
.into_inner();
self.stack.push(Value::List(NixList::from(lhs + rhs)))
}
OpCode::OpResolveWith => {
let ident = self.stack_pop().to_str().with_span(&frame)?;
let ident = self.stack_pop().to_str().with_span(&frame, self)?;
// Re-enqueue this frame.
let op_span = frame.current_light_span();
@ -664,13 +745,16 @@ impl<'o> VM<'o> {
"OpValidateClosedFormals called within the frame of a lambda without formals",
);
let args = self.stack_peek(0).to_attrs().with_span(&frame)?;
let args = self.stack_peek(0).to_attrs().with_span(&frame, self)?;
for arg in args.keys() {
if !formals.contains(arg) {
return Err(frame.error(ErrorKind::UnexpectedArgument {
arg: arg.clone(),
formals_span: formals.span,
}));
return frame.error(
self,
ErrorKind::UnexpectedArgument {
arg: arg.clone(),
formals_span: formals.span,
},
);
}
}
}
@ -692,14 +776,14 @@ impl<'o> VM<'o> {
OpCode::OpSub => {
let b = self.stack_pop();
let a = self.stack_pop();
let result = arithmetic_op!(&a, &b, -).with_span(&frame)?;
let result = arithmetic_op!(&a, &b, -).with_span(&frame, self)?;
self.stack.push(result);
}
OpCode::OpMul => {
let b = self.stack_pop();
let a = self.stack_pop();
let result = arithmetic_op!(&a, &b, *).with_span(&frame)?;
let result = arithmetic_op!(&a, &b, *).with_span(&frame, self)?;
self.stack.push(result);
}
@ -707,15 +791,15 @@ impl<'o> VM<'o> {
let b = self.stack_pop();
match b {
Value::Integer(0) => return Err(frame.error(ErrorKind::DivisionByZero)),
Value::Integer(0) => return frame.error(self, ErrorKind::DivisionByZero),
Value::Float(b) if b == 0.0_f64 => {
return Err(frame.error(ErrorKind::DivisionByZero))
return frame.error(self, ErrorKind::DivisionByZero)
}
_ => {}
};
let a = self.stack_pop();
let result = arithmetic_op!(&a, &b, /).with_span(&frame)?;
let result = arithmetic_op!(&a, &b, /).with_span(&frame, self)?;
self.stack.push(result);
}
@ -723,10 +807,13 @@ impl<'o> VM<'o> {
Value::Integer(i) => self.stack.push(Value::Integer(-i)),
Value::Float(f) => self.stack.push(Value::Float(-f)),
v => {
return Err(frame.error(ErrorKind::TypeError {
expected: "number (either int or float)",
actual: v.type_of(),
}));
return frame.error(
self,
ErrorKind::TypeError {
expected: "number (either int or float)",
actual: v.type_of(),
},
);
}
},
@ -740,7 +827,7 @@ impl<'o> VM<'o> {
let resolved = self
.nix_search_path
.resolve(&*self.io_handle, *path)
.with_span(&frame)?;
.with_span(&frame, self)?;
self.stack.push(resolved.into());
}
@ -751,9 +838,12 @@ impl<'o> VM<'o> {
Value::UnresolvedPath(path) => {
match dirs::home_dir() {
None => {
return Err(frame.error(ErrorKind::RelativePathResolution(
"failed to determine home directory".into(),
)));
return frame.error(
self,
ErrorKind::RelativePathResolution(
"failed to determine home directory".into(),
),
);
}
Some(mut buf) => {
buf.push(*path);
@ -774,7 +864,7 @@ impl<'o> VM<'o> {
}
OpCode::OpAssertFail => {
return Err(frame.error(ErrorKind::AssertionFailed));
frame.error(self, ErrorKind::AssertionFailed)?;
}
// Data-carrying operands should never be executed,
@ -802,7 +892,7 @@ impl<'o> VM<'o> {
fn run_attrset(&mut self, frame: &CallFrame, count: usize) -> EvalResult<()> {
let attrs = NixAttrs::construct(count, self.stack.split_off(self.stack.len() - count * 2))
.with_span(frame)?;
.with_span(frame, self)?;
self.stack.push(Value::attrs(attrs));
Ok(())
@ -842,7 +932,7 @@ impl<'o> VM<'o> {
let mut out = String::new();
for _ in 0..count {
out.push_str(self.stack_pop().to_str().with_span(frame)?.as_str());
out.push_str(self.stack_pop().to_str().with_span(frame, self)?.as_str());
}
self.stack.push(Value::String(out.into()));
@ -855,12 +945,6 @@ impl<'o> VM<'o> {
self.reasonable_span.clone()
}
/// Construct an error from the given ErrorKind and the source
/// span of the current instruction.
pub fn error(&self, kind: ErrorKind) -> Error {
Error::new(kind, self.reasonable_span.span())
}
/// Apply an argument from the stack to a builtin, and attempt to call it.
///
/// All calls are tail-calls in Tvix, as every function application is a
@ -932,7 +1016,7 @@ impl<'o> VM<'o> {
self.enqueue_generator("__functor call", gen_span, |co| call_functor(co, val));
Ok(())
}
v => Err(self.error(ErrorKind::NotCallable(v.type_of()))),
v => Err(ErrorKind::NotCallable(v.type_of())).with_span(&span, self),
}
}
@ -951,14 +1035,17 @@ impl<'o> VM<'o> {
let val = match self.stack.get(idx) {
Some(val) => val.clone(),
None => {
return Err(frame.error(ErrorKind::TvixBug {
msg: "upvalue to be captured was missing on stack",
metadata: Some(Rc::new(json!({
"ip": format!("{:#x}", frame.ip.0 - 1),
"stack_idx(relative)": stack_idx,
"stack_idx(absolute)": idx,
}))),
}))
return frame.error(
self,
ErrorKind::TvixBug {
msg: "upvalue to be captured was missing on stack",
metadata: Some(Rc::new(json!({
"ip": format!("{:#x}", frame.ip.0 - 1),
"stack_idx(relative)": stack_idx,
"stack_idx(absolute)": idx,
}))),
},
);
}
};