fix(tvix/eval): Propagate catchables in NixAttrs::construct
Correctly propagate the case where the *key* of an attrset is a Value::Catchable (eg { "${builtins.throw "c"}" = "b"; }) in `NixAttrs::construct`, by converting the return type to `Result<Result<Self, CatchableErrorKind>, ErrorKind>` (ugh!!) and correctly handling that everywhere (including an `expect` in the Deserialize impl for NixAttrs, since afaict this is impossible to hit when deserializing from stuff like JSON). Change-Id: Ic4bc611fbfdab27c0bd8a40759689a87c4004a17 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10786 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
This commit is contained in:
parent
7a589b3ec6
commit
3fb0697a71
5 changed files with 31 additions and 13 deletions
|
@ -0,0 +1 @@
|
|||
false
|
|
@ -0,0 +1 @@
|
|||
(builtins.tryEval { "${builtins.throw "a"}" = "b"; }).success
|
|
@ -19,6 +19,7 @@ use super::thunk::ThunkSet;
|
|||
use super::TotalDisplay;
|
||||
use super::Value;
|
||||
use crate::errors::ErrorKind;
|
||||
use crate::CatchableErrorKind;
|
||||
|
||||
lazy_static! {
|
||||
static ref NAME_S: NixString = "name".into();
|
||||
|
@ -170,7 +171,9 @@ impl<'de> Deserialize<'de> for NixAttrs {
|
|||
stack_array.push(value);
|
||||
}
|
||||
|
||||
NixAttrs::construct(stack_array.len() / 2, stack_array).map_err(A::Error::custom)
|
||||
Ok(NixAttrs::construct(stack_array.len() / 2, stack_array)
|
||||
.map_err(A::Error::custom)?
|
||||
.expect("Catchable values are unreachable here"))
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -333,7 +336,10 @@ 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>) -> Result<Self, ErrorKind> {
|
||||
pub fn construct(
|
||||
count: usize,
|
||||
mut stack_slice: Vec<Value>,
|
||||
) -> Result<Result<Self, CatchableErrorKind>, ErrorKind> {
|
||||
debug_assert!(
|
||||
stack_slice.len() == count * 2,
|
||||
"construct_attrs called with count == {}, but slice.len() == {}",
|
||||
|
@ -343,13 +349,13 @@ impl NixAttrs {
|
|||
|
||||
// Optimisation: Empty attribute set
|
||||
if count == 0 {
|
||||
return Ok(NixAttrs(AttrsRep::Empty));
|
||||
return Ok(Ok(NixAttrs(AttrsRep::Empty)));
|
||||
}
|
||||
|
||||
// Optimisation: KV pattern
|
||||
if count == 2 {
|
||||
if let Some(kv) = attempt_optimise_kv(&mut stack_slice) {
|
||||
return Ok(kv);
|
||||
return Ok(Ok(kv));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -369,11 +375,13 @@ impl NixAttrs {
|
|||
continue;
|
||||
}
|
||||
|
||||
Value::Catchable(err) => return Ok(Err(err)),
|
||||
|
||||
other => return Err(ErrorKind::InvalidAttributeName(other)),
|
||||
}
|
||||
}
|
||||
|
||||
Ok(attrs)
|
||||
Ok(Ok(attrs))
|
||||
}
|
||||
|
||||
/// Construct an optimized "KV"-style attribute set given the value for the
|
||||
|
|
|
@ -4,7 +4,9 @@ use super::*;
|
|||
|
||||
#[test]
|
||||
fn test_empty_attrs() {
|
||||
let attrs = NixAttrs::construct(0, vec![]).expect("empty attr construction should succeed");
|
||||
let attrs = NixAttrs::construct(0, vec![])
|
||||
.expect("empty attr construction should succeed")
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
matches!(attrs, NixAttrs(AttrsRep::Empty)),
|
||||
|
@ -15,7 +17,8 @@ fn test_empty_attrs() {
|
|||
#[test]
|
||||
fn test_simple_attrs() {
|
||||
let attrs = NixAttrs::construct(1, vec![Value::from("key"), Value::from("value")])
|
||||
.expect("simple attr construction should succeed");
|
||||
.expect("simple attr construction should succeed")
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
matches!(attrs, NixAttrs(AttrsRep::Im(_))),
|
||||
|
@ -39,7 +42,8 @@ fn test_kv_attrs() {
|
|||
meaning_val.clone(),
|
||||
],
|
||||
)
|
||||
.expect("constructing K/V pair attrs should succeed");
|
||||
.expect("constructing K/V pair attrs should succeed")
|
||||
.unwrap();
|
||||
|
||||
match kv_attrs {
|
||||
NixAttrs(AttrsRep::KV { name, value })
|
||||
|
@ -55,7 +59,7 @@ fn test_kv_attrs() {
|
|||
|
||||
#[test]
|
||||
fn test_empty_attrs_iter() {
|
||||
let attrs = NixAttrs::construct(0, vec![]).unwrap();
|
||||
let attrs = NixAttrs::construct(0, vec![]).unwrap().unwrap();
|
||||
assert!(attrs.iter().next().is_none());
|
||||
}
|
||||
|
||||
|
@ -75,7 +79,8 @@ fn test_kv_attrs_iter() {
|
|||
meaning_val.clone(),
|
||||
],
|
||||
)
|
||||
.expect("constructing K/V pair attrs should succeed");
|
||||
.expect("constructing K/V pair attrs should succeed")
|
||||
.unwrap();
|
||||
|
||||
let mut iter = kv_attrs.iter().collect::<Vec<_>>().into_iter();
|
||||
let (k, v) = iter.next().unwrap();
|
||||
|
@ -90,7 +95,8 @@ fn test_kv_attrs_iter() {
|
|||
#[test]
|
||||
fn test_map_attrs_iter() {
|
||||
let attrs = NixAttrs::construct(1, vec![Value::from("key"), Value::from("value")])
|
||||
.expect("simple attr construction should succeed");
|
||||
.expect("simple attr construction should succeed")
|
||||
.unwrap();
|
||||
|
||||
let mut iter = attrs.iter().collect::<Vec<_>>().into_iter();
|
||||
let (k, v) = iter.next().unwrap();
|
||||
|
|
|
@ -932,9 +932,11 @@ where
|
|||
|
||||
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, self)?;
|
||||
.with_span(frame, self)?
|
||||
.map(Value::attrs)
|
||||
.into();
|
||||
|
||||
self.stack.push(Value::attrs(attrs));
|
||||
self.stack.push(attrs);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue