From 926459ce694536432c36d8f0d3fb25b821945852 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sat, 9 Sep 2023 00:10:06 -0700 Subject: refactor(tvix/eval): factor CatchableErrorKind out of ErrorKind This commit creates a separate enum for "catchable" errors (the kind that `builtins.tryEval` can detect). Change-Id: Ie81d1112526d852255d9842f67045f88eab192af Reviewed-on: https://cl.tvl.fyi/c/depot/+/9287 Tested-by: BuildkiteCI Reviewed-by: tazjin Autosubmit: Adam Joseph --- tvix/eval/src/builtins/mod.rs | 6 +++-- tvix/eval/src/compiler/mod.rs | 6 +++-- tvix/eval/src/errors.rs | 50 ++++++++++++++++++++++++++-------------- tvix/eval/src/nix_search_path.rs | 17 +++++++++----- tvix/eval/src/vm/mod.rs | 7 ++++-- 5 files changed, 57 insertions(+), 29 deletions(-) (limited to 'tvix/eval') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index d701fa08a08d..322f91710898 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -17,7 +17,7 @@ use crate::vm::generators::{self, GenCo}; use crate::warnings::WarningKind; use crate::{ self as tvix_eval, - errors::ErrorKind, + errors::{CatchableErrorKind, ErrorKind}, value::{CoercionKind, NixAttrs, NixList, NixString, SharedThunkSet, Thunk, Value}, }; @@ -893,7 +893,9 @@ mod pure_builtins { #[builtin("throw")] async fn builtin_throw(co: GenCo, message: Value) -> Result { - Err(ErrorKind::Throw(message.to_str()?.to_string())) + Err(ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw( + message.to_str()?.to_string(), + ))) } #[builtin("toString")] diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 3ab0e4d93360..5c2825507f1e 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -27,7 +27,7 @@ use std::rc::{Rc, Weak}; use std::sync::Arc; use crate::chunk::Chunk; -use crate::errors::{Error, ErrorKind, EvalResult}; +use crate::errors::{CatchableErrorKind, Error, ErrorKind, EvalResult}; use crate::observer::CompilerObserver; use crate::opcode::{CodeIdx, ConstantIdx, Count, JumpOffset, OpCode, UpvalueIdx}; use crate::spans::LightSpan; @@ -398,7 +398,9 @@ impl Compiler<'_> { if raw_path.len() == 2 { return self.emit_error( node, - ErrorKind::NixPathResolution("Empty <> path not allowed".into()), + ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution( + "Empty <> path not allowed".into(), + )), ); } let path = &raw_path[1..(raw_path.len() - 1)]; diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs index 76f55d681299..54def8d334df 100644 --- a/tvix/eval/src/errors.rs +++ b/tvix/eval/src/errors.rs @@ -16,12 +16,19 @@ use crate::spans::ToSpan; use crate::value::{CoercionKind, NixString}; use crate::{SourceCode, Value}; +/// "CatchableErrorKind" errors -- those which can be detected by `builtins.tryEval`. +#[derive(Clone, Debug)] +pub enum CatchableErrorKind { + Throw(String), + AssertionFailed, + /// Resolving a user-supplied angle brackets path literal failed in some way. + NixPathResolution(String), +} + #[derive(Clone, Debug)] pub enum ErrorKind { /// These are user-generated errors through builtins. - Throw(String), Abort(String), - AssertionFailed, DivisionByZero, @@ -55,9 +62,6 @@ pub enum ErrorKind { rhs: &'static str, }, - /// Resolving a user-supplied angle brackets path literal failed in some way. - NixPathResolution(String), - /// Resolving a user-supplied relative or home-relative path literal failed in some way. RelativePathResolution(String), @@ -176,6 +180,14 @@ pub enum ErrorKind { context: String, underlying: Box, }, + + CatchableErrorKind(CatchableErrorKind), +} + +impl From for ErrorKind { + fn from(c: CatchableErrorKind) -> ErrorKind { + ErrorKind::CatchableErrorKind(c) + } } impl error::Error for Error { @@ -235,7 +247,7 @@ impl ErrorKind { /// Returns `true` if this error can be caught by `builtins.tryEval` pub fn is_catchable(&self) -> bool { match self { - Self::Throw(_) | Self::AssertionFailed | Self::NixPathResolution(_) => true, + Self::CatchableErrorKind(_) => true, Self::NativeError { err, .. } | Self::BytecodeError(err) => err.kind.is_catchable(), _ => false, } @@ -285,9 +297,13 @@ impl Error { impl Display for ErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match &self { - ErrorKind::Throw(msg) => write!(f, "error thrown: {}", msg), + ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(msg)) => { + write!(f, "error thrown: {}", msg) + } ErrorKind::Abort(msg) => write!(f, "evaluation aborted: {}", msg), - ErrorKind::AssertionFailed => write!(f, "assertion failed"), + ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed) => { + write!(f, "assertion failed") + } ErrorKind::DivisionByZero => write!(f, "division by zero"), @@ -324,7 +340,8 @@ impl Display for ErrorKind { write!(f, "can not compare a {} with a {}", lhs, rhs) } - ErrorKind::NixPathResolution(err) | ErrorKind::RelativePathResolution(err) => { + ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(err)) + | ErrorKind::RelativePathResolution(err) => { write!(f, "could not resolve path: {}", err) } @@ -724,16 +741,15 @@ impl Error { let label = match &self.kind { ErrorKind::DuplicateAttrsKey { .. } => "in this attribute set", ErrorKind::InvalidAttributeName(_) => "in this attribute set", - ErrorKind::NixPathResolution(_) | ErrorKind::RelativePathResolution(_) => { - "in this path literal" - } + ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(_)) + | ErrorKind::RelativePathResolution(_) => "in this path literal", ErrorKind::UnexpectedArgument { .. } => "in this function call", // The spans for some errors don't have any more descriptive stuff // in them, or we don't utilise it yet. - ErrorKind::Throw(_) + ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(_)) | ErrorKind::Abort(_) - | ErrorKind::AssertionFailed + | ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed) | ErrorKind::AttributeNotFound { .. } | ErrorKind::IndexOutOfBounds { .. } | ErrorKind::TailEmptyList @@ -774,14 +790,14 @@ impl Error { /// used to refer users to documentation. fn code(&self) -> &'static str { match self.kind { - ErrorKind::Throw(_) => "E001", + ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(_)) => "E001", ErrorKind::Abort(_) => "E002", - ErrorKind::AssertionFailed => "E003", + ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed) => "E003", ErrorKind::InvalidAttributeName { .. } => "E004", ErrorKind::AttributeNotFound { .. } => "E005", ErrorKind::TypeError { .. } => "E006", ErrorKind::Incomparable { .. } => "E007", - ErrorKind::NixPathResolution(_) => "E008", + ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(_)) => "E008", ErrorKind::DynamicKeyInScope(_) => "E009", ErrorKind::UnknownStaticVariable => "E010", ErrorKind::UnknownDynamicVariable(_) => "E011", diff --git a/tvix/eval/src/nix_search_path.rs b/tvix/eval/src/nix_search_path.rs index 79c19752f6c1..3e553e62bb85 100644 --- a/tvix/eval/src/nix_search_path.rs +++ b/tvix/eval/src/nix_search_path.rs @@ -3,7 +3,7 @@ use std::convert::Infallible; use std::path::{Path, PathBuf}; use std::str::FromStr; -use crate::errors::ErrorKind; +use crate::errors::{CatchableErrorKind, ErrorKind}; use crate::EvalIO; #[derive(Debug, Clone, PartialEq, Eq)] @@ -134,10 +134,12 @@ impl NixSearchPath { return Ok(p); } } - Err(ErrorKind::NixPathResolution(format!( - "path '{}' was not found in the Nix search path", - path.display() - ))) + Err(ErrorKind::CatchableErrorKind( + CatchableErrorKind::NixPathResolution(format!( + "path '{}' was not found in the Nix search path", + path.display() + )), + )) } } @@ -211,7 +213,10 @@ mod tests { let mut io = StdIO {}; let err = nix_search_path.resolve(&mut io, "nope").unwrap_err(); assert!( - matches!(err, ErrorKind::NixPathResolution(..)), + matches!( + err, + ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(..)) + ), "err = {err:?}" ); } diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index d883b250001d..07d3725fd9d2 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -21,7 +21,7 @@ use crate::{ chunk::Chunk, cmp_op, compiler::GlobalsMap, - errors::{Error, ErrorKind, EvalResult}, + errors::{CatchableErrorKind, Error, ErrorKind, EvalResult}, io::EvalIO, nix_search_path::NixSearchPath, observer::RuntimeObserver, @@ -925,7 +925,10 @@ impl<'o> VM<'o> { } OpCode::OpAssertFail => { - frame.error(self, ErrorKind::AssertionFailed)?; + frame.error( + self, + ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed), + )?; } // Data-carrying operands should never be executed, -- cgit 1.4.1