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 <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit is contained in:
Adam Joseph 2023-09-09 00:10:06 -07:00 committed by clbot
parent 6015604bd8
commit 926459ce69
5 changed files with 57 additions and 29 deletions

View file

@ -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<Value, ErrorKind> {
Err(ErrorKind::Throw(message.to_str()?.to_string()))
Err(ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(
message.to_str()?.to_string(),
)))
}
#[builtin("toString")]

View file

@ -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)];

View file

@ -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<ErrorKind>,
},
CatchableErrorKind(CatchableErrorKind),
}
impl From<CatchableErrorKind> 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",

View file

@ -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!(
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:?}"
);
}

View file

@ -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,