From 3730d676869d927b31d0098cf7a4fe53d5e5bae1 Mon Sep 17 00:00:00 2001 From: Лач Date: Sun, 28 Jun 2020 12:29:12 +0000 Subject: [PATCH] refactor(evaluator): simplify state internals --- --- a/crates/jrsonnet-evaluator/src/lib.rs +++ b/crates/jrsonnet-evaluator/src/lib.rs @@ -24,7 +24,7 @@ pub use import::*; use jrsonnet_parser::*; pub use obj::*; -use std::{cell::RefCell, collections::HashMap, fmt::Debug, path::PathBuf, rc::Rc}; +use std::{cell::{Ref, RefCell, RefMut}, collections::HashMap, fmt::Debug, path::PathBuf, rc::Rc}; pub use val::*; type BindableFn = dyn Fn(Option, Option) -> Result; @@ -48,35 +48,40 @@ } } -pub struct EvaluationSettings { - pub max_stack_frames: usize, - pub max_stack_trace_size: usize, +struct EvaluationSettings { + max_stack_frames: usize, + max_stack_trace_size: usize, + ext_vars: HashMap, Val>, + globals: HashMap, Val>, + import_resolver: Box, } impl Default for EvaluationSettings { fn default() -> Self { EvaluationSettings { max_stack_frames: 200, max_stack_trace_size: 20, + globals: Default::default(), + ext_vars: Default::default(), + import_resolver: Box::new(DummyImportResolver), } } } -pub struct FileData(Rc, LocExpr, Option); #[derive(Default)] -pub struct EvaluationStateInternals { +struct EvaluationData { /// Used for stack-overflows and stacktraces - stack: RefCell>, + stack: Vec, /// Contains file source codes and evaluated results for imports and pretty /// printing stacktraces - files: RefCell, FileData>>, - str_files: RefCell, Rc>>, - globals: RefCell, Val>>, + files: HashMap, FileData>, + str_files: HashMap, Rc>, +} - /// Values to use with std.extVar - ext_vars: RefCell, Val>>, - - settings: EvaluationSettings, - import_resolver: Box, +pub struct FileData(Rc, LocExpr, Option); +#[derive(Default)] +pub struct EvaluationStateInternals { + data: RefCell, + settings: RefCell, } thread_local! { @@ -87,10 +92,10 @@ pub(crate) fn with_state(f: impl FnOnce(&EvaluationState) -> T) -> T { EVAL_STATE.with(|s| f(s.borrow().as_ref().unwrap())) } -pub(crate) fn create_error(err: Error) -> LocError { +pub fn create_error(err: Error) -> LocError { with_state(|s| s.error(err)) } -pub(crate) fn create_error_result(err: Error) -> Result { +pub fn create_error_result(err: Error) -> Result { Err(with_state(|s| s.error(err))) } pub(crate) fn push( @@ -109,13 +114,26 @@ #[derive(Default, Clone)] pub struct EvaluationState(Rc); impl EvaluationState { - pub fn new(settings: EvaluationSettings, import_resolver: Box) -> Self { - EvaluationState(Rc::new(EvaluationStateInternals { - settings, - import_resolver, - ..Default::default() - })) + fn data(&self) -> Ref { + self.0.data.borrow() + } + fn data_mut(&self) -> RefMut { + self.0.data.borrow_mut() + } + fn settings(&self) -> Ref { + self.0.settings.borrow() + } + fn settings_mut(&self) -> RefMut { + self.0.settings.borrow_mut() } + + pub fn set_import_resolver(&self, resolver: Box) { + self.settings_mut().import_resolver = resolver; + } + pub fn import_resolver(&self) -> Ref { + Ref::map(self.settings(), |s|&*s.import_resolver) + } + pub fn evaluate_file_to_json( &self, path: &PathBuf, @@ -127,7 +145,7 @@ name: Rc, code: Rc, ) -> std::result::Result<(), ParseError> { - self.0.files.borrow_mut().insert( + self.data_mut().files.insert( name.clone(), FileData( code.clone(), @@ -150,21 +168,20 @@ code: Rc, parsed: LocExpr, ) -> std::result::Result<(), ()> { - self.0 + self.data_mut() .files - .borrow_mut() .insert(name, FileData(code, parsed, None)); Ok(()) } pub fn get_source(&self, name: &PathBuf) -> Option> { - let ro_map = self.0.files.borrow(); + let ro_map = &self.data().files; ro_map.get(name).map(|value| value.0.clone()) } pub fn evaluate_file(&self, name: &PathBuf) -> Result { self.run_in_state(|| { let expr: LocExpr = { - let ro_map = self.0.files.borrow(); + let ro_map = &self.data().files; let value = ro_map .get(name) .unwrap_or_else(|| panic!("file not added: {:?}", name)); @@ -176,8 +193,8 @@ let value = evaluate(self.create_default_context()?, &expr)?; { self.0 + .data.borrow_mut() .files - .borrow_mut() .get_mut(name) .unwrap() .2 @@ -187,29 +204,28 @@ }) } pub(crate) fn import_file(&self, from: &PathBuf, path: &PathBuf) -> Result { - let file_path = self.0.import_resolver.resolve_file(from, path)?; + let file_path = self.settings().import_resolver.resolve_file(from, path)?; { - let files = self.0.files.borrow(); + let files = &self.data().files; if files.contains_key(&file_path) { return self.evaluate_file(&file_path); } } - let contents = self.0.import_resolver.load_file_contents(&file_path)?; + let contents = self.settings().import_resolver.load_file_contents(&file_path)?; self.add_file(file_path.clone(), contents).map_err(|e| { create_error(Error::ImportSyntaxError(e)) })?; self.evaluate_file(&file_path) } pub(crate) fn import_file_str(&self, from: &PathBuf, path: &PathBuf) -> Result> { - let path = self.0.import_resolver.resolve_file(from, path)?; - if !self.0.str_files.borrow().contains_key(&path) { - let file_str = self.0.import_resolver.load_file_contents(&path)?; - self.0 + let path = self.settings().import_resolver.resolve_file(from, path)?; + if !self.data().str_files.contains_key(&path) { + let file_str = self.settings().import_resolver.load_file_contents(&path)?; + self.data_mut() .str_files - .borrow_mut() .insert(path.clone(), file_str); } - Ok(self.0.str_files.borrow().get(&path).cloned().unwrap()) + Ok(self.data().str_files.get(&path).cloned().unwrap()) } pub fn parse_evaluate_raw(&self, code: &str) -> Result { @@ -229,10 +245,10 @@ } pub fn add_global(&self, name: Rc, value: Val) { - self.0.globals.borrow_mut().insert(name, value); + self.settings_mut().globals.insert(name, value); } pub fn add_ext_var(&self, name: Rc, value: Val) { - self.0.ext_vars.borrow_mut().insert(name, value); + self.settings_mut().ext_vars.insert(name, value); } pub fn with_stdlib(&self) -> &Self { @@ -278,7 +294,7 @@ } pub fn create_default_context(&self) -> Result { - let globals = self.0.globals.borrow(); + let globals = &self.settings().globals; let mut new_bindings: HashMap, LazyBinding> = HashMap::new(); for (name, value) in globals.iter() { new_bindings.insert( @@ -296,16 +312,18 @@ f: impl FnOnce() -> Result, ) -> Result { { - let mut stack = self.0.stack.borrow_mut(); - if stack.len() > self.0.settings.max_stack_frames { - drop(stack); + let mut data = self.data_mut(); + let stack = &mut data.stack; + if stack.len() > self.settings().max_stack_frames { + // Error creation uses data, so i drop guard here + drop(data); return Err(self.error(Error::StackOverflow)); } else { stack.push(StackTraceElement(e, comment)); } } let result = f(); - self.0.stack.borrow_mut().pop(); + self.data_mut().stack.pop(); result } pub fn print_stack_trace(&self) { @@ -315,12 +333,11 @@ } pub fn stack_trace(&self) -> StackTrace { StackTrace( - self.0 + self.data() .stack - .borrow() .iter() .rev() - .take(self.0.settings.max_stack_trace_size) + .take(self.settings().max_stack_trace_size) .cloned() .collect(), ) -- gitstuff