From ec654c44d309eb26bb163be0d9a6b6bb60145d1c Mon Sep 17 00:00:00 2001 From: Yaroslav Bolyukin Date: Mon, 22 Feb 2021 12:29:41 +0000 Subject: [PATCH] refactor: get rid of LazyBinding variants --- --- a/crates/jrsonnet-evaluator/src/ctx.rs +++ b/crates/jrsonnet-evaluator/src/ctx.rs @@ -1,17 +1,24 @@ use crate::{ - error::Error::*, map::LayeredHashMap, rc_fn_helper, resolved_lazy_val, FutureWrapper, - LazyBinding, LazyVal, ObjValue, Result, Val, + error::Error::*, map::LayeredHashMap, resolved_lazy_val, FutureWrapper, LazyBinding, LazyVal, + ObjValue, Result, Val, }; use jrsonnet_interner::IStr; use rustc_hash::FxHashMap; use std::hash::BuildHasherDefault; -use std::{collections::HashMap, fmt::Debug, rc::Rc}; +use std::{fmt::Debug, rc::Rc}; -rc_fn_helper!( - ContextCreator, - context_creator, - dyn Fn(Option, Option) -> Result -); +#[derive(Clone)] +pub struct ContextCreator(pub Context, pub FutureWrapper>); +impl ContextCreator { + pub fn create(&self, this: Option, super_obj: Option) -> Result { + self.0.clone().extend_unbound( + self.1.clone().unwrap(), + self.0.dollar().clone().or_else(|| this.clone()), + this, + super_obj, + ) + } +} struct ContextInternals { dollar: Option, @@ -120,9 +127,14 @@ } } } + pub fn extend_bound(self, new_bindings: FxHashMap) -> Self { + let new_this = self.0.this.clone(); + let new_super_obj = self.0.super_obj.clone(); + self.extend(new_bindings, None, new_this, new_super_obj) + } pub fn extend_unbound( self, - new_bindings: HashMap, + new_bindings: FxHashMap, new_dollar: Option, new_this: Option, new_super_obj: Option, --- a/crates/jrsonnet-evaluator/src/dynamic.rs +++ b/crates/jrsonnet-evaluator/src/dynamic.rs @@ -12,7 +12,7 @@ } } impl FutureWrapper { - pub fn unwrap(self) -> T { + pub fn unwrap(&self) -> T { self.0.borrow().as_ref().cloned().unwrap() } } @@ -21,30 +21,4 @@ fn default() -> Self { Self::new() } -} - -#[macro_export] -macro_rules! rc_fn_helper { - ($name: ident, $macro_name: ident, $fn: ty) => { - #[derive(Clone)] - #[doc = "Function wrapper"] - pub struct $name(pub std::rc::Rc<$fn>); - impl std::fmt::Debug for $name { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct(std::stringify!($name)).finish() - } - } - impl std::cmp::PartialEq for $name { - fn eq(&self, other: &$name) -> bool { - std::ptr::eq(&self.0, &other.0) - } - } - #[doc = "Macro to ease wrapper creation"] - #[macro_export] - macro_rules! $macro_name { - ($val: expr) => { - $crate::$name(std::rc::Rc::new($val)) - }; - } - }; } --- a/crates/jrsonnet-evaluator/src/evaluate.rs +++ b/crates/jrsonnet-evaluator/src/evaluate.rs @@ -1,6 +1,6 @@ use crate::{ - context_creator, error::Error::*, lazy_val, push, throw, with_state, Context, ContextCreator, - FuncDesc, FuncVal, FutureWrapper, LazyBinding, LazyVal, ObjMember, ObjValue, Result, Val, + error::Error::*, lazy_val, push, throw, with_state, Context, ContextCreator, FuncDesc, FuncVal, + FutureWrapper, LazyBinding, LazyVal, ObjMember, ObjValue, Result, Val, }; use closure::closure; use jrsonnet_interner::IStr; @@ -10,8 +10,30 @@ Visibility, }; use jrsonnet_types::ValType; -use rustc_hash::FxHashMap; -use std::{collections::HashMap, rc::Rc}; +use rustc_hash::{FxHashMap, FxHasher}; +use std::{collections::HashMap, hash::BuildHasherDefault, rc::Rc}; + +pub fn evaluate_binding_in_future( + b: &BindSpec, + context_creator: FutureWrapper, +) -> LazyVal { + let b = b.clone(); + if let Some(params) = &b.params { + let params = params.clone(); + LazyVal::new(Box::new(move || { + Ok(evaluate_method( + context_creator.unwrap(), + b.name.clone(), + params.clone(), + b.value.clone(), + )) + })) + } else { + LazyVal::new(Box::new(move || { + evaluate_named(context_creator.unwrap(), &b.value, b.name.clone()) + })) + } +} pub fn evaluate_binding(b: &BindSpec, context_creator: ContextCreator) -> (IStr, LazyBinding) { let b = b.clone(); @@ -22,7 +44,7 @@ LazyBinding::Bindable(Rc::new(move |this, super_obj| { Ok(lazy_val!( closure!(clone b, clone params, clone context_creator, || Ok(evaluate_method( - context_creator.0(this.clone(), super_obj.clone())?, + context_creator.create(this.clone(), super_obj.clone())?, b.name.clone(), params.clone(), b.value.clone(), @@ -35,11 +57,11 @@ b.name.clone(), LazyBinding::Bindable(Rc::new(move |this, super_obj| { Ok(lazy_val!(closure!(clone context_creator, clone b, || - evaluate_named( - context_creator.0(this.clone(), super_obj.clone())?, - &b.value, - b.name.clone() - ) + evaluate_named( + context_creator.create(this.clone(), super_obj.clone())?, + &b.value, + b.name.clone() + ) ))) })), ) @@ -217,18 +239,10 @@ pub fn evaluate_member_list_object(context: Context, members: &[Member]) -> Result { let new_bindings = FutureWrapper::new(); let future_this = FutureWrapper::new(); - let context_creator = context_creator!( - closure!(clone context, clone new_bindings, |this: Option, super_obj: Option| { - context.clone().extend_unbound( - new_bindings.clone().unwrap(), - context.dollar().clone().or_else(||this.clone()), - Some(this.unwrap()), - super_obj - ) - }) - ); + let context_creator = ContextCreator(context.clone(), new_bindings.clone()); { - let mut bindings: HashMap = HashMap::new(); + let mut bindings: FxHashMap = + FxHashMap::with_capacity_and_hasher(members.len(), BuildHasherDefault::default()); for (n, b) in members .iter() .filter_map(|m| match m { @@ -265,7 +279,7 @@ invoke: LazyBinding::Bindable(Rc::new( closure!(clone name, clone value, clone context_creator, |this, super_obj| { Ok(LazyVal::new_resolved(evaluate( - context_creator.0(this, super_obj)?, + context_creator.create(this, super_obj)?, &value, )?)) }), @@ -294,7 +308,7 @@ closure!(clone value, clone context_creator, clone params, clone name, |this, super_obj| { // TODO: Assert Ok(LazyVal::new_resolved(evaluate_method( - context_creator.0(this, super_obj)?, + context_creator.create(this, super_obj)?, name.clone(), params.clone(), value.clone(), @@ -324,17 +338,8 @@ context.clone(), &|ctx| { let new_bindings = FutureWrapper::new(); - let context_creator = context_creator!( - closure!(clone context, clone new_bindings, |this: Option, super_obj: Option| { - context.clone().extend_unbound( - new_bindings.clone().unwrap(), - context.dollar().clone().or_else(||this.clone()), - None, - super_obj - ) - }) - ); - let mut bindings: HashMap = HashMap::new(); + let context_creator = ContextCreator(context.clone(), new_bindings.clone()); + let mut bindings: FxHashMap = FxHashMap::with_capacity_and_hasher(obj.pre_locals.len() + obj.post_locals.len(), BuildHasherDefault::default()); for (n, b) in obj .pre_locals .iter() @@ -497,22 +502,19 @@ } } LocalExpr(bindings, returned) => { - let mut new_bindings: HashMap = HashMap::new(); + let mut new_bindings: FxHashMap = HashMap::with_capacity_and_hasher( + bindings.len(), + BuildHasherDefault::::default(), + ); let future_context = Context::new_future(); - - let context_creator = context_creator!( - closure!(clone future_context, |_, _| Ok(future_context.clone().unwrap())) - ); - - for (k, v) in bindings - .iter() - .map(|b| evaluate_binding(b, context_creator.clone())) - { - new_bindings.insert(k, v); + for b in bindings { + new_bindings.insert( + b.name.clone(), + evaluate_binding_in_future(b, future_context.clone()), + ); } - let context = context - .extend_unbound(new_bindings, None, None, None)? + .extend_bound(new_bindings) .into_future(future_context); evaluate(context, &returned.clone())? } --- a/crates/jrsonnet-evaluator/src/lib.rs +++ b/crates/jrsonnet-evaluator/src/lib.rs @@ -27,10 +27,12 @@ use jrsonnet_parser::*; use native::NativeCallback; pub use obj::*; +use rustc_hash::FxHashMap; use std::{ cell::{Ref, RefCell, RefMut}, collections::HashMap, fmt::Debug, + hash::BuildHasherDefault, path::PathBuf, rc::Rc, }; @@ -230,7 +232,7 @@ } value.parsed.clone() }; - let value = evaluate(self.create_default_context()?, &expr)?; + let value = evaluate(self.create_default_context(), &expr)?; { self.data_mut() .files @@ -260,16 +262,14 @@ } /// Creates context with all passed global variables - pub fn create_default_context(&self) -> Result { + pub fn create_default_context(&self) -> Context { let globals = &self.settings().globals; - let mut new_bindings: HashMap = HashMap::new(); + let mut new_bindings: FxHashMap = + FxHashMap::with_capacity_and_hasher(globals.len(), BuildHasherDefault::default()); for (name, value) in globals.iter() { - new_bindings.insert( - name.clone(), - LazyBinding::Bound(resolved_lazy_val!(value.clone())), - ); + new_bindings.insert(name.clone(), resolved_lazy_val!(value.clone())); } - Context::new().extend_unbound(new_bindings, None, None, None) + Context::new().extend_bound(new_bindings) } /// Executes code creating a new stack frame @@ -345,7 +345,7 @@ || "during TLA call".to_owned(), || { func.evaluate_map( - self.create_default_context()?, + self.create_default_context(), &self.settings().tla_vars, true, ) @@ -396,7 +396,7 @@ } /// Evaluates the parsed expression pub fn evaluate_expr_raw(&self, code: LocExpr) -> Result { - self.run_in_state(|| evaluate(self.create_default_context()?, &code)) + self.run_in_state(|| evaluate(self.create_default_context(), &code)) } } @@ -950,6 +950,23 @@ Ok(()) } + #[test] + fn comp_self() -> crate::error::Result<()> { + assert_eval!( + r#" + std.objectFields({ + a:{ + [name]: name for name in std.objectFields(self) + }, + b: 2, + c: 3, + }.a) == ['a', 'b', 'c'] + "# + ); + + Ok(()) + } + struct TestImportResolver(IStr); impl crate::import::ImportResolver for TestImportResolver { fn resolve_file(&self, _: &PathBuf, _: &PathBuf) -> crate::error::Result> { --- a/crates/jrsonnet-evaluator/src/val.rs +++ b/crates/jrsonnet-evaluator/src/val.rs @@ -535,7 +535,7 @@ pub fn to_yaml(&self, padding: usize) -> Result { with_state(|s| { let ctx = s - .create_default_context()? + .create_default_context() .with_var("__tmp__to_json__".into(), self.clone()); evaluate( ctx, -- gitstuff