From 05f42519b53575ad3235b5e0a0cd7d71f04076a5 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sat, 9 Sep 2023 22:02:56 -0700 Subject: fix(tvix/eval): fix b/281 by adding Value::Catchable This commit makes catchable errors a variant of Value. The main downside of this approach is that we lose the ability to use Rust's `?` syntax for propagating catchable errors. Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289 Reviewed-by: tazjin Autosubmit: Adam Joseph Tested-by: BuildkiteCI --- tvix/eval/src/vm/generators.rs | 28 ++++++------- tvix/eval/src/vm/mod.rs | 92 ++++++++++++------------------------------ 2 files changed, 38 insertions(+), 82 deletions(-) (limited to 'tvix/eval/src/vm') diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index a195dac24f..f9c5786d8f 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -190,10 +190,6 @@ pub enum VMResponse { /// VM response with a span to use at the current point. Span(LightSpan), - - /// Message returned by the VM when a catchable error is encountered during - /// the evaluation of `builtins.tryEval`. - ForceError, } impl Display for VMResponse { @@ -204,7 +200,6 @@ impl Display for VMResponse { VMResponse::Path(p) => write!(f, "path({})", p.to_string_lossy()), VMResponse::Directory(d) => write!(f, "dir(len = {})", d.len()), VMResponse::Span(_) => write!(f, "span"), - VMResponse::ForceError => write!(f, "force_error"), } } } @@ -539,20 +534,18 @@ pub async fn request_force(co: &GenCo, val: Value) -> Value { } } -/// Force a value, but inform the caller (by returning `None`) if a catchable -/// error occured. -pub(crate) async fn request_try_force(co: &GenCo, val: Value) -> Option { +/// Force a value +pub(crate) async fn request_try_force(co: &GenCo, val: Value) -> Value { if let Value::Thunk(_) = val { match co.yield_(VMRequest::TryForce(val)).await { - VMResponse::Value(value) => Some(value), - VMResponse::ForceError => None, + VMResponse::Value(value) => value, msg => panic!( "Tvix bug: VM responded with incorrect generator message: {}", msg ), } } else { - Some(val) + val } } @@ -592,13 +585,18 @@ where callable } -pub async fn request_string_coerce(co: &GenCo, val: Value, kind: CoercionKind) -> NixString { +pub async fn request_string_coerce( + co: &GenCo, + val: Value, + kind: CoercionKind, +) -> Result { match val { - Value::String(s) => s, + Value::String(s) => Ok(s), _ => match co.yield_(VMRequest::StringCoerce(val, kind)).await { - VMResponse::Value(value) => value + VMResponse::Value(Value::Catchable(c)) => Err(c), + VMResponse::Value(value) => Ok(value .to_str() - .expect("coerce_to_string always returns a string"), + .expect("coerce_to_string always returns a string")), msg => panic!( "Tvix bug: VM responded with incorrect generator message: {}", msg diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index 07d3725fd9..d8f38718c6 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -84,13 +84,6 @@ impl WithSpan for Result { 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() { @@ -360,8 +353,6 @@ impl<'o> VM<'o> { /// Run the VM's primary (outer) execution loop, continuing execution based /// on the current frame at the top of the frame stack. fn execute(mut self) -> EvalResult { - let mut catchable_error_occurred = false; - while let Some(frame) = self.frames.pop() { self.reasonable_span = frame.span(); let frame_id = self.frames.len(); @@ -377,21 +368,7 @@ impl<'o> VM<'o> { .observer .observe_suspend_call_frame(frame_id, &self.stack), - Err(err) => { - if let Some(catching_frame_idx) = self.try_eval_frames.pop() { - if err.kind.is_catchable() { - self.observer.observe_exit_call_frame(frame_id, &self.stack); - catchable_error_occurred = true; - - // truncate the frame stack back to the - // frame that can catch this error - self.frames.truncate(/* len = */ catching_frame_idx + 1); - continue; - } - } - - return Err(err); - } + Err(err) => return Err(err), }; } @@ -406,14 +383,7 @@ impl<'o> VM<'o> { self.observer .observe_enter_generator(frame_id, name, &self.stack); - let initial_msg = if catchable_error_occurred { - catchable_error_occurred = false; - Some(VMResponse::ForceError) - } else { - None - }; - - match self.run_generator(name, span, frame_id, state, generator, initial_msg) { + match self.run_generator(name, span, frame_id, state, generator, None) { Ok(true) => { self.observer .observe_exit_generator(frame_id, name, &self.stack) @@ -423,25 +393,7 @@ impl<'o> VM<'o> { .observe_suspend_generator(frame_id, name, &self.stack) } - Err(err) => { - if let Some(catching_frame_idx) = self.try_eval_frames.pop() { - if err.kind.is_catchable() { - self.observer.observe_exit_generator( - frame_id, - name, - &self.stack, - ); - catchable_error_occurred = true; - - // truncate the frame stack back to the - // frame that can catch this error - self.frames.truncate(/* len = */ catching_frame_idx + 1); - continue; - } - } - - return Err(err); - } + Err(err) => return Err(err), }; } } @@ -449,12 +401,12 @@ impl<'o> VM<'o> { // Once no more frames are present, return the stack's top value as the // result. + let value = self + .stack + .pop() + .expect("tvix bug: runtime stack empty after execution"); Ok(RuntimeResult { - value: self - .stack - .pop() - .expect("tvix bug: runtime stack empty after execution"), - + value: value, warnings: self.warnings, }) } @@ -925,10 +877,8 @@ impl<'o> VM<'o> { } OpCode::OpAssertFail => { - frame.error( - self, - ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed), - )?; + self.stack + .push(Value::Catchable(CatchableErrorKind::AssertionFailed)); } // Data-carrying operands should never be executed, @@ -1214,18 +1164,26 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result { let result = match (a, b) { (Value::Path(p), v) => { let mut path = p.to_string_lossy().into_owned(); - let vs = generators::request_string_coerce(&co, v, CoercionKind::Weak).await; - path.push_str(vs.as_str()); - crate::value::canon_path(PathBuf::from(path)).into() + match generators::request_string_coerce(&co, v, CoercionKind::Weak).await { + Ok(vs) => { + path.push_str(vs.as_str()); + crate::value::canon_path(PathBuf::from(path)).into() + } + Err(c) => Value::Catchable(c), + } } (Value::String(s1), Value::String(s2)) => Value::String(s1.concat(&s2)), (Value::String(s1), v) => Value::String( - s1.concat(&generators::request_string_coerce(&co, v, CoercionKind::Weak).await), + match generators::request_string_coerce(&co, v, CoercionKind::Weak).await { + Ok(s2) => s1.concat(&s2), + Err(c) => return Ok(Value::Catchable(c)), + }, ), (v, Value::String(s2)) => Value::String( - generators::request_string_coerce(&co, v, CoercionKind::Weak) - .await - .concat(&s2), + match generators::request_string_coerce(&co, v, CoercionKind::Weak).await { + Ok(s1) => s1.concat(&s2), + Err(c) => return Ok(Value::Catchable(c)), + }, ), (a, b) => arithmetic_op!(&a, &b, +)?, }; -- cgit 1.4.1