From d6b0f9978196f7e8e0f73082deff8be02c2d2528 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 10 Oct 2022 16:46:35 +0800 Subject: [PATCH] Refactor. --- CHANGELOG.md | 2 +- src/api/limits.rs | 63 ++++++++++++++++++++++-------------- src/bin/rhai-dbg.rs | 30 +++++++---------- src/bin/rhai-repl.rs | 27 +++++++++------- src/eval/data_check.rs | 58 ++++++++++++++------------------- src/eval/debugger.rs | 11 +++---- src/eval/global_state.rs | 7 ++-- src/func/hashing.rs | 9 ++---- src/module/mod.rs | 14 ++++---- src/module/resolvers/file.rs | 7 ++-- src/optimizer.rs | 43 ++++++++++++------------ src/packages/map_basic.rs | 9 +++--- src/packages/string_more.rs | 26 ++++++++------- src/serde/deserialize.rs | 7 ++-- src/serde/serialize.rs | 14 ++++---- 15 files changed, 161 insertions(+), 166 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1fe04c4..9c2848dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ Enhancements * `Scope` is now serializable and deserializable via `serde`. * `Scope` now contains a const generic parameter that allows specifying how many entries to be kept inline. * `parse_json` function is added to parse a JSON string into an object map. -* Methods return maximum limits (e.g. `max_string_len`) are now available even under `unchecked` in order to avoid unnecessary feature flags in third-party library code. +* Methods returning maximum limits (e.g. `Engine::max_string_len`) are now available even under `unchecked` in order to avoid unnecessary feature flags in third-party library code. Version 1.10.1 diff --git a/src/api/limits.rs b/src/api/limits.rs index 8f6f2be9..32daf13e 100644 --- a/src/api/limits.rs +++ b/src/api/limits.rs @@ -101,6 +101,27 @@ impl Default for Limits { } impl Engine { + /// Is there a data size limit set? + #[inline] + pub(crate) const fn has_data_size_limit(&self) -> bool { + self.limits.max_string_size.is_some() + || { + #[cfg(not(feature = "no_index"))] + { + self.limits.max_array_size.is_some() + } + #[cfg(feature = "no_index")] + false + } + || { + #[cfg(not(feature = "no_object"))] + { + self.limits.max_map_size.is_some() + } + #[cfg(feature = "no_object")] + false + } + } /// Set the maximum levels of function calls allowed for a script in order to avoid /// infinite recursion and stack overflows. /// @@ -137,10 +158,9 @@ impl Engine { #[inline] #[must_use] pub const fn max_operations(&self) -> u64 { - if let Some(n) = self.limits.max_operations { - n.get() - } else { - 0 + match self.limits.max_operations { + Some(n) => n.get(), + None => 0, } } /// Set the maximum number of imported [modules][crate::Module] allowed for a script. @@ -183,10 +203,9 @@ impl Engine { #[inline] #[must_use] pub const fn max_expr_depth(&self) -> usize { - if let Some(n) = self.limits.max_expr_depth { - n.get() - } else { - 0 + match self.limits.max_expr_depth { + Some(n) => n.get(), + None => 0, } } /// The depth limit for expressions in functions (0 for unlimited). @@ -196,10 +215,9 @@ impl Engine { #[must_use] pub const fn max_function_expr_depth(&self) -> usize { #[cfg(not(feature = "no_function"))] - return if let Some(n) = self.limits.max_function_expr_depth { - n.get() - } else { - 0 + return match self.limits.max_function_expr_depth { + Some(n) => n.get(), + None => 0, }; #[cfg(feature = "no_function")] return 0; @@ -216,10 +234,9 @@ impl Engine { #[inline] #[must_use] pub const fn max_string_size(&self) -> usize { - if let Some(n) = self.limits.max_string_size { - n.get() - } else { - 0 + match self.limits.max_string_size { + Some(n) => n.get(), + None => 0, } } /// Set the maximum length of [arrays][crate::Array] (0 for unlimited). @@ -238,10 +255,9 @@ impl Engine { #[must_use] pub const fn max_array_size(&self) -> usize { #[cfg(not(feature = "no_index"))] - return if let Some(n) = self.limits.max_array_size { - n.get() - } else { - 0 + return match self.limits.max_array_size { + Some(n) => n.get(), + None => 0, }; #[cfg(feature = "no_index")] return 0; @@ -262,10 +278,9 @@ impl Engine { #[must_use] pub const fn max_map_size(&self) -> usize { #[cfg(not(feature = "no_object"))] - return if let Some(n) = self.limits.max_map_size { - n.get() - } else { - 0 + return match self.limits.max_map_size { + Some(n) => n.get(), + None => 0, }; #[cfg(feature = "no_object")] return 0; diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index a83b680c..71d5152a 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -45,7 +45,6 @@ fn print_source(lines: &[String], pos: Position, offset: usize, window: (usize, if n == line { if let Some(pos) = pos.position() { let shift = offset + line_no_len + marker.len() + 2; - println!("{0:>1$}{2:>3$}", "│ ", shift, "\x1b[36m^\x1b[39m", pos + 10); } } @@ -310,10 +309,11 @@ fn debug_callback( ["node"] => { if pos.is_none() { println!("{:?}", node); - } else if let Some(source) = source { - println!("{:?} {} @ {:?}", node, source, pos); } else { - println!("{:?} @ {:?}", node, pos); + match source { + Some(source) => println!("{:?} {} @ {:?}", node, source, pos), + None => println!("{:?} @ {:?}", node, pos), + } } println!(); } @@ -339,20 +339,14 @@ fn debug_callback( ["over" | "o"] => break Ok(DebuggerCommand::StepOver), ["next" | "n"] => break Ok(DebuggerCommand::Next), ["scope"] => println!("{}", context.scope()), - ["print" | "p", "this"] => { - if let Some(value) = context.this_ptr() { - println!("=> {:?}", value); - } else { - println!("`this` pointer is unbound."); - } - } - ["print" | "p", var_name] => { - if let Some(value) = context.scope().get_value::(var_name) { - println!("=> {:?}", value); - } else { - eprintln!("Variable not found: {}", var_name); - } - } + ["print" | "p", "this"] => match context.this_ptr() { + Some(value) => println!("=> {:?}", value), + None => println!("`this` pointer is unbound."), + }, + ["print" | "p", var_name] => match context.scope().get_value::(var_name) { + Some(value) => println!("=> {:?}", value), + None => eprintln!("Variable not found: {}", var_name), + }, ["print" | "p"] => { println!("{}", context.scope().clone_visible()); if let Some(value) = context.this_ptr() { diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 00e01e8b..dcab9974 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -482,27 +482,30 @@ fn main() { continue; } "!!" => { - if let Some(line) = rl.history().last() { - replacement = Some(line.clone()); - replacement_index = history_offset + rl.history().len() - 1; - } else { - eprintln!("No lines history!"); + match rl.history().last() { + Some(line) => { + replacement = Some(line.clone()); + replacement_index = history_offset + rl.history().len() - 1; + } + None => eprintln!("No lines history!"), } continue; } _ if cmd.starts_with("!?") => { let text = cmd[2..].trim(); - if let Some((n, line)) = rl + let history = rl .history() .iter() .rev() .enumerate() - .find(|&(.., h)| h.contains(text)) - { - replacement = Some(line.clone()); - replacement_index = history_offset + (rl.history().len() - 1 - n); - } else { - eprintln!("History line not found: {}", text); + .find(|&(.., h)| h.contains(text)); + + match history { + Some((n, line)) => { + replacement = Some(line.clone()); + replacement_index = history_offset + (rl.history().len() - 1 - n); + } + None => eprintln!("History line not found: {}", text), } continue; } diff --git a/src/eval/data_check.rs b/src/eval/data_check.rs index 377a7757..76cf7a5c 100644 --- a/src/eval/data_check.rs +++ b/src/eval/data_check.rs @@ -4,7 +4,6 @@ use super::GlobalRuntimeState; use crate::types::dynamic::Union; use crate::{Dynamic, Engine, Position, RhaiResultOf, ERR}; -use std::num::NonZeroUsize; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -21,40 +20,40 @@ impl Engine { #[cfg(not(feature = "no_index"))] Union::Array(ref arr, ..) => { arr.iter() - .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 { + .fold((0, 0, 0), |(ax, mx, sx), value| match value.0 { Union::Array(..) => { let (a, m, s) = Self::calc_data_sizes(value, false); - (arrays + a + 1, maps + m, strings + s) + (ax + a + 1, mx + m, sx + s) } - Union::Blob(ref a, ..) => (arrays + 1 + a.len(), maps, strings), + Union::Blob(ref a, ..) => (ax + 1 + a.len(), mx, sx), #[cfg(not(feature = "no_object"))] Union::Map(..) => { let (a, m, s) = Self::calc_data_sizes(value, false); - (arrays + a + 1, maps + m, strings + s) + (ax + a + 1, mx + m, sx + s) } - Union::Str(ref s, ..) => (arrays + 1, maps, strings + s.len()), - _ => (arrays + 1, maps, strings), + Union::Str(ref s, ..) => (ax + 1, mx, sx + s.len()), + _ => (ax + 1, mx, sx), }) } #[cfg(not(feature = "no_index"))] - Union::Blob(ref arr, ..) => (arr.len(), 0, 0), + Union::Blob(ref blob, ..) => (blob.len(), 0, 0), #[cfg(not(feature = "no_object"))] Union::Map(ref map, ..) => { map.values() - .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 { + .fold((0, 0, 0), |(ax, mx, sx), value| match value.0 { #[cfg(not(feature = "no_index"))] Union::Array(..) => { let (a, m, s) = Self::calc_data_sizes(value, false); - (arrays + a, maps + m + 1, strings + s) + (ax + a, mx + m + 1, sx + s) } #[cfg(not(feature = "no_index"))] - Union::Blob(ref a, ..) => (arrays + a.len(), maps, strings), + Union::Blob(ref a, ..) => (ax + a.len(), mx, sx), Union::Map(..) => { let (a, m, s) = Self::calc_data_sizes(value, false); - (arrays + a, maps + m + 1, strings + s) + (ax + a, mx + m + 1, sx + s) } - Union::Str(ref s, ..) => (arrays, maps + 1, strings + s.len()), - _ => (arrays, maps + 1, strings), + Union::Str(ref s, ..) => (ax, mx + 1, sx + s.len()), + _ => (ax, mx + 1, sx), }) } Union::Str(ref s, ..) => (0, 0, s.len()), @@ -70,43 +69,34 @@ impl Engine { } } - /// Is there a data size limit set? - pub(crate) const fn has_data_size_limit(&self) -> bool { - self.max_string_size() > 0 || self.max_array_size() > 0 || self.max_map_size() > 0 - } - /// Raise an error if any data size exceeds limit. pub(crate) fn raise_err_if_over_data_size_limit( &self, - sizes: (usize, usize, usize), + (_arr, _map, s): (usize, usize, usize), pos: Position, ) -> RhaiResultOf<()> { - let (_arr, _map, s) = sizes; - - if s > self + if self .limits .max_string_size - .map_or(usize::MAX, NonZeroUsize::get) + .map_or(false, |max| s > max.get()) { return Err(ERR::ErrorDataTooLarge("Length of string".to_string(), pos).into()); } #[cfg(not(feature = "no_index"))] - if _arr - > self - .limits - .max_array_size - .map_or(usize::MAX, NonZeroUsize::get) + if self + .limits + .max_array_size + .map_or(false, |max| _arr > max.get()) { return Err(ERR::ErrorDataTooLarge("Size of array".to_string(), pos).into()); } #[cfg(not(feature = "no_object"))] - if _map - > self - .limits - .max_map_size - .map_or(usize::MAX, NonZeroUsize::get) + if self + .limits + .max_map_size + .map_or(false, |max| _map > max.get()) { return Err(ERR::ErrorDataTooLarge("Size of object map".to_string(), pos).into()); } diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index d6a33624..249b0319 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -496,13 +496,10 @@ impl Engine { let event = match event { Some(e) => e, - None => { - if let Some(bp) = global.debugger.is_break_point(&global.source, node) { - DebuggerEvent::BreakPoint(bp) - } else { - return Ok(None); - } - } + None => match global.debugger.is_break_point(&global.source, node) { + Some(bp) => DebuggerEvent::BreakPoint(bp), + None => return Ok(None), + }, }; self.run_debugger_raw(scope, global, lib, this_ptr, node, event, level) diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index 12e2ce5f..1144b851 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -107,10 +107,9 @@ impl GlobalRuntimeState<'_> { } else { crate::eval::DebuggerStatus::CONTINUE }, - if let Some((ref init, ..)) = engine.debugger { - init(engine) - } else { - Dynamic::UNIT + match engine.debugger { + Some((ref init, ..)) => init(engine), + None => Dynamic::UNIT, }, ), diff --git a/src/func/hashing.rs b/src/func/hashing.rs index de261de7..5f6a806c 100644 --- a/src/func/hashing.rs +++ b/src/func/hashing.rs @@ -77,14 +77,11 @@ impl BuildHasher for StraightHasherBuilder { #[inline(always)] #[must_use] pub fn get_hasher() -> ahash::AHasher { - if let Some([seed1, seed2, seed3, seed4]) = config::AHASH_SEED { - if seed1 | seed2 | seed3 | seed4 != 0 { + match config::AHASH_SEED { + Some([seed1, seed2, seed3, seed4]) if seed1 | seed2 | seed3 | seed4 != 0 => { ahash::RandomState::with_seeds(seed1, seed2, seed3, seed4).build_hasher() - } else { - ahash::AHasher::default() } - } else { - ahash::AHasher::default() + _ => ahash::AHasher::default(), } } diff --git a/src/module/mod.rs b/src/module/mod.rs index 9e5afca5..fe48d77f 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1742,10 +1742,9 @@ impl Module { } if let Some(ref variables) = other.variables { - if let Some(ref mut m) = self.variables { - m.extend(variables.iter().map(|(k, v)| (k.clone(), v.clone()))); - } else { - self.variables = other.variables.clone(); + match self.variables { + Some(ref mut m) => m.extend(variables.iter().map(|(k, v)| (k.clone(), v.clone()))), + None => self.variables = other.variables.clone(), } } @@ -1767,10 +1766,9 @@ impl Module { self.dynamic_functions_filter += &other.dynamic_functions_filter; if let Some(ref type_iterators) = other.type_iterators { - if let Some(ref mut t) = self.type_iterators { - t.extend(type_iterators.iter().map(|(&k, v)| (k, v.clone()))); - } else { - self.type_iterators = other.type_iterators.clone(); + match self.type_iterators { + Some(ref mut t) => t.extend(type_iterators.iter().map(|(&k, v)| (k, v.clone()))), + None => self.type_iterators = other.type_iterators.clone(), } } self.all_functions = None; diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index 07718c6c..d68ddfd1 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -322,10 +322,9 @@ impl FileModuleResolver { let scope = Scope::new(); - let m: Shared<_> = if let Some(global) = global { - Module::eval_ast_as_new_raw(engine, scope, global, &ast) - } else { - Module::eval_ast_as_new(scope, &ast, engine) + let m: Shared<_> = match global { + Some(global) => Module::eval_ast_as_new_raw(engine, scope, global, &ast), + None => Module::eval_ast_as_new(scope, &ast, engine), } .map_err(|err| Box::new(ERR::ErrorInModule(path.to_string(), err, pos)))? .into(); diff --git a/src/optimizer.rs b/src/optimizer.rs index 79e1c130..a0f187c9 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -551,13 +551,14 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // switch const { case if condition => stmt, _ => def } => if condition { stmt } else { def } optimize_expr(&mut b.condition, state, false); - let else_stmt = if let Some(index) = def_case { - let mut def_stmt = - Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); - optimize_stmt(&mut def_stmt, state, true); - def_stmt.into() - } else { - StmtBlock::NONE + let else_stmt = match def_case { + Some(index) => { + let mut def_stmt = + Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); + optimize_stmt(&mut def_stmt, state, true); + def_stmt.into() + } + _ => StmtBlock::NONE, }; *stmt = Stmt::If( @@ -616,13 +617,14 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // switch const { range if condition => stmt, _ => def } => if condition { stmt } else { def } optimize_expr(&mut condition, state, false); - let else_stmt = if let Some(index) = def_case { - let mut def_stmt = - Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); - optimize_stmt(&mut def_stmt, state, true); - def_stmt.into() - } else { - StmtBlock::NONE + let else_stmt = match def_case { + Some(index) => { + let mut def_stmt = + Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); + optimize_stmt(&mut def_stmt, state, true); + def_stmt.into() + } + _ => StmtBlock::NONE, }; let if_stmt = @@ -665,12 +667,13 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // Promote the default case state.set_dirty(); - if let Some(index) = def_case { - let mut def_stmt = Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); - optimize_stmt(&mut def_stmt, state, true); - *stmt = def_stmt; - } else { - *stmt = StmtBlock::empty(*pos).into(); + match def_case { + Some(index) => { + let mut def_stmt = Stmt::Expr(mem::take(&mut expressions[*index].expr).into()); + optimize_stmt(&mut def_stmt, state, true); + *stmt = def_stmt; + } + _ => *stmt = StmtBlock::empty(*pos).into(), } } // switch diff --git a/src/packages/map_basic.rs b/src/packages/map_basic.rs index 2d25641d..3360be62 100644 --- a/src/packages/map_basic.rs +++ b/src/packages/map_basic.rs @@ -82,10 +82,11 @@ mod map_functions { /// print(m); // prints "#{a: 1, b: 42, c: 3, x: 0}" /// ``` pub fn set(map: &mut Map, property: &str, value: Dynamic) { - if let Some(value_ref) = map.get_mut(property) { - *value_ref = value; - } else { - map.insert(property.into(), value); + match map.get_mut(property) { + Some(value_ref) => *value_ref = value, + _ => { + map.insert(property.into(), value); + } } } /// Clear the object map. diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index cfaf3e83..72c4a6ca 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -239,10 +239,9 @@ mod string_functions { /// Clear the string, making it empty. pub fn clear(string: &mut ImmutableString) { if !string.is_empty() { - if let Some(s) = string.get_mut() { - s.clear(); - } else { - *string = ImmutableString::new(); + match string.get_mut() { + Some(s) => s.clear(), + _ => *string = ImmutableString::new(), } } } @@ -287,17 +286,20 @@ mod string_functions { /// print(text); // prints "hello" /// ``` pub fn trim(string: &mut ImmutableString) { - if let Some(s) = string.get_mut() { - let trimmed = s.trim(); + match string.get_mut() { + Some(s) => { + let trimmed = s.trim(); - if trimmed != s { - *s = trimmed.into(); + if trimmed != s { + *s = trimmed.into(); + } } - } else { - let trimmed = string.trim(); + None => { + let trimmed = string.trim(); - if trimmed != string { - *string = trimmed.into(); + if trimmed != string { + *string = trimmed.into(); + } } } } diff --git a/src/serde/deserialize.rs b/src/serde/deserialize.rs index e6c5ae3a..2534a1d5 100644 --- a/src/serde/deserialize.rs +++ b/src/serde/deserialize.rs @@ -215,10 +215,9 @@ impl<'de> Deserialize<'de> for Scope<'de> { where A: SeqAccess<'de>, { - let mut scope = if let Some(size) = access.size_hint() { - Scope::with_capacity(size) - } else { - Scope::new() + let mut scope = match access.size_hint() { + Some(size) => Scope::with_capacity(size), + None => Scope::new(), }; while let Some(ScopeEntry { diff --git a/src/serde/serialize.rs b/src/serde/serialize.rs index 818b0e9e..69a280ef 100644 --- a/src/serde/serialize.rs +++ b/src/serde/serialize.rs @@ -37,10 +37,9 @@ impl Serialize for Dynamic { Union::Decimal(ref x, ..) => { use rust_decimal::prelude::ToPrimitive; - if let Some(v) = x.to_f64() { - ser.serialize_f64(v) - } else { - ser.serialize_str(&x.to_string()) + match x.to_f64() { + Some(v) => ser.serialize_f64(v), + None => ser.serialize_str(&x.to_string()), } } #[cfg(feature = "decimal")] @@ -48,10 +47,9 @@ impl Serialize for Dynamic { Union::Decimal(ref x, ..) => { use rust_decimal::prelude::ToPrimitive; - if let Some(v) = x.to_f32() { - ser.serialize_f32(v) - } else { - ser.serialize_str(&x.to_string()) + match x.to_f32() { + Some(v) => ser.serialize_f32(v), + _ => ser.serialize_str(&x.to_string()), } }