From 22ffd20c5335f315adad324f8337576f5ad90b43 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 14 Jun 2024 18:42:23 +0200 Subject: [PATCH] refact: Use Option instead of custom ValueTypeOrThunk This loses the custom name for "thunk", but checking thunkness is a niche use case that I don't think we should spend much code on. (cherry picked from commit 7bdff525c13234ce6a32ea9346292d948b0840c1) --- rust/nix-expr/src/eval_state.rs | 46 ++++++++++++++++----------------- rust/nix-expr/src/value.rs | 41 +++++++++++++++-------------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/rust/nix-expr/src/eval_state.rs b/rust/nix-expr/src/eval_state.rs index f9dd0ba..705d8a4 100644 --- a/rust/nix-expr/src/eval_state.rs +++ b/rust/nix-expr/src/eval_state.rs @@ -1,4 +1,4 @@ -use crate::value::{Int, Value, ValueType, ValueTypeOrThunk}; +use crate::value::{Int, Value, ValueType}; use anyhow::Context as _; use anyhow::{bail, Result}; use lazy_static::lazy_static; @@ -133,20 +133,20 @@ impl EvalState { } self.context.check_err() } - pub fn value_type_unforced(&self, value: &Value) -> ValueTypeOrThunk { + pub fn value_type_unforced(&self, value: &Value) -> Option { let r = unsafe { raw::get_type(self.context.ptr(), value.raw_ptr()) }; self.context.check_err().unwrap(); - ValueTypeOrThunk::from_raw(r) + ValueType::from_raw(r) } pub fn value_type(&self, value: &Value) -> Result { match self.value_type_unforced(value) { - ValueTypeOrThunk::ValueType(a) => Ok(a), - ValueTypeOrThunk::Thunk => { + Some(a) => Ok(a), + None => { self.force(value)?; match self.value_type_unforced(value) { - ValueTypeOrThunk::ValueType(a) => Ok(a), - ValueTypeOrThunk::Thunk => { - panic!("values should not be thunks after having been forced.") + Some(a) => Ok(a), + None => { + panic!("Nix value must not be thunk after being forced.") } } } @@ -488,9 +488,9 @@ mod tests { let v2 = v.clone(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::Int)); + assert!(t == Some(ValueType::Int)); let t2 = es.value_type_unforced(&v2); - assert!(t2 == ValueTypeOrThunk::ValueType(ValueType::Int)); + assert!(t2 == Some(ValueType::Int)); gc_now(); }) .unwrap(); @@ -504,7 +504,7 @@ mod tests { let v = es.eval_from_string("true", "").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::Bool)); + assert!(t == Some(ValueType::Bool)); }) .unwrap(); } @@ -532,7 +532,7 @@ mod tests { let v = es.eval_from_string("{ }", "").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::AttrSet)); + assert!(t == Some(ValueType::AttrSet)); let attrs = es.require_attrs_names(&v).unwrap(); assert_eq!(attrs.len(), 0); }) @@ -653,7 +653,7 @@ mod tests { let v = es.eval_from_string("\"hello\"", "").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::String)); + assert!(t == Some(ValueType::String)); let s = es.require_string(&v).unwrap(); assert!(s == "hello"); }) @@ -705,7 +705,7 @@ mod tests { .unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::String)); + assert!(t == Some(ValueType::String)); let r = es.require_string(&v); assert!(r.is_err()); assert!(r @@ -726,7 +726,7 @@ mod tests { .unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::String)); + assert!(t == Some(ValueType::String)); // TODO // let r = es.require_string_without_context(&v); // assert!(r.is_err()); @@ -743,7 +743,7 @@ mod tests { let v = es.new_value_str("hello").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::String)); + assert!(t == Some(ValueType::String)); let s = es.require_string(&v).unwrap(); assert!(s == "hello"); }) @@ -758,7 +758,7 @@ mod tests { let v = es.new_value_str("").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::String)); + assert!(t == Some(ValueType::String)); let s = es.require_string(&v).unwrap(); assert!(s == ""); }) @@ -792,7 +792,7 @@ mod tests { let v = es.new_value_int(42).unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::Int)); + assert!(t == Some(ValueType::Int)); let i = es.require_int(&v).unwrap(); assert!(i == 42); }) @@ -807,7 +807,7 @@ mod tests { let v = es.eval_from_string("{ }", "").unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::AttrSet)); + assert!(t == Some(ValueType::AttrSet)); }) .unwrap(); } @@ -822,7 +822,7 @@ mod tests { .unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::List)); + assert!(t == Some(ValueType::List)); }) .unwrap(); } @@ -884,7 +884,7 @@ mod tests { let v = es.call(f, a).unwrap(); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::Int)); + assert!(t == Some(ValueType::Int)); let i = es.require_int(&v).unwrap(); assert!(i == 3); }) @@ -900,10 +900,10 @@ mod tests { let f = es.eval_from_string("x: x + 1", "").unwrap(); let a = es.eval_from_string("2", "").unwrap(); let v = es.new_value_apply(&f, &a).unwrap(); - assert!(es.value_type_unforced(&v) == ValueTypeOrThunk::Thunk); + assert!(es.value_type_unforced(&v) == None); es.force(&v).unwrap(); let t = es.value_type_unforced(&v); - assert!(t == ValueTypeOrThunk::ValueType(ValueType::Int)); + assert!(t == Some(ValueType::Int)); let i = es.require_int(&v).unwrap(); assert!(i == 3); }) diff --git a/rust/nix-expr/src/value.rs b/rust/nix-expr/src/value.rs index dfc160e..da33c87 100644 --- a/rust/nix-expr/src/value.rs +++ b/rust/nix-expr/src/value.rs @@ -22,28 +22,29 @@ pub enum ValueType { Unknown, } -#[derive(Eq, PartialEq, Debug)] -pub enum ValueTypeOrThunk { - ValueType(ValueType), - Thunk, -} - -impl ValueTypeOrThunk { - pub(crate) fn from_raw(raw: raw::ValueType) -> ValueTypeOrThunk { +impl ValueType { + /// Convert a raw value type to a `ValueType`. + /// + /// Return `None` if the Value is still a thunk (i.e. not yet evaluated). + /// + /// Return `Some(ValueType::Unknown)` if the value type is not recognized. + pub(crate) fn from_raw(raw: raw::ValueType) -> Option { match raw { - raw::ValueType_NIX_TYPE_ATTRS => ValueTypeOrThunk::ValueType(ValueType::AttrSet), - raw::ValueType_NIX_TYPE_BOOL => ValueTypeOrThunk::ValueType(ValueType::Bool), - raw::ValueType_NIX_TYPE_EXTERNAL => ValueTypeOrThunk::ValueType(ValueType::External), - raw::ValueType_NIX_TYPE_FLOAT => ValueTypeOrThunk::ValueType(ValueType::Float), - raw::ValueType_NIX_TYPE_FUNCTION => ValueTypeOrThunk::ValueType(ValueType::Function), - raw::ValueType_NIX_TYPE_INT => ValueTypeOrThunk::ValueType(ValueType::Int), - raw::ValueType_NIX_TYPE_LIST => ValueTypeOrThunk::ValueType(ValueType::List), - raw::ValueType_NIX_TYPE_NULL => ValueTypeOrThunk::ValueType(ValueType::Null), - raw::ValueType_NIX_TYPE_PATH => ValueTypeOrThunk::ValueType(ValueType::Path), - raw::ValueType_NIX_TYPE_STRING => ValueTypeOrThunk::ValueType(ValueType::String), - raw::ValueType_NIX_TYPE_THUNK => ValueTypeOrThunk::Thunk, + raw::ValueType_NIX_TYPE_ATTRS => Some(ValueType::AttrSet), + raw::ValueType_NIX_TYPE_BOOL => Some(ValueType::Bool), + raw::ValueType_NIX_TYPE_EXTERNAL => Some(ValueType::External), + raw::ValueType_NIX_TYPE_FLOAT => Some(ValueType::Float), + raw::ValueType_NIX_TYPE_FUNCTION => Some(ValueType::Function), + raw::ValueType_NIX_TYPE_INT => Some(ValueType::Int), + raw::ValueType_NIX_TYPE_LIST => Some(ValueType::List), + raw::ValueType_NIX_TYPE_NULL => Some(ValueType::Null), + raw::ValueType_NIX_TYPE_PATH => Some(ValueType::Path), + raw::ValueType_NIX_TYPE_STRING => Some(ValueType::String), + + raw::ValueType_NIX_TYPE_THUNK => None, + // This would happen if a new type of value is added in Nix. - _ => ValueTypeOrThunk::ValueType(ValueType::Unknown), + _ => Some(ValueType::Unknown), } } }