feat(tvix/eval): ensure all errors always carry a span

Previously error spans were optional because the information about
code spans was not available at runtime. Now that this information has
been added, the error type will always carry a span.

This change is very invasive all throughout the codebase. This is due
to the fact that many functions that are called *by* the VM expected
to return `EvalResult`, but this no longer works as the span
information is not available to those functions - only to the VM
itself.

To work around this the majority of these functions have been changed
to return `Result<T, ErrorKind>` instead and an accompanying macro in
the VM constructs the "real" error.

Note that this implementatino currently has a bug where errors
occuring within thunks will yield the location at which the thunk was
forced, not the location at which the error occured within the code.
This will be fixed soon, but the commit is large enough as is.

Change-Id: Ib1ecb81a4d09d464a95ea7ea9e589f3bd08d5202
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6408
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI

tvix/eval/src/builtins/mod.rs

@@ -20,12 +20,12 @@ fn pure_builtins() -> Vec<Builtin> {
         Builtin::new("add", 2, |mut args| {
             let b = args.pop().unwrap();
             let a = args.pop().unwrap();
-            Ok(arithmetic_op!(a, b, +))
+            arithmetic_op!(a, b, +)
         }),
         Builtin::new("abort", 1, |mut args| {
-            return Err(
+            return Err(ErrorKind::Abort(
-                ErrorKind::Abort(args.pop().unwrap().to_string()?.as_str().to_owned()).into(),
+                args.pop().unwrap().to_string()?.as_str().to_owned(),
-            );
+            ));
         }),
         Builtin::new("catAttrs", 2, |mut args| {
             let list = args.pop().unwrap().to_list()?;
@@ -43,7 +43,7 @@ fn pure_builtins() -> Vec<Builtin> {
         Builtin::new("div", 2, |mut args| {
             let b = args.pop().unwrap();
             let a = args.pop().unwrap();
-            Ok(arithmetic_op!(a, b, /))
+            arithmetic_op!(a, b, /)
         }),
         Builtin::new("length", 1, |args| {
             Ok(Value::Integer(args[0].as_list()?.len() as i64))
@@ -81,17 +81,17 @@ fn pure_builtins() -> Vec<Builtin> {
         Builtin::new("mul", 2, |mut args| {
             let b = args.pop().unwrap();
             let a = args.pop().unwrap();
-            Ok(arithmetic_op!(a, b, *))
+            arithmetic_op!(a, b, *)
         }),
         Builtin::new("sub", 2, |mut args| {
             let b = args.pop().unwrap();
             let a = args.pop().unwrap();
-            Ok(arithmetic_op!(a, b, -))
+            arithmetic_op!(a, b, -)
         }),
         Builtin::new("throw", 1, |mut args| {
-            return Err(
+            return Err(ErrorKind::Throw(
-                ErrorKind::Throw(args.pop().unwrap().to_string()?.as_str().to_owned()).into(),
+                args.pop().unwrap().to_string()?.as_str().to_owned(),
-            );
+            ));
         }),
         Builtin::new("toString", 1, |args| {
             // TODO: toString is actually not the same as Display

tvix/eval/src/compiler/mod.rs

@@ -1217,10 +1217,7 @@ impl Compiler<'_> {
     }
 
     fn emit_error(&mut self, span: codemap::Span, kind: ErrorKind) {
-        self.errors.push(Error {
+        self.errors.push(Error { kind, span })
-            kind,
-            span: Some(span),
-        })
     }
 
     /// Convert a non-dynamic string expression to a string if possible,
@@ -1234,7 +1231,7 @@ impl Compiler<'_> {
 
         return Err(Error {
             kind: ErrorKind::DynamicKeyInLet(expr.syntax().clone()),
-            span: Some(self.span_for(&expr)),
+            span: self.span_for(&expr),
         });
     }
 
@@ -1253,7 +1250,7 @@ impl Compiler<'_> {
                 ast::Expr::Str(s) => self.expr_str_to_string(s),
                 _ => Err(Error {
                     kind: ErrorKind::DynamicKeyInLet(node.syntax().clone()),
-                    span: Some(self.span_for(&node)),
+                    span: self.span_for(&node),
                 }),
             },
         }
@@ -1319,8 +1316,12 @@ pub fn compile<'code>(
 ) -> EvalResult<CompilationOutput> {
     let mut root_dir = match location {
         Some(dir) => Ok(dir),
-        None => std::env::current_dir().map_err(|e| {
+        None => std::env::current_dir().map_err(|e| Error {
-            ErrorKind::PathResolution(format!("could not determine current directory: {}", e))
+            kind: ErrorKind::PathResolution(format!(
+                "could not determine current directory: {}",
+                e
+            )),
+            span: file.span,
         }),
     }?;
 

tvix/eval/src/errors.rs

@@ -53,13 +53,7 @@ pub enum ErrorKind {
 #[derive(Clone, Debug)]
 pub struct Error {
     pub kind: ErrorKind,
-    pub span: Option<codemap::Span>,
+    pub span: codemap::Span,
-}
-
-impl From<ErrorKind> for Error {
-    fn from(kind: ErrorKind) -> Self {
-        Error { span: None, kind }
-    }
 }
 
 impl Display for Error {

tvix/eval/src/eval.rs

@@ -2,7 +2,7 @@ use std::path::PathBuf;
 
 use crate::{
     builtins::global_builtins,
-    errors::{ErrorKind, EvalResult},
+    errors::{Error, ErrorKind, EvalResult},
     value::Value,
 };
 
@@ -23,7 +23,10 @@ pub fn interpret(code: &str, location: Option<PathBuf>) -> EvalResult<Value> {
         for err in errors {
             eprintln!("parse error: {}", err);
         }
-        return Err(ErrorKind::ParseErrors(errors.to_vec()).into());
+        return Err(Error {
+            kind: ErrorKind::ParseErrors(errors.to_vec()).into(),
+            span: file.span,
+        });
     }
 
     // If we've reached this point, there are no errors.
@@ -54,8 +57,8 @@ pub fn interpret(code: &str, location: Option<PathBuf>) -> EvalResult<Value> {
         eprintln!(
             "compiler error: {:?} at `{}`[line {}]",
             error.kind,
-            file.source_slice(error.span.expect("TODO: non-optional")),
+            file.source_slice(error.span),
-            file.find_line(error.span.unwrap().low()) + 1
+            file.find_line(error.span.low()) + 1
         );
     }
 

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::{ErrorKind, EvalResult};
+use crate::errors::ErrorKind;
 
 use super::string::NixString;
 use super::Value;
@@ -236,7 +236,7 @@ impl NixAttrs {
 
     /// Implement construction logic of an attribute set, to encapsulate
     /// logic about attribute set optimisations inside of this module.
-    pub fn construct(count: usize, mut stack_slice: Vec<Value>) -> EvalResult<Self> {
+    pub fn construct(count: usize, mut stack_slice: Vec<Value>) -> Result<Self, ErrorKind> {
         debug_assert!(
             stack_slice.len() == count * 2,
             "construct_attrs called with count == {}, but slice.len() == {}",
@@ -342,12 +342,11 @@ fn attempt_optimise_kv(slice: &mut [Value]) -> Option<NixAttrs> {
 
 // Set an attribute on an in-construction attribute set, while
 // checking against duplicate keys.
-fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> EvalResult<()> {
+fn set_attr(attrs: &mut NixAttrs, key: NixString, value: Value) -> Result<(), ErrorKind> {
     match attrs.0.map_mut().entry(key) {
         btree_map::Entry::Occupied(entry) => Err(ErrorKind::DuplicateAttrsKey {
             key: entry.key().as_str().to_string(),
-        }
+        }),
-        .into()),
 
         btree_map::Entry::Vacant(entry) => {
             entry.insert(value);
@@ -367,7 +366,7 @@ fn set_nested_attr(
     key: NixString,
     mut path: Vec<NixString>,
     value: Value,
-) -> EvalResult<()> {
+) -> Result<(), ErrorKind> {
     // If there is no next key we are at the point where we
     // should insert the value itself.
     if path.is_empty() {
@@ -408,8 +407,7 @@ fn set_nested_attr(
             _ => {
                 return Err(ErrorKind::DuplicateAttrsKey {
                     key: entry.key().as_str().to_string(),
-                }
+                })
-                .into())
             }
         },
     }

tvix/eval/src/value/builtin.rs

@@ -3,13 +3,13 @@
 //!
 //! Builtins are directly backed by Rust code operating on Nix values.
 
-use crate::errors::EvalResult;
+use crate::errors::ErrorKind;
 
 use super::Value;
 
 use std::fmt::{Debug, Display};
 
-pub type BuiltinFn = fn(arg: Vec<Value>) -> EvalResult<Value>;
+pub type BuiltinFn = fn(arg: Vec<Value>) -> Result<Value, ErrorKind>;
 
 /// Represents a single built-in function which directly executes Rust
 /// code that operates on a Nix value.
@@ -50,7 +50,7 @@ impl Builtin {
     /// Apply an additional argument to the builtin, which will either
     /// lead to execution of the function or to returning a partial
     /// builtin.
-    pub fn apply(mut self, arg: Value) -> EvalResult<Value> {
+    pub fn apply(mut self, arg: Value) -> Result<Value, ErrorKind> {
         self.partials.push(arg);
 
         if self.partials.len() == self.arity {

tvix/eval/src/value/mod.rs

@@ -10,7 +10,7 @@ mod list;
 mod string;
 mod thunk;
 
-use crate::errors::{ErrorKind, EvalResult};
+use crate::errors::ErrorKind;
 use crate::opcode::StackIdx;
 pub use attrs::NixAttrs;
 pub use builtin::Builtin;
@@ -70,91 +70,59 @@ impl Value {
         }
     }
 
-    pub fn as_bool(&self) -> EvalResult<bool> {
+    pub fn as_bool(&self) -> Result<bool, ErrorKind> {
         match self {
             Value::Bool(b) => Ok(*b),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("bool", &other)),
-                expected: "bool",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn as_attrs(&self) -> EvalResult<&NixAttrs> {
+    pub fn as_attrs(&self) -> Result<&NixAttrs, ErrorKind> {
         match self {
             Value::Attrs(attrs) => Ok(attrs),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("set", &other)),
-                expected: "set",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn as_str(&self) -> EvalResult<&str> {
+    pub fn as_str(&self) -> Result<&str, ErrorKind> {
         match self {
             Value::String(s) => Ok(s.as_str()),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("string", &other)),
-                expected: "string",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn as_list(&self) -> EvalResult<&NixList> {
+    pub fn as_list(&self) -> Result<&NixList, ErrorKind> {
         match self {
             Value::List(xs) => Ok(xs),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("list", &other)),
-                expected: "list",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn to_string(self) -> EvalResult<NixString> {
+    pub fn to_string(self) -> Result<NixString, ErrorKind> {
         match self {
             Value::String(s) => Ok(s),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("string", &other)),
-                expected: "string",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn to_attrs(self) -> EvalResult<Rc<NixAttrs>> {
+    pub fn to_attrs(self) -> Result<Rc<NixAttrs>, ErrorKind> {
         match self {
             Value::Attrs(s) => Ok(s),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("set", &other)),
-                expected: "set",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn to_list(self) -> EvalResult<NixList> {
+    pub fn to_list(self) -> Result<NixList, ErrorKind> {
         match self {
             Value::List(l) => Ok(l),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("list", &other)),
-                expected: "list",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
-    pub fn to_closure(self) -> EvalResult<Closure> {
+    pub fn to_closure(self) -> Result<Closure, ErrorKind> {
         match self {
             Value::Closure(c) => Ok(c),
-            other => Err(ErrorKind::TypeError {
+            other => Err(type_error("lambda", &other)),
-                expected: "lambda",
-                actual: other.type_of(),
-            }
-            .into()),
         }
     }
 
@@ -231,3 +199,10 @@ impl PartialEq for Value {
         }
     }
 }
+
+fn type_error(expected: &'static str, actual: &Value) -> ErrorKind {
+    ErrorKind::TypeError {
+        expected,
+        actual: actual.type_of(),
+    }
+}

tvix/eval/src/value/thunk.rs

@@ -24,7 +24,7 @@ use std::{
     rc::Rc,
 };
 
-use crate::{errors::ErrorKind, upvalues::UpvalueCarrier, vm::VM, EvalResult, Value};
+use crate::{errors::ErrorKind, upvalues::UpvalueCarrier, vm::VM, Value};
 
 use super::Lambda;
 
@@ -64,7 +64,7 @@ impl Thunk {
     /// to it, providing memoization) through interior mutability. In
     /// case of nested thunks, the intermediate thunk representations
     /// are replaced.
-    pub fn force(&self, vm: &mut VM) -> EvalResult<()> {
+    pub fn force(&self, vm: &mut VM) -> Result<(), ErrorKind> {
         // Due to mutable borrowing rules, the following code can't
         // easily use a match statement or something like that; it
         // requires a bit of manual fiddling.
@@ -78,14 +78,16 @@ impl Thunk {
                 }
 
                 ThunkRepr::Evaluated(_) => return Ok(()),
-                ThunkRepr::Blackhole => return Err(ErrorKind::InfiniteRecursion.into()),
+                ThunkRepr::Blackhole => return Err(ErrorKind::InfiniteRecursion),
 
                 ThunkRepr::Suspended { .. } => {
                     if let ThunkRepr::Suspended { lambda, upvalues } =
                         std::mem::replace(&mut *thunk_mut, ThunkRepr::Blackhole)
                     {
                         vm.call(lambda, upvalues, 0);
-                        *thunk_mut = ThunkRepr::Evaluated(vm.run()?);
+                        // TODO: find a cheap way to actually retain
+                        // the original error span
+                        *thunk_mut = ThunkRepr::Evaluated(vm.run().map_err(|e| e.kind)?);
                     }
                 }
             }

tvix/eval/src/vm.rs

@@ -6,7 +6,7 @@ use std::{cell::RefMut, rc::Rc};
 use crate::{
     chunk::Chunk,
     errors::{Error, ErrorKind, EvalResult},
-    opcode::{ConstantIdx, Count, JumpOffset, OpCode, StackIdx, UpvalueIdx},
+    opcode::{CodeIdx, ConstantIdx, Count, JumpOffset, OpCode, StackIdx, UpvalueIdx},
     upvalues::UpvalueCarrier,
     value::{Closure, Lambda, NixAttrs, NixList, Thunk, Value},
 };
@@ -37,30 +37,50 @@ pub struct VM {
     with_stack: Vec<usize>,
 }
 
+/// This macro wraps a computation that returns an ErrorKind or a
+/// result, and wraps the ErrorKind in an Error struct if present.
+///
+/// The reason for this macro's existence is that calculating spans is
+/// potentially expensive, so it should be avoided to the last moment
+/// (i.e. definite instantiation of a runtime error) if possible.
+macro_rules! fallible {
+    ( $self:ident, $body:expr) => {
+        match $body {
+            Ok(result) => result,
+            Err(kind) => {
+                return Err(Error {
+                    kind,
+                    span: $self.current_span(),
+                })
+            }
+        }
+    };
+}
+
 #[macro_export]
 macro_rules! arithmetic_op {
     ( $self:ident, $op:tt ) => {{
         let b = $self.pop();
         let a = $self.pop();
-        let result = arithmetic_op!(a, b, $op);
+        let result = fallible!($self, arithmetic_op!(a, b, $op));
         $self.push(result);
     }};
 
     ( $a:ident, $b:ident, $op:tt ) => {{
         match ($a, $b) {
-            (Value::Integer(i1), Value::Integer(i2)) => Value::Integer(i1 $op i2),
+            (Value::Integer(i1), Value::Integer(i2)) => Ok(Value::Integer(i1 $op i2)),
-            (Value::Float(f1), Value::Float(f2)) => Value::Float(f1 $op f2),
+            (Value::Float(f1), Value::Float(f2)) => Ok(Value::Float(f1 $op f2)),
-            (Value::Integer(i1), Value::Float(f2)) => Value::Float(i1 as f64 $op f2),
+            (Value::Integer(i1), Value::Float(f2)) => Ok(Value::Float(i1 as f64 $op f2)),
-            (Value::Float(f1), Value::Integer(i2)) => Value::Float(f1 $op i2 as f64),
+            (Value::Float(f1), Value::Integer(i2)) => Ok(Value::Float(f1 $op i2 as f64)),
 
-            (v1, v2) => return Err(ErrorKind::TypeError {
+            (v1, v2) => Err(ErrorKind::TypeError {
                 expected: "number (either int or float)",
                 actual: if v1.is_number() {
                     v2.type_of()
                 } else {
                     v1.type_of()
                 },
-            }.into()),
+            }),
         }
     }};
 }
@@ -80,10 +100,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(ErrorKind::Incomparable {
+            (lhs, rhs) => return Err($self.error(ErrorKind::Incomparable {
                 lhs: lhs.type_of(),
                 rhs: rhs.type_of(),
-            }.into()),
+            })),
         };
 
         $self.push(Value::Bool(result));
@@ -126,6 +146,21 @@ impl VM {
         &self.stack[self.stack.len() - 1 - offset]
     }
 
+    /// Returns the source span of the instruction currently being
+    /// executed.
+    fn current_span(&self) -> codemap::Span {
+        self.chunk().get_span(CodeIdx(self.frame().ip - 1))
+    }
+
+    /// Construct an error from the given ErrorKind and the source
+    /// span of the current instruction.
+    fn error(&self, kind: ErrorKind) -> Error {
+        Error {
+            kind,
+            span: self.current_span(),
+        }
+    }
+
     pub fn call(&mut self, lambda: Rc<Lambda>, upvalues: Vec<Value>, arg_count: usize) {
         let frame = CallFrame {
             lambda,
@@ -171,7 +206,7 @@ impl VM {
                     let result = if let (Value::String(s1), Value::String(s2)) = (&a, &b) {
                         Value::String(s1.concat(s2))
                     } else {
-                        arithmetic_op!(a, b, +)
+                        fallible!(self, arithmetic_op!(a, b, +))
                     };
 
                     self.push(result)
@@ -182,7 +217,7 @@ impl VM {
                 OpCode::OpDiv => arithmetic_op!(self, /),
 
                 OpCode::OpInvert => {
-                    let v = self.pop().as_bool()?;
+                    let v = fallible!(self, self.pop().as_bool());
                     self.push(Value::Bool(!v));
                 }
 
@@ -190,11 +225,10 @@ impl VM {
                     Value::Integer(i) => self.push(Value::Integer(-i)),
                     Value::Float(f) => self.push(Value::Float(-f)),
                     v => {
-                        return Err(ErrorKind::TypeError {
+                        return Err(self.error(ErrorKind::TypeError {
                             expected: "number (either int or float)",
                             actual: v.type_of(),
-                        }
+                        }));
-                        .into())
                     }
                 },
 
@@ -218,30 +252,29 @@ impl VM {
                 OpCode::OpAttrPath(Count(count)) => self.run_attr_path(count)?,
 
                 OpCode::OpAttrsUpdate => {
-                    let rhs = unwrap_or_clone_rc(self.pop().to_attrs()?);
+                    let rhs = unwrap_or_clone_rc(fallible!(self, self.pop().to_attrs()));
-                    let lhs = unwrap_or_clone_rc(self.pop().to_attrs()?);
+                    let lhs = unwrap_or_clone_rc(fallible!(self, self.pop().to_attrs()));
 
                     self.push(Value::Attrs(Rc::new(lhs.update(rhs))))
                 }
 
                 OpCode::OpAttrsSelect => {
-                    let key = self.pop().to_string()?;
+                    let key = fallible!(self, self.pop().to_string());
-                    let attrs = self.pop().to_attrs()?;
+                    let attrs = fallible!(self, self.pop().to_attrs());
 
                     match attrs.select(key.as_str()) {
                         Some(value) => self.push(value.clone()),
 
                         None => {
-                            return Err(ErrorKind::AttributeNotFound {
+                            return Err(self.error(ErrorKind::AttributeNotFound {
                                 name: key.as_str().to_string(),
-                            }
+                            }))
-                            .into())
                         }
                     }
                 }
 
                 OpCode::OpAttrsTrySelect => {
-                    let key = self.pop().to_string()?;
+                    let key = fallible!(self, self.pop().to_string());
                     let value = match self.pop() {
                         Value::Attrs(attrs) => match attrs.select(key.as_str()) {
                             Some(value) => value.clone(),
@@ -255,7 +288,7 @@ impl VM {
                 }
 
                 OpCode::OpAttrsIsSet => {
-                    let key = self.pop().to_string()?;
+                    let key = fallible!(self, self.pop().to_string());
                     let result = match self.pop() {
                         Value::Attrs(attrs) => attrs.contains(key.as_str()),
 
@@ -274,8 +307,8 @@ impl VM {
                 }
 
                 OpCode::OpConcat => {
-                    let rhs = self.pop().to_list()?;
+                    let rhs = fallible!(self, self.pop().to_list());
-                    let lhs = self.pop().to_list()?;
+                    let lhs = fallible!(self, self.pop().to_list());
                     self.push(Value::List(lhs.concat(&rhs)))
                 }
 
@@ -286,13 +319,13 @@ impl VM {
                 }
 
                 OpCode::OpJumpIfTrue(JumpOffset(offset)) => {
-                    if self.peek(0).as_bool()? {
+                    if fallible!(self, self.peek(0).as_bool()) {
                         self.frame_mut().ip += offset;
                     }
                 }
 
                 OpCode::OpJumpIfFalse(JumpOffset(offset)) => {
-                    if !self.peek(0).as_bool()? {
+                    if !fallible!(self, self.peek(0).as_bool()) {
                         self.frame_mut().ip += offset;
                     }
                 }
@@ -311,11 +344,10 @@ impl VM {
                 OpCode::OpAssertBool => {
                     let val = self.peek(0);
                     if !val.is_bool() {
-                        return Err(ErrorKind::TypeError {
+                        return Err(self.error(ErrorKind::TypeError {
                             expected: "bool",
                             actual: val.type_of(),
-                        }
+                        }));
-                        .into());
                     }
                 }
 
@@ -347,13 +379,13 @@ impl VM {
                 }
 
                 OpCode::OpResolveWith => {
-                    let ident = self.pop().to_string()?;
+                    let ident = fallible!(self, self.pop().to_string());
                     let value = self.resolve_with(ident.as_str())?;
                     self.push(value)
                 }
 
                 OpCode::OpResolveWithOrUpvalue(idx) => {
-                    let ident = self.pop().to_string()?;
+                    let ident = fallible!(self, self.pop().to_string());
                     match self.resolve_with(ident.as_str()) {
                         // Variable found in local `with`-stack.
                         Ok(value) => self.push(value),
@@ -372,8 +404,8 @@ impl VM {
                 }
 
                 OpCode::OpAssert => {
-                    if !self.pop().as_bool()? {
+                    if !fallible!(self, self.pop().as_bool()) {
-                        return Err(ErrorKind::AssertionFailed.into());
+                        return Err(self.error(ErrorKind::AssertionFailed));
                     }
                 }
 
@@ -386,19 +418,18 @@ impl VM {
 
                         Value::Builtin(builtin) => {
                             let arg = self.pop();
-                            let result = builtin.apply(arg)?;
+                            let result = fallible!(self, builtin.apply(arg));
                             self.push(result);
                         }
-                        _ => return Err(ErrorKind::NotCallable.into()),
+                        _ => return Err(self.error(ErrorKind::NotCallable)),
                     };
                 }
 
                 OpCode::OpGetUpvalue(upv_idx) => {
                     let value = self.frame().upvalue(upv_idx).clone();
                     if let Value::DynamicUpvalueMissing(name) = value {
-                        return Err(
+                        return Err(self
-                            ErrorKind::UnknownDynamicVariable(name.as_str().to_string()).into()
+                            .error(ErrorKind::UnknownDynamicVariable(name.as_str().to_string())));
-                        );
                     }
 
                     self.push(value);
@@ -446,7 +477,7 @@ impl VM {
                     let mut value = self.pop();
 
                     if let Value::Thunk(thunk) = value {
-                        thunk.force(self)?;
+                        fallible!(self, thunk.force(self));
                         value = thunk.value().clone();
                     }
 
@@ -498,7 +529,7 @@ impl VM {
         let mut path = Vec::with_capacity(count);
 
         for _ in 0..count {
-            path.push(self.pop().to_string()?);
+            path.push(fallible!(self, self.pop().to_string()));
         }
 
         self.push(Value::AttrPath(path));
@@ -506,7 +537,11 @@ impl VM {
     }
 
     fn run_attrset(&mut self, count: usize) -> EvalResult<()> {
-        let attrs = NixAttrs::construct(count, self.stack.split_off(self.stack.len() - count * 2))?;
+        let attrs = fallible!(
+            self,
+            NixAttrs::construct(count, self.stack.split_off(self.stack.len() - count * 2))
+        );
+
         self.push(Value::Attrs(Rc::new(attrs)));
         Ok(())
     }
@@ -518,7 +553,7 @@ impl VM {
         let mut out = String::new();
 
         for _ in 0..count {
-            out.push_str(self.pop().to_string()?.as_str());
+            out.push_str(fallible!(self, self.pop().to_string()).as_str());
         }
 
         self.push(Value::String(out.into()));
@@ -527,7 +562,7 @@ impl VM {
 
     fn resolve_dynamic_upvalue(&mut self, ident_idx: ConstantIdx) -> EvalResult<Value> {
         let chunk = self.chunk();
-        let ident = chunk.constant(ident_idx).as_str()?.to_string();
+        let ident = fallible!(self, chunk.constant(ident_idx).as_str()).to_string();
 
         // Peek at the current instruction (note: IP has already
         // advanced!) to see if it is actually data indicating a
@@ -560,14 +595,14 @@ impl VM {
     /// Resolve a dynamic identifier through the with-stack at runtime.
     fn resolve_with(&self, ident: &str) -> EvalResult<Value> {
         for idx in self.with_stack.iter().rev() {
-            let with = self.stack[*idx].as_attrs()?;
+            let with = fallible!(self, self.stack[*idx].as_attrs());
             match with.select(ident) {
                 None => continue,
                 Some(val) => return Ok(val.clone()),
             }
         }
 
-        Err(ErrorKind::UnknownDynamicVariable(ident.to_string()).into())
+        Err(self.error(ErrorKind::UnknownDynamicVariable(ident.to_string())))
     }
 
     /// Populate the upvalue fields of a thunk or closure under construction.
@@ -619,7 +654,7 @@ impl VM {
             Value::List(list) => list.iter().try_for_each(|elem| self.force_for_output(elem)),
 
             Value::Thunk(thunk) => {
-                thunk.force(self)?;
+                fallible!(self, thunk.force(self));
                 self.force_for_output(&thunk.value())
             }