maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(tvix/eval): remove impl PartialEq for Value

Browse source 
It isn't possible to implement PartialEq properly for Value, because
any sensible implementation needs to force() thunks, which cannot be
done without a `&mut VM`.

The existing derive(PartialEq) has false negatives, which caused the
bug which cl/7142 fixed.  Fortunately that bug was easy to find, but
a silent false negative deep within the bowels of nixpkgs could be a
real nightmare to hunt down.

Let's just remove the PartialEq impl for Value, and the other
derive(PartialEq)'s that depend on it.

Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Iacd3726fefc7fc1edadcd7e9b586e04cf8466775
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7144
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit is contained in:
Adam Joseph 2022-10-31 17:13:38 -07:00 • committed by clbot
parent 73fb474752
commit 0649474206
8 changed files with 31 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

35
tvix/eval/src/value/attrs/tests.rs
View file

@ -80,8 +80,9 @@ fn test_kv_attrs() {
.expect("constructing K/V pair attrs should succeed");
match kv_attrs {
NixAttrs(AttrsRep::KV { name, value }) if name == meaning_val || value == forty_two_val => {
}
NixAttrs(AttrsRep::KV { name, value })
if name.to_str().unwrap() == meaning_val.to_str().unwrap()
|| value.to_str().unwrap() == forty_two_val.to_str().unwrap() => {}
_ => panic!(
"K/V attribute set should use optimised representation, but got {:?}",
@ -93,7 +94,7 @@ fn test_kv_attrs() {
#[test]
fn test_empty_attrs_iter() {
let attrs = NixAttrs::construct(0, vec![]).unwrap();
assert_eq!(attrs.iter().next(), None);
assert!(attrs.iter().next().is_none());
}
#[test]
@ -114,13 +115,18 @@ fn test_kv_attrs_iter() {
)
.expect("constructing K/V pair attrs should succeed");
assert_eq!(
kv_attrs.iter().collect::<Vec<_>>(),
vec![
(NixString::NAME_REF, &meaning_val),
(NixString::VALUE_REF, &forty_two_val)
]
);
let mut iter = kv_attrs
.iter()
.collect::<Vec<_>>()
.into_iter()
.map(|(k, v)| (k, v));
let (k, v) = iter.next().unwrap();
assert!(k == NixString::NAME_REF);
assert!(v.to_str().unwrap() == meaning_val.to_str().unwrap());
let (k, v) = iter.next().unwrap();
assert!(k == NixString::VALUE_REF);
assert!(v.as_int().unwrap() == forty_two_val.as_int().unwrap());
assert!(iter.next().is_none());
}
#[test]
@ -131,8 +137,9 @@ fn test_map_attrs_iter() {
)
.expect("simple attr construction should succeed");
assert_eq!(
attrs.iter().collect::<Vec<_>>(),
vec![(&NixString::from("key"), &Value::String("value".into()))],
);
let mut iter = attrs.iter().collect::<Vec<_>>().into_iter();
let (k, v) = iter.next().unwrap();
assert!(k == &NixString::from("key"));
assert!(v.to_str().unwrap().as_str() == "value");
assert!(iter.next().is_none());
}