From 028057c85dc5cefb25c6c555a70d5a34744b9281 Mon Sep 17 00:00:00 2001 From: Yaroslav Bolyukin Date: Tue, 01 Dec 2020 10:01:52 +0000 Subject: [PATCH] refactor!: remove Lazy from Val Now lazy is used only in objects/arrays BREAKING CHANGE: value_type() is never fails --- --- a/bindings/jsonnet/src/val_extract.rs +++ b/bindings/jsonnet/src/val_extract.rs @@ -9,7 +9,7 @@ #[no_mangle] pub extern "C" fn jsonnet_json_extract_string(_vm: &EvaluationState, v: &Val) -> *mut c_char { - match v.unwrap_if_lazy().unwrap() { + match v { Val::Str(s) => CString::new(&*s as &str).unwrap().into_raw(), _ => std::ptr::null_mut(), } @@ -20,9 +20,9 @@ v: &Val, out: &mut c_double, ) -> c_int { - match v.unwrap_if_lazy().unwrap() { + match v { Val::Num(n) => { - *out = n; + *out = *n; 1 } _ => 0, @@ -30,7 +30,7 @@ } #[no_mangle] pub extern "C" fn jsonnet_json_extract_bool(_vm: &EvaluationState, v: &Val) -> c_int { - match v.unwrap_if_lazy().unwrap() { + match v { Val::Bool(false) => 0, Val::Bool(true) => 1, _ => 2, @@ -38,7 +38,7 @@ } #[no_mangle] pub extern "C" fn jsonnet_json_extract_null(_vm: &EvaluationState, v: &Val) -> c_int { - match v.unwrap_if_lazy().unwrap() { + match v { Val::Null => 1, _ => 0, } --- a/crates/jrsonnet-evaluator/src/builtin/format.rs +++ b/crates/jrsonnet-evaluator/src/builtin/format.rs @@ -1,7 +1,8 @@ //! faster std.format impl #![allow(clippy::too_many_arguments)] -use crate::{error::Error::*, throw, LocError, ObjValue, Result, Val, ValType}; +use crate::{error::Error::*, throw, LocError, ObjValue, Result, Val}; +use jrsonnet_types::ValType; use thiserror::Error; #[derive(Debug, Clone, Error)] @@ -573,7 +574,7 @@ ); } } - ConvTypeV::Char => match value.clone().unwrap_if_lazy()? { + ConvTypeV::Char => match value.clone() { Val::Num(n) => tmp_out.push( std::char::from_u32(n as u32) .ok_or_else(|| InvalidUnicodeCodepointGot(n as u32))?, @@ -590,7 +591,7 @@ throw!(TypeMismatch( "%c requires number/string", vec![ValType::Num, ValType::Str], - value.value_type()?, + value.value_type(), )); } }, --- a/crates/jrsonnet-evaluator/src/builtin/manifest.rs +++ b/crates/jrsonnet-evaluator/src/builtin/manifest.rs @@ -33,9 +33,9 @@ ) -> Result<()> { use std::fmt::Write; let mtype = options.mtype; - match val.unwrap_if_lazy()? { + match val { Val::Bool(v) => { - if v { + if *v { buf.push_str("true"); } else { buf.push_str("false"); @@ -63,7 +63,7 @@ } } buf.push_str(cur_padding); - manifest_json_ex_buf(item, buf, cur_padding, options)?; + manifest_json_ex_buf(&item?, buf, cur_padding, options)?; } cur_padding.truncate(old_len); @@ -118,7 +118,6 @@ buf.push('}'); } Val::Func(_) => throw!(RuntimeError("tried to manifest function".into())), - Val::Lazy(_) => unreachable!(), }; Ok(()) } --- a/crates/jrsonnet-evaluator/src/builtin/sort.rs +++ b/crates/jrsonnet-evaluator/src/builtin/sort.rs @@ -46,7 +46,6 @@ let mut sort_type = SortKeyType::Unknown; for i in values.iter_mut() { let i = key_getter(i); - i.inplace_unwrap()?; match (i, sort_type) { (Val::Str(_), SortKeyType::Unknown) => sort_type = SortKeyType::String, (Val::Num(_), SortKeyType::Unknown) => sort_type = SortKeyType::Number, --- a/crates/jrsonnet-evaluator/src/evaluate.rs +++ b/crates/jrsonnet-evaluator/src/evaluate.rs @@ -1,7 +1,6 @@ use crate::{ context_creator, error::Error::*, future_wrapper, lazy_val, push, throw, with_state, Context, ContextCreator, FuncDesc, FuncVal, LazyBinding, LazyVal, ObjMember, ObjValue, Result, Val, - ValType, }; use closure::closure; use jrsonnet_parser::{ @@ -9,6 +8,7 @@ ForSpecData, IfSpecData, LiteralType, LocExpr, Member, ObjBody, ParamsDesc, UnaryOpType, Visibility, }; +use jrsonnet_types::ValType; use rustc_hash::FxHashMap; use std::{collections::HashMap, rc::Rc}; @@ -61,8 +61,7 @@ Ok(match field_name { jrsonnet_parser::FieldName::Fixed(n) => Some(n.clone()), jrsonnet_parser::FieldName::Dyn(expr) => { - let lazy = evaluate(context, expr)?; - let value = lazy.unwrap_if_lazy()?; + let value = evaluate(context, expr)?; if matches!(value, Val::Null) { None } else { @@ -74,11 +73,10 @@ pub fn evaluate_unary_op(op: UnaryOpType, b: &Val) -> Result { Ok(match (op, b) { - (o, Val::Lazy(l)) => evaluate_unary_op(o, &l.evaluate()?)?, (UnaryOpType::Not, Val::Bool(v)) => Val::Bool(!v), (UnaryOpType::Minus, Val::Num(n)) => Val::Num(-*n), (UnaryOpType::BitNot, Val::Num(n)) => Val::Num(!(*n as i32) as f64), - (op, o) => throw!(UnaryOperatorDoesNotOperateOnType(op, o.value_type()?)), + (op, o) => throw!(UnaryOperatorDoesNotOperateOnType(op, o.value_type())), }) } @@ -94,12 +92,17 @@ (o, Val::Str(s)) => Val::Str(format!("{}{}", o.clone().to_string()?, s).into()), (Val::Obj(v1), Val::Obj(v2)) => Val::Obj(v2.with_super(v1.clone())), - (Val::Arr(a), Val::Arr(b)) => Val::Arr(Rc::new([&a[..], &b[..]].concat())), + (Val::Arr(a), Val::Arr(b)) => { + let mut out = Vec::with_capacity(a.len() + b.len()); + out.extend(a.iter_lazy()); + out.extend(b.iter_lazy()); + Val::Arr(out.into()) + } (Val::Num(v1), Val::Num(v2)) => Val::new_checked_num(v1 + v2)?, _ => throw!(BinaryOperatorDoesNotOperateOnValues( BinaryOpType::Add, - a.value_type()?, - b.value_type()?, + a.value_type(), + b.value_type(), )), }) } @@ -110,15 +113,11 @@ op: BinaryOpType, b: &LocExpr, ) -> Result { - Ok( - match (evaluate(context.clone(), a)?.unwrap_if_lazy()?, op, b) { - (Val::Bool(true), BinaryOpType::Or, _o) => Val::Bool(true), - (Val::Bool(false), BinaryOpType::And, _o) => Val::Bool(false), - (a, op, eb) => { - evaluate_binary_op_normal(&a, op, &evaluate(context, eb)?.unwrap_if_lazy()?)? - } - }, - ) + Ok(match (evaluate(context.clone(), a)?, op, b) { + (Val::Bool(true), BinaryOpType::Or, _o) => Val::Bool(true), + (Val::Bool(false), BinaryOpType::And, _o) => Val::Bool(false), + (a, op, eb) => evaluate_binary_op_normal(&a, op, &evaluate(context, eb)?)?, + }) } pub fn evaluate_binary_op_normal(a: &Val, op: BinaryOpType, b: &Val) -> Result { @@ -177,8 +176,8 @@ _ => throw!(BinaryOperatorDoesNotOperateOnValues( op, - a.value_type()?, - b.value_type()?, + a.value_type(), + b.value_type(), )), }) } @@ -200,23 +199,20 @@ None } } - Some(CompSpec::ForSpec(ForSpecData(var, expr))) => { - match evaluate(context.clone(), expr)?.unwrap_if_lazy()? { - Val::Arr(list) => { - let mut out = Vec::new(); - for item in list.iter() { - let item = item.unwrap_if_lazy()?; - out.push(evaluate_comp( - context.clone().with_var(var.clone(), item.clone()), - value, - &specs[1..], - )?); - } - Some(out.into_iter().flatten().flatten().collect()) + Some(CompSpec::ForSpec(ForSpecData(var, expr))) => match evaluate(context.clone(), expr)? { + Val::Arr(list) => { + let mut out = Vec::new(); + for item in list.iter() { + out.push(evaluate_comp( + context.clone().with_var(var.clone(), item?.clone()), + value, + &specs[1..], + )?); } - _ => throw!(InComprehensionCanOnlyIterateOverArray), + Some(out.into_iter().flatten().flatten().collect()) } - } + _ => throw!(InComprehensionCanOnlyIterateOverArray), + }, }) } @@ -375,7 +371,7 @@ }, ); } - v => throw!(FieldMustBeStringGot(v.value_type()?)), + v => throw!(FieldMustBeStringGot(v.value_type())), } } @@ -391,8 +387,7 @@ loc: &Option, tailstrict: bool, ) -> Result { - let lazy = evaluate(context.clone(), value)?; - let value = lazy.unwrap_if_lazy()?; + let value = evaluate(context.clone(), value)?; Ok(match value { Val::Func(f) => { let body = || f.evaluate(context, loc, args, tailstrict); @@ -402,7 +397,7 @@ push(loc, || format!("function <{}> call", f.name()), body)? } } - v => throw!(OnlyFunctionsCanBeCalledGot(v.value_type()?)), + v => throw!(OnlyFunctionsCanBeCalledGot(v.value_type())), }) } @@ -436,7 +431,7 @@ Var(name) => push( loc, || format!("variable <{}>", name), - || Ok(Val::Lazy(context.binding(name.clone())?).unwrap_if_lazy()?), + || Ok(context.binding(name.clone())?.evaluate()?), )?, Index(LocExpr(v, _), index) if matches!(&**v, Expr::Literal(LiteralType::Super)) => { let name = evaluate(context.clone(), index)?.try_cast_str("object index")?; @@ -444,14 +439,11 @@ .super_obj() .clone() .expect("no super found") - .get_raw(name, &context.this().clone().expect("no this found"))? + .get_raw(name, Some(&context.this().clone().expect("no this found")))? .expect("value not found") } Index(value, index) => { - match ( - evaluate(context.clone(), value)?.unwrap_if_lazy()?, - evaluate(context, index)?, - ) { + match (evaluate(context.clone(), value)?, evaluate(context, index)?) { (Val::Obj(v), Val::Str(s)) => { let sn = s.clone(); push( @@ -459,7 +451,7 @@ || format!("field <{}> access", sn), || { if let Some(v) = v.get(s.clone())? { - Ok(v.unwrap_if_lazy()?) + Ok(v) } else if v.get("__intrinsic_namespace__".into())?.is_some() { Ok(Val::Func(Rc::new(FuncVal::Intrinsic(s)))) } else { @@ -471,23 +463,22 @@ (Val::Obj(_), n) => throw!(ValueIndexMustBeTypeGot( ValType::Obj, ValType::Str, - n.value_type()?, + n.value_type(), )), (Val::Arr(v), Val::Num(n)) => { if n.fract() > f64::EPSILON { throw!(FractionalIndex) } - v.get(n as usize) + v.get(n as usize)? .ok_or_else(|| ArrayBoundsError(n as usize, v.len()))? .clone() - .unwrap_if_lazy()? } (Val::Arr(_), Val::Str(n)) => throw!(AttemptedIndexAnArrayWithString(n)), (Val::Arr(_), n) => throw!(ValueIndexMustBeTypeGot( ValType::Arr, ValType::Num, - n.value_type()?, + n.value_type(), )), (Val::Str(s), Val::Num(n)) => Val::Str( @@ -500,10 +491,10 @@ (Val::Str(_), n) => throw!(ValueIndexMustBeTypeGot( ValType::Str, ValType::Num, - n.value_type()?, + n.value_type(), )), - (v, _) => throw!(CantIndexInto(v.value_type()?)), + (v, _) => throw!(CantIndexInto(v.value_type())), } } LocalExpr(bindings, returned) => { @@ -529,17 +520,19 @@ Arr(items) => { let mut out = Vec::with_capacity(items.len()); for item in items { - out.push(Val::Lazy(lazy_val!( + out.push(LazyVal::new(Box::new( closure!(clone context, clone item, || { evaluate(context.clone(), &item) - }) + }), ))); } - Val::Arr(Rc::new(out)) + Val::Arr(out.into()) } ArrComp(expr, comp_specs) => Val::Arr( // First comp_spec should be for_spec, so no "None" possible here - Rc::new(evaluate_comp(context, &|ctx| evaluate(ctx, expr), comp_specs)?.unwrap()), + evaluate_comp(context, &|ctx| evaluate(ctx, expr), comp_specs)? + .unwrap() + .into(), ), Obj(body) => Val::Obj(evaluate_object(context, body)?), ObjExtend(s, t) => evaluate_add_op( --- a/crates/jrsonnet-evaluator/src/integrations/serde.rs +++ b/crates/jrsonnet-evaluator/src/integrations/serde.rs @@ -22,11 +22,10 @@ } else { Number::from_f64(*n).expect("to json number") }), - Val::Lazy(v) => (&v.evaluate()?).try_into()?, Val::Arr(a) => { let mut out = Vec::with_capacity(a.len()); for item in a.iter() { - out.push(item.try_into()?); + out.push((&item?).try_into()?); } Self::Array(out) } @@ -55,9 +54,9 @@ Value::Array(a) => { let mut out = Vec::with_capacity(a.len()); for v in a { - out.push(v.into()); + out.push(LazyVal::new_resolved(v.into())); } - Self::Arr(Rc::new(out)) + Self::Arr(out.into()) } Value::Object(o) => { let mut entries = HashMap::with_capacity(o.len()); --- a/crates/jrsonnet-evaluator/src/obj.rs +++ b/crates/jrsonnet-evaluator/src/obj.rs @@ -102,9 +102,10 @@ visible_fields } pub fn get(&self, key: Rc) -> Result> { - Ok(self.get_raw(key, self)?) + Ok(self.get_raw(key, None)?) } - pub(crate) fn get_raw(&self, key: Rc, real_this: &Self) -> Result> { + pub(crate) fn get_raw(&self, key: Rc, real_this: Option<&Self>) -> Result> { + let real_this = real_this.unwrap_or(self); let cache_key = (key.clone(), Rc::as_ptr(&real_this.0) as usize); if let Some(v) = self.0.value_cache.borrow().get(&cache_key) { @@ -115,7 +116,7 @@ (Some(k), Some(s)) => { let our = self.evaluate_this(k, real_this)?; if k.add { - s.get_raw(key, real_this)? + s.get_raw(key, Some(real_this))? .map_or(Ok(Some(our.clone())), |v| { Ok(Some(evaluate_add_op(&v, &our)?)) }) @@ -123,7 +124,7 @@ Ok(Some(our)) } } - (None, Some(s)) => s.get_raw(key, real_this), + (None, Some(s)) => s.get_raw(key, Some(real_this)), (None, None) => Ok(None), }?; self.0 --- a/crates/jrsonnet-evaluator/src/val.rs +++ b/crates/jrsonnet-evaluator/src/val.rs @@ -10,12 +10,8 @@ throw, with_state, Context, ObjValue, Result, }; use jrsonnet_parser::{el, Arg, ArgsDesc, Expr, ExprLocation, LiteralType, LocExpr, ParamsDesc}; -use std::{ - cell::RefCell, - collections::HashMap, - fmt::{Debug, Display}, - rc::Rc, -}; +use jrsonnet_types::ValType; +use std::{cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc}; enum LazyValInternals { Computed(Val), @@ -166,43 +162,108 @@ } } -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum ValType { - Bool, - Null, - Str, - Num, - Arr, - Obj, - Func, +#[derive(Clone)] +pub enum ManifestFormat { + YamlStream(Box), + Yaml(usize), + Json(usize), + ToString, + String, +} + +#[derive(Debug, Clone)] +pub enum ArrValue { + Lazy(Rc>), + Eager(Rc>), } -impl ValType { - pub const fn name(&self) -> &'static str { - use ValType::*; +impl ArrValue { + pub fn len(&self) -> usize { match self { - Bool => "boolean", - Null => "null", - Str => "string", - Num => "number", - Arr => "array", - Obj => "object", - Func => "function", + ArrValue::Lazy(l) => l.len(), + ArrValue::Eager(e) => e.len(), } } + + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + pub fn get(&self, index: usize) -> Result> { + match self { + ArrValue::Lazy(vec) => { + if let Some(v) = vec.get(index) { + Ok(Some(v.evaluate()?)) + } else { + Ok(None) + } + } + ArrValue::Eager(vec) => Ok(vec.get(index).cloned()), + } + } + + pub fn get_lazy(&self, index: usize) -> Option { + match self { + ArrValue::Lazy(vec) => vec.get(index).cloned(), + ArrValue::Eager(vec) => vec + .get(index) + .cloned() + .map(|val| LazyVal::new_resolved(val)), + } + } + + pub fn evaluated(&self) -> Result>> { + Ok(match self { + ArrValue::Lazy(vec) => { + let mut out = Vec::with_capacity(vec.len()); + for item in vec.iter() { + out.push(item.evaluate()?); + } + Rc::new(out) + } + ArrValue::Eager(vec) => vec.clone(), + }) + } + + pub fn iter(&self) -> impl DoubleEndedIterator> + '_ { + (0..self.len()).map(move |idx| match self { + ArrValue::Lazy(l) => l[idx].evaluate(), + ArrValue::Eager(e) => Ok(e[idx].clone()), + }) + } + + pub fn iter_lazy(&self) -> impl DoubleEndedIterator + '_ { + (0..self.len()).map(move |idx| match self { + ArrValue::Lazy(l) => l[idx].clone(), + ArrValue::Eager(e) => LazyVal::new_resolved(e[idx].clone()), + }) + } + + pub fn reversed(self) -> Self { + match self { + ArrValue::Lazy(vec) => { + let mut out = (&vec as &Vec<_>).clone(); + out.reverse(); + Self::Lazy(Rc::new(out)) + } + ArrValue::Eager(vec) => { + let mut out = (&vec as &Vec<_>).clone(); + out.reverse(); + Self::Eager(Rc::new(out)) + } + } + } } -impl Display for ValType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name()) + +impl From> for ArrValue { + fn from(v: Vec) -> Self { + Self::Lazy(Rc::new(v)) } } -#[derive(Clone)] -pub enum ManifestFormat { - YamlStream(Box), - Yaml(usize), - Json(usize), - ToString, - String, +impl From> for ArrValue { + fn from(v: Vec) -> Self { + Self::Eager(Rc::new(v)) + } } #[derive(Debug, Clone)] @@ -211,8 +272,7 @@ Null, Str(Rc), Num(f64), - Lazy(LazyVal), - Arr(Rc>), + Arr(ArrValue), Obj(ObjValue), Func(Rc), } @@ -237,40 +297,33 @@ } pub fn assert_type(&self, context: &'static str, val_type: ValType) -> Result<()> { - let this_type = self.value_type()?; + let this_type = self.value_type(); if this_type != val_type { throw!(TypeMismatch(context, vec![val_type], this_type)) } else { Ok(()) } } + pub fn unwrap_num(self) -> Result { + Ok(matches_unwrap!(self, Self::Num(v), v)) + } + pub fn unwrap_func(self) -> Result> { + Ok(matches_unwrap!(self, Self::Func(v), v)) + } pub fn try_cast_bool(self, context: &'static str) -> Result { self.assert_type(context, ValType::Bool)?; - Ok(matches_unwrap!(self.unwrap_if_lazy()?, Self::Bool(v), v)) + Ok(matches_unwrap!(self, Self::Bool(v), v)) } pub fn try_cast_str(self, context: &'static str) -> Result> { self.assert_type(context, ValType::Str)?; - Ok(matches_unwrap!(self.unwrap_if_lazy()?, Self::Str(v), v)) + Ok(matches_unwrap!(self, Self::Str(v), v)) } pub fn try_cast_num(self, context: &'static str) -> Result { self.assert_type(context, ValType::Num)?; - Ok(matches_unwrap!(self.unwrap_if_lazy()?, Self::Num(v), v)) - } - pub fn inplace_unwrap(&mut self) -> Result<()> { - while let Self::Lazy(lazy) = self { - *self = lazy.evaluate()?; - } - Ok(()) + self.unwrap_num() } - pub fn unwrap_if_lazy(&self) -> Result { - Ok(if let Self::Lazy(v) = self { - v.evaluate()?.unwrap_if_lazy()? - } else { - self.clone() - }) - } - pub fn value_type(&self) -> Result { - Ok(match self { + pub fn value_type(&self) -> ValType { + match self { Self::Str(..) => ValType::Str, Self::Num(..) => ValType::Num, Self::Arr(..) => ValType::Arr, @@ -278,16 +331,15 @@ Self::Bool(_) => ValType::Bool, Self::Null => ValType::Null, Self::Func(..) => ValType::Func, - Self::Lazy(_) => self.clone().unwrap_if_lazy()?.value_type()?, - }) + } } pub fn to_string(&self) -> Result> { - Ok(match self.unwrap_if_lazy()? { + Ok(match self { Self::Bool(true) => "true".into(), Self::Bool(false) => "false".into(), Self::Null => "null".into(), - Self::Str(s) => s, + Self::Str(s) => s.clone(), v => manifest_json_ex( &v, &ManifestJsonOptions { @@ -325,7 +377,7 @@ }; let mut out = Vec::with_capacity(arr.len()); for i in arr.iter() { - out.push(i.manifest(ty)?); + out.push(i?.manifest(ty)?); } Ok(out) } @@ -348,7 +400,7 @@ if !arr.is_empty() { for v in arr.iter() { out.push_str("---\n"); - out.push_str(&v.manifest(format)?); + out.push_str(&v?.manifest(format)?); out.push('\n'); } out.push_str("..."); @@ -456,7 +508,7 @@ /// Native implementation of `std.primitiveEquals` pub fn primitive_equals(val_a: &Val, val_b: &Val) -> Result { - Ok(match (val_a.unwrap_if_lazy()?, val_b.unwrap_if_lazy()?) { + Ok(match (val_a, val_b) { (Val::Bool(a), Val::Bool(b)) => a == b, (Val::Null, Val::Null) => true, (Val::Str(a), Val::Str(b)) => a == b, @@ -476,10 +528,7 @@ /// Native implementation of `std.equals` pub fn equals(val_a: &Val, val_b: &Val) -> Result { - let val_a = val_a.unwrap_if_lazy()?; - let val_b = val_b.unwrap_if_lazy()?; - - if val_a.value_type()? != val_b.value_type()? { + if val_a.value_type() != val_b.value_type() { return Ok(false); } match (val_a, val_b) { @@ -489,7 +538,7 @@ return Ok(false); } for (a, b) in a.iter().zip(b.iter()) { - if !equals(&a.unwrap_if_lazy()?, &b.unwrap_if_lazy()?)? { + if !equals(&a?, &b?)? { return Ok(false); } } --- a/crates/jrsonnet-parser/src/expr.rs +++ b/crates/jrsonnet-parser/src/expr.rs @@ -97,6 +97,7 @@ Mul, Div, + /// Implemented as intrinsic, put here for completeness Mod, Add, -- gitstuff