From 0a959483bbf95da60385241df9dd9d27a2dd90e0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 22 Mar 2020 00:29:33 +0800 Subject: [PATCH 1/9] Add LICENSE. --- LICENSE.txt | 201 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 LICENSE.txt diff --git a/LICENSE.txt b/LICENSE.txt new file mode 100644 index 00000000..261eeb9e --- /dev/null +++ b/LICENSE.txt @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. From 60b52c142e793e78e731662dc63f3995e3a361c2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 29 Apr 2020 23:03:18 +0800 Subject: [PATCH 2/9] Fix README example for on_print and on_debug. --- README.md | 13 ++++++++----- tests/print.rs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 tests/print.rs diff --git a/README.md b/README.md index 0e8d4b77..f89bc940 100644 --- a/README.md +++ b/README.md @@ -1979,17 +1979,20 @@ engine.on_print(|x| println!("hello: {}", x)); engine.on_debug(|x| println!("DEBUG: {}", x)); // Example: quick-'n-dirty logging -let mut log: Vec = Vec::new(); +let logbook = Arc::new(RwLock::new(Vec::::new())); // Redirect print/debug output to 'log' -engine.on_print(|s| log.push(format!("entry: {}", s))); -engine.on_debug(|s| log.push(format!("DEBUG: {}", s))); +let log = logbook.clone(); +engine.on_print(move |s| log.write().unwrap().push(format!("entry: {}", s))); + +let log = logbook.clone(); +engine.on_debug(move |s| log.write().unwrap().push(format!("DEBUG: {}", s))); // Evaluate script engine.eval::<()>(script)?; -// 'log' captures all the 'print' and 'debug' output -for entry in log { +// 'logbook' captures all the 'print' and 'debug' output +for entry in logbook.read().unwrap().iter() { println!("{}", entry); } ``` diff --git a/tests/print.rs b/tests/print.rs new file mode 100644 index 00000000..ec707e8b --- /dev/null +++ b/tests/print.rs @@ -0,0 +1,31 @@ +use rhai::{Engine, EvalAltResult}; +use std::sync::{Arc, RwLock}; + +#[test] +fn test_print() -> Result<(), Box> { + let mut engine = Engine::new(); + + let logbook = Arc::new(RwLock::new(Vec::::new())); + + // Redirect print/debug output to 'log' + let log = logbook.clone(); + engine.on_print(move |s| log.write().unwrap().push(format!("entry: {}", s))); + + let log = logbook.clone(); + engine.on_debug(move |s| log.write().unwrap().push(format!("DEBUG: {}", s))); + + // Evaluate script + engine.eval::<()>("print(40 + 2)")?; + engine.eval::<()>(r#"debug("hello!")"#)?; + + // 'logbook' captures all the 'print' and 'debug' output + assert_eq!(logbook.read().unwrap().len(), 2); + assert_eq!(logbook.read().unwrap()[0], "entry: 42"); + assert_eq!(logbook.read().unwrap()[1], r#"DEBUG: "hello!""#); + + for entry in logbook.read().unwrap().iter() { + println!("{}", entry); + } + + Ok(()) +} From 6dcbfc8e4b609181c766880335042dd0d9838143 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 29 Apr 2020 23:19:29 +0800 Subject: [PATCH 3/9] Delete benchmark.yml --- .github/workflows/benchmark.yml | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 .github/workflows/benchmark.yml diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml deleted file mode 100644 index df310705..00000000 --- a/.github/workflows/benchmark.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Benchmark -on: - push: - branches: - - master - -jobs: - benchmark: - name: Run Rust benchmark - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - run: rustup toolchain update nightly && rustup default nightly - - name: Run benchmark - run: cargo +nightly bench | tee output.txt - - name: Store benchmark result - uses: rhysd/github-action-benchmark@v1 - with: - name: Rust Benchmark - tool: 'cargo' - output-file-path: output.txt - # Use personal access token instead of GITHUB_TOKEN due to https://github.community/t5/GitHub-Actions/Github-action-not-triggering-gh-pages-upon-push/td-p/26869/highlight/false - github-token: ${{ secrets.RHAI }} - auto-push: true - # Show alert with commit comment on detecting possible performance regression - alert-threshold: '200%' - comment-on-alert: true - fail-on-alert: true - alert-comment-cc-users: '@schungx' From 4f91e7fbcf5c51acd00ad6338422cdb348bd358b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 30 Apr 2020 22:52:36 +0800 Subject: [PATCH 4/9] Avoid copying in Dynamic. --- src/any.rs | 30 ++++---- src/engine.rs | 201 ++++++++++++++++++++++---------------------------- 2 files changed, 102 insertions(+), 129 deletions(-) diff --git a/src/any.rs b/src/any.rs index 46ae6044..d9aad2a6 100644 --- a/src/any.rs +++ b/src/any.rs @@ -252,6 +252,12 @@ impl Clone for Dynamic { } } +impl Default for Dynamic { + fn default() -> Self { + Self(Union::Unit(())) + } +} + /// Cast a Boxed type into another type. fn cast_box(item: Box) -> Result> { // Only allow casting to the exact same type @@ -359,22 +365,16 @@ impl Dynamic { return cast_box::<_, T>(Box::new(self)).ok(); } - match &self.0 { - Union::Unit(value) => (value as &dyn Variant).downcast_ref::().cloned(), - Union::Bool(value) => (value as &dyn Variant).downcast_ref::().cloned(), - Union::Str(value) => (value.as_ref() as &dyn Variant) - .downcast_ref::() - .cloned(), - Union::Char(value) => (value as &dyn Variant).downcast_ref::().cloned(), - Union::Int(value) => (value as &dyn Variant).downcast_ref::().cloned(), + match self.0 { + Union::Unit(ref value) => (value as &dyn Variant).downcast_ref::().cloned(), + Union::Bool(ref value) => (value as &dyn Variant).downcast_ref::().cloned(), + Union::Str(value) => cast_box::<_, T>(value).ok(), + Union::Char(ref value) => (value as &dyn Variant).downcast_ref::().cloned(), + Union::Int(ref value) => (value as &dyn Variant).downcast_ref::().cloned(), #[cfg(not(feature = "no_float"))] - Union::Float(value) => (value as &dyn Variant).downcast_ref::().cloned(), - Union::Array(value) => (value.as_ref() as &dyn Variant) - .downcast_ref::() - .cloned(), - Union::Map(value) => (value.as_ref() as &dyn Variant) - .downcast_ref::() - .cloned(), + Union::Float(ref value) => (value as &dyn Variant).downcast_ref::().cloned(), + Union::Array(value) => cast_box::<_, T>(value).ok(), + Union::Map(value) => cast_box::<_, T>(value).ok(), Union::Variant(value) => value.as_ref().as_ref().downcast_ref::().cloned(), } } diff --git a/src/engine.rs b/src/engine.rs index 9c0ed804..f2fe8824 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -88,7 +88,7 @@ enum Target<'a> { impl Target<'_> { /// Get the value of the `Target` as a `Dynamic`. - pub fn into_dynamic(self) -> Dynamic { + pub fn clone_into_dynamic(self) -> Dynamic { match self { Target::Ref(r) => r.clone(), Target::Value(v) => *v, @@ -111,7 +111,7 @@ impl Target<'_> { .map_err(|_| EvalAltResult::ErrorCharMismatch(pos))?; let mut chars: Vec = s.chars().collect(); - let ch = *chars.get(x.1).expect("string index out of bounds"); + let ch = chars[x.1]; // See if changed - if so, update the String if ch != new_ch { @@ -141,26 +141,31 @@ impl> From for Target<'_> { /// A type to hold a number of `Dynamic` values in static storage for speed, /// and any spill-overs in a `Vec`. -struct StaticVec { +struct StaticVec { /// Total number of values held. len: usize, - /// Static storage. - list: [Dynamic; 4], - /// Dynamic storage. - more: Vec, + /// Static storage. 4 slots should be enough for most cases - i.e. four levels of indirection. + list: [T; 4], + /// Dynamic storage. For spill-overs. + more: Vec, } -impl StaticVec { +impl StaticVec { /// Create a new `StaticVec`. pub fn new() -> Self { Self { len: 0, - list: [().into(), ().into(), ().into(), ().into()], + list: [ + Default::default(), + Default::default(), + Default::default(), + Default::default(), + ], more: Vec::new(), } } /// Push a new value to the end of this `StaticVec`. - pub fn push>(&mut self, value: T) { + pub fn push>(&mut self, value: X) { if self.len >= self.list.len() { self.more.push(value.into()); } else { @@ -173,11 +178,11 @@ impl StaticVec { /// # Panics /// /// Panics if the `StaticVec` is empty. - pub fn pop(&mut self) -> Dynamic { + pub fn pop(&mut self) -> T { let result = if self.len <= 0 { panic!("nothing to pop!") } else if self.len <= self.list.len() { - mem::replace(self.list.get_mut(self.len - 1).unwrap(), ().into()) + mem::replace(self.list.get_mut(self.len - 1).unwrap(), Default::default()) } else { self.more.pop().unwrap() }; @@ -572,24 +577,24 @@ impl Engine { }); } + // Getter function not found? if let Some(prop) = extract_prop_from_getter(fn_name) { - // Getter function not found return Err(Box::new(EvalAltResult::ErrorDotExpr( format!("- property '{}' unknown or write-only", prop), pos, ))); } + // Setter function not found? if let Some(prop) = extract_prop_from_setter(fn_name) { - // Setter function not found return Err(Box::new(EvalAltResult::ErrorDotExpr( format!("- property '{}' unknown or read-only", prop), pos, ))); } + // Return default value (if any) if let Some(val) = def_val { - // Return default value return Ok(val.clone()); } @@ -621,8 +626,8 @@ impl Engine { let scope_len = scope.len(); let mut state = State::new(); + // Put arguments into scope as variables - variable name is copied scope.extend( - // Put arguments into scope as variables - variable name is copied // TODO - avoid copying variable name fn_def .params @@ -649,8 +654,8 @@ impl Engine { let mut scope = Scope::new(); let mut state = State::new(); + // Put arguments into scope as variables scope.extend( - // Put arguments into scope as variables fn_def .params .iter() @@ -698,7 +703,7 @@ impl Engine { Ok(self.map_type_name(args[0].type_name()).to_string().into()) } - // eval + // eval - reaching this point it must be a method-style call KEYWORD_EVAL if args.len() == 1 && !self.has_override(fn_lib, KEYWORD_EVAL) => { Err(Box::new(EvalAltResult::ErrorRuntime( "'eval' should not be called in method style. Try eval(...);".into(), @@ -706,6 +711,7 @@ impl Engine { ))) } + // Normal method call _ => self.call_fn_raw(None, fn_lib, fn_name, args, def_val, pos, level), } } @@ -757,7 +763,7 @@ impl Engine { fn_lib: &FunctionsLib, mut target: Target, rhs: &Expr, - idx_values: &mut StaticVec, + idx_values: &mut StaticVec, is_index: bool, op_pos: Position, level: usize, @@ -790,12 +796,12 @@ impl Engine { _ if new_val.is_some() => { let mut indexed_val = self.get_indexed_mut(obj, idx_val, rhs.position(), op_pos, true)?; indexed_val.set_value(new_val.unwrap(), rhs.position())?; - Ok((().into(), true)) + Ok((Default::default(), true)) } // xxx[rhs] _ => self .get_indexed_mut(obj, idx_val, rhs.position(), op_pos, false) - .map(|v| (v.into_dynamic(), false)) + .map(|v| (v.clone_into_dynamic(), false)) } } else { match rhs { @@ -814,13 +820,13 @@ impl Engine { let mut indexed_val = self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, true)?; indexed_val.set_value(new_val.unwrap(), rhs.position())?; - Ok((().into(), true)) + Ok((Default::default(), true)) } // {xxx:map}.id Expr::Property(id, pos) if obj.is::() => { let indexed_val = self.get_indexed_mut(obj, id.to_string().into(), *pos, op_pos, false)?; - Ok((indexed_val.into_dynamic(), false)) + Ok((indexed_val.clone_into_dynamic(), false)) } // xxx.id = ??? Expr::Property(id, pos) if new_val.is_some() => { @@ -858,8 +864,7 @@ impl Engine { // xxx.dot_lhs.rhs Expr::Dot(dot_lhs, dot_rhs, pos) => { let is_index = matches!(rhs, Expr::Index(_,_,_)); - let mut buf: Dynamic = ().into(); - let mut args = [obj, &mut buf]; + let mut args = [obj, &mut Default::default()]; let indexed_val = &mut (if let Expr::Property(id, pos) = dot_lhs.as_ref() { let fn_name = make_getter(id); @@ -871,12 +876,12 @@ impl Engine { rhs.position(), ))); }); - let (result, changed) = self.eval_dot_index_chain_helper( + let (result, may_be_changed) = self.eval_dot_index_chain_helper( fn_lib, indexed_val.into(), dot_rhs, idx_values, is_index, *pos, level, new_val )?; // Feed the value back via a setter just in case it has been updated - if changed { + if may_be_changed { if let Expr::Property(id, pos) = dot_lhs.as_ref() { let fn_name = make_setter(id); args[1] = indexed_val; @@ -884,7 +889,7 @@ impl Engine { } } - Ok((result, changed)) + Ok((result, may_be_changed)) } // Syntax error _ => Err(Box::new(EvalAltResult::ErrorDotExpr( @@ -931,15 +936,9 @@ impl Engine { _ => (), } + let this_ptr = target.into(); self.eval_dot_index_chain_helper( - fn_lib, - target.into(), - dot_rhs, - idx_values, - is_index, - op_pos, - level, - new_val, + fn_lib, this_ptr, dot_rhs, idx_values, is_index, op_pos, level, new_val, ) .map(|(v, _)| v) } @@ -952,16 +951,9 @@ impl Engine { // {expr}.??? or {expr}[???] expr => { let val = self.eval_expr(scope, state, fn_lib, expr, level)?; - + let this_ptr = val.into(); self.eval_dot_index_chain_helper( - fn_lib, - val.into(), - dot_rhs, - idx_values, - is_index, - op_pos, - level, - new_val, + fn_lib, this_ptr, dot_rhs, idx_values, is_index, op_pos, level, new_val, ) .map(|(v, _)| v) } @@ -979,7 +971,7 @@ impl Engine { state: &mut State, fn_lib: &FunctionsLib, expr: &Expr, - idx_values: &mut StaticVec, + idx_values: &mut StaticVec, size: usize, level: usize, ) -> Result<(), Box> { @@ -992,12 +984,11 @@ impl Engine { idx_values.push(arg_values) } - // Store a placeholder - no need to copy the property name - Expr::Property(_, _) => idx_values.push(()), + Expr::Property(_, _) => idx_values.push(()), // Store a placeholder - no need to copy the property name Expr::Index(lhs, rhs, _) | Expr::Dot(lhs, rhs, _) => { // Evaluate in left-to-right order let lhs_val = match lhs.as_ref() { - Expr::Property(_, _) => ().into(), // Store a placeholder in case of a property + Expr::Property(_, _) => Default::default(), // Store a placeholder in case of a property _ => self.eval_expr(scope, state, fn_lib, lhs, level)?, }; @@ -1052,7 +1043,7 @@ impl Engine { .map_err(|_| EvalAltResult::ErrorStringIndexExpr(idx_pos))?; Ok(if create { - map.entry(index).or_insert(().into()).into() + map.entry(index).or_insert(Default::default()).into() } else { map.get_mut(&index) .map(Target::from) @@ -1103,40 +1094,35 @@ impl Engine { match rhs_value { Dynamic(Union::Array(mut rhs_value)) => { let def_value = false.into(); - let mut result = false; // Call the '==' operator to compare each value for value in rhs_value.iter_mut() { let args = &mut [&mut lhs_value, value]; let def_value = Some(&def_value); + if self .call_fn_raw(None, fn_lib, "==", args, def_value, rhs.position(), level)? .as_bool() .unwrap_or(false) { - result = true; - break; + return Ok(true.into()); } } - Ok(result.into()) + Ok(false.into()) } - Dynamic(Union::Map(rhs_value)) => { + Dynamic(Union::Map(rhs_value)) => match lhs_value { // Only allows String or char - match lhs_value { - Dynamic(Union::Str(s)) => Ok(rhs_value.contains_key(s.as_ref()).into()), - Dynamic(Union::Char(c)) => Ok(rhs_value.contains_key(&c.to_string()).into()), - _ => Err(Box::new(EvalAltResult::ErrorInExpr(lhs.position()))), - } - } - Dynamic(Union::Str(rhs_value)) => { + Dynamic(Union::Str(s)) => Ok(rhs_value.contains_key(s.as_ref()).into()), + Dynamic(Union::Char(c)) => Ok(rhs_value.contains_key(&c.to_string()).into()), + _ => Err(Box::new(EvalAltResult::ErrorInExpr(lhs.position()))), + }, + Dynamic(Union::Str(rhs_value)) => match lhs_value { // Only allows String or char - match lhs_value { - Dynamic(Union::Str(s)) => Ok(rhs_value.contains(s.as_ref()).into()), - Dynamic(Union::Char(c)) => Ok(rhs_value.contains(c).into()), - _ => Err(Box::new(EvalAltResult::ErrorInExpr(lhs.position()))), - } - } + Dynamic(Union::Str(s)) => Ok(rhs_value.contains(s.as_ref()).into()), + Dynamic(Union::Char(c)) => Ok(rhs_value.contains(c).into()), + _ => Err(Box::new(EvalAltResult::ErrorInExpr(lhs.position()))), + }, _ => Err(Box::new(EvalAltResult::ErrorInExpr(rhs.position()))), } } @@ -1178,18 +1164,14 @@ impl Engine { *pos, ))) } - - Some((index, ScopeEntryType::Normal)) => { - // Avoid referencing scope which is used below as mut - *scope.get_mut(index).0 = rhs_val.clone(); - Ok(rhs_val) - } - Some((_, ScopeEntryType::Constant)) => Err(Box::new( EvalAltResult::ErrorAssignmentToConstant(name.to_string(), *op_pos), )), + Some((index, ScopeEntryType::Normal)) => { + *scope.get_mut(index).0 = rhs_val; + Ok(Default::default()) + } }, - // idx_lhs[idx_expr] = rhs #[cfg(not(feature = "no_index"))] Expr::Index(idx_lhs, idx_expr, op_pos) => { @@ -1213,7 +1195,6 @@ impl Engine { lhs.position(), ))) } - // Syntax error _ => Err(Box::new(EvalAltResult::ErrorAssignmentToUnknownLHS( lhs.position(), @@ -1234,30 +1215,23 @@ impl Engine { ), #[cfg(not(feature = "no_index"))] - Expr::Array(contents, _) => { - let mut arr = Array::new(); - - contents.into_iter().try_for_each(|item| { - self.eval_expr(scope, state, fn_lib, item, level) - .map(|val| arr.push(val)) - })?; - - Ok(Dynamic(Union::Array(Box::new(arr)))) - } + Expr::Array(contents, _) => Ok(Dynamic(Union::Array(Box::new( + contents + .into_iter() + .map(|item| self.eval_expr(scope, state, fn_lib, item, level)) + .collect::, _>>()?, + )))), #[cfg(not(feature = "no_object"))] - Expr::Map(contents, _) => { - let mut map = Map::new(); - - contents.into_iter().try_for_each(|(key, expr, _)| { - self.eval_expr(scope, state, fn_lib, &expr, level) - .map(|val| { - map.insert(key.clone(), val); - }) - })?; - - Ok(Dynamic(Union::Map(Box::new(map)))) - } + Expr::Map(contents, _) => Ok(Dynamic(Union::Map(Box::new( + contents + .into_iter() + .map(|(key, expr, _)| { + self.eval_expr(scope, state, fn_lib, &expr, level) + .map(|val| (key.clone(), val)) + }) + .collect::, _>>()?, + )))), Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => { let mut arg_values = arg_exprs @@ -1287,9 +1261,8 @@ impl Engine { return result; } - // Normal function call - let def_val = def_val.as_deref(); - self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, level) + // Normal function call - except for eval (handled above) + self.exec_fn_call(fn_lib, fn_name, &mut args, def_val.as_deref(), *pos, level) } Expr::In(lhs, rhs, _) => { @@ -1345,7 +1318,7 @@ impl Engine { ) -> Result> { match stmt { // No-op - Stmt::Noop(_) => Ok(().into()), + Stmt::Noop(_) => Ok(Default::default()), // Expression as statement Stmt::Expr(expr) => { @@ -1353,7 +1326,7 @@ impl Engine { Ok(if let Expr::Assignment(_, _, _) = *expr.as_ref() { // If it is an assignment, erase the result at the root - ().into() + Default::default() } else { result }) @@ -1363,7 +1336,7 @@ impl Engine { Stmt::Block(block, _) => { let prev_len = scope.len(); - let result = block.iter().try_fold(().into(), |_, stmt| { + let result = block.iter().try_fold(Default::default(), |_, stmt| { self.eval_stmt(scope, state, fn_lib, stmt, level) }); @@ -1387,7 +1360,7 @@ impl Engine { } else if let Some(stmt) = else_body { self.eval_stmt(scope, state, fn_lib, stmt.as_ref(), level) } else { - Ok(().into()) + Ok(Default::default()) } }), @@ -1401,11 +1374,11 @@ impl Engine { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), - EvalAltResult::ErrorLoopBreak(true, _) => return Ok(().into()), + EvalAltResult::ErrorLoopBreak(true, _) => return Ok(Default::default()), _ => return Err(err), }, }, - Ok(false) => return Ok(().into()), + Ok(false) => return Ok(Default::default()), Err(_) => { return Err(Box::new(EvalAltResult::ErrorLogicGuard(guard.position()))) } @@ -1418,7 +1391,7 @@ impl Engine { Ok(_) => (), Err(err) => match *err { EvalAltResult::ErrorLoopBreak(false, _) => (), - EvalAltResult::ErrorLoopBreak(true, _) => return Ok(().into()), + EvalAltResult::ErrorLoopBreak(true, _) => return Ok(Default::default()), _ => return Err(err), }, } @@ -1453,7 +1426,7 @@ impl Engine { } scope.rewind(scope.len() - 1); - Ok(().into()) + Ok(Default::default()) } else { Err(Box::new(EvalAltResult::ErrorFor(expr.position()))) } @@ -1467,7 +1440,7 @@ impl Engine { // Empty return Stmt::ReturnWithVal(None, ReturnType::Return, pos) => { - Err(Box::new(EvalAltResult::Return(().into(), *pos))) + Err(Box::new(EvalAltResult::Return(Default::default(), *pos))) } // Return value @@ -1494,13 +1467,13 @@ impl Engine { let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? scope.push_dynamic_value(name.clone(), ScopeEntryType::Normal, val, false); - Ok(().into()) + Ok(Default::default()) } Stmt::Let(name, None, _) => { // TODO - avoid copying variable name in inner block? scope.push(name.clone(), ()); - Ok(().into()) + Ok(Default::default()) } // Const statement @@ -1508,7 +1481,7 @@ impl Engine { let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? scope.push_dynamic_value(name.clone(), ScopeEntryType::Constant, val, true); - Ok(().into()) + Ok(Default::default()) } Stmt::Const(_, _, _) => panic!("constant expression not constant!"), From 5c5e1db61edede669ba0f21ae475bb05d70527ce Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 1 May 2020 10:17:19 +0800 Subject: [PATCH 5/9] Cleanup code. --- src/packages/iter_basic.rs | 5 ++--- src/packages/utils.rs | 38 -------------------------------------- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index 9672d61c..031ac095 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -1,8 +1,7 @@ -use super::{reg_binary, reg_trinary, reg_unary_mut, PackageStore}; +use super::{reg_binary, reg_trinary, PackageStore}; -use crate::any::{Dynamic, Union, Variant}; +use crate::any::{Dynamic, Variant}; use crate::def_package; -use crate::engine::{Array, Map}; use crate::fn_register::map_dynamic as map; use crate::parser::INT; diff --git a/src/packages/utils.rs b/src/packages/utils.rs index 122bb63a..fd631b22 100644 --- a/src/packages/utils.rs +++ b/src/packages/utils.rs @@ -239,44 +239,6 @@ pub fn reg_unary_mut( lib.functions.insert(hash, f); } -#[cfg(not(feature = "sync"))] -pub(crate) fn reg_test<'a, A: Variant + Clone, B: Variant + Clone, X, R>( - lib: &mut PackageStore, - fn_name: &'static str, - - #[cfg(not(feature = "sync"))] func: impl Fn(X, B) -> R + 'static, - #[cfg(feature = "sync")] func: impl Fn(X, B) -> R + Send + Sync + 'static, - - map: impl Fn(&mut A) -> X + 'static, - - #[cfg(not(feature = "sync"))] map_result: impl Fn(R, Position) -> Result> - + 'static, - #[cfg(feature = "sync")] map_result: impl Fn(R, Position) -> Result> - + Send - + Sync - + 'static, -) { - //println!("register {}({}, {})", fn_name, crate::std::any::type_name::(), crate::std::any::type_name::()); - - let hash = calc_fn_hash( - fn_name, - [TypeId::of::(), TypeId::of::()].iter().cloned(), - ); - - let f = Box::new(move |args: &mut FnCallArgs, pos: Position| { - check_num_args(fn_name, 2, args, pos)?; - - let mut drain = args.iter_mut(); - let x: X = map(drain.next().unwrap().downcast_mut::().unwrap()); - let y: B = drain.next().unwrap().downcast_mut::().unwrap().clone(); - - let r = func(x, y); - map_result(r, pos) - }); - - lib.functions.insert(hash, f); -} - /// Add a function with two parameters to the package. /// /// `map_result` is a function that maps the return type of the function to `Result`. From fb2e48cca24a0eef6880ebc2534cc265ac0f54d8 Mon Sep 17 00:00:00 2001 From: Cheng JIANG Date: Fri, 1 May 2020 11:27:27 +0200 Subject: [PATCH 6/9] fix naming --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0e8d4b77..f66ea4ca 100644 --- a/README.md +++ b/README.md @@ -376,7 +376,7 @@ The follow packages are available: | Package | Description | In `CorePackage` | In `StandardPackage` | | ------------------------ | ----------------------------------------------- | :--------------: | :------------------: | -| `BasicArithmeticPackage` | Arithmetic operators (e.g. `+`, `-`, `*`, `/`) | Yes | Yes | +| `ArithmeticPackage` | Arithmetic operators (e.g. `+`, `-`, `*`, `/`) | Yes | Yes | | `BasicIteratorPackage` | Numeric ranges (e.g. `range(1, 10)`) | Yes | Yes | | `LogicPackage` | Logic and comparison operators (e.g. `==`, `>`) | Yes | Yes | | `BasicStringPackage` | Basic string functions | Yes | Yes | From c7c7fe3dfc059795ed01b159a48cec6d6c041ee4 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 1 May 2020 17:32:39 +0800 Subject: [PATCH 7/9] Reduce size of Expr and Stmt by Boxing strings. --- src/engine.rs | 20 +++--- src/optimize.rs | 18 ++--- src/parser.rs | 187 ++++++++++++++++++++++++++++++------------------ 3 files changed, 140 insertions(+), 85 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index f2fe8824..cb92a5a9 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -806,7 +806,7 @@ impl Engine { } else { match rhs { // xxx.fn_name(arg_expr_list) - Expr::FunctionCall(fn_name, _, def_val, pos) => { + Expr::FnCall(fn_name, _, def_val, pos) => { let mut args: Vec<_> = once(obj) .chain(idx_val.downcast_mut::().unwrap().iter_mut()) .collect(); @@ -976,7 +976,7 @@ impl Engine { level: usize, ) -> Result<(), Box> { match expr { - Expr::FunctionCall(_, arg_exprs, _, _) => { + Expr::FnCall(_, arg_exprs, _, _) => { let arg_values = arg_exprs .iter() .map(|arg_expr| self.eval_expr(scope, state, fn_lib, arg_expr, level)) @@ -1233,7 +1233,7 @@ impl Engine { .collect::, _>>()?, )))), - Expr::FunctionCall(fn_name, arg_exprs, def_val, pos) => { + Expr::FnCall(fn_name, arg_exprs, def_val, pos) => { let mut arg_values = arg_exprs .iter() .map(|expr| self.eval_expr(scope, state, fn_lib, expr, level)) @@ -1242,7 +1242,7 @@ impl Engine { let mut args: Vec<_> = arg_values.iter_mut().collect(); // eval - only in function call style - if fn_name == KEYWORD_EVAL + if fn_name.as_ref() == KEYWORD_EVAL && args.len() == 1 && !self.has_override(fn_lib, KEYWORD_EVAL) { @@ -1409,7 +1409,8 @@ impl Engine { .and_then(|pkg| pkg.type_iterators.get(&tid)) }) { // Add the loop variable - scope.push(name.clone(), ()); + let var_name = name.as_ref().clone(); + scope.push(var_name, ()); let index = scope.len() - 1; for a in iter_fn(arr) { @@ -1466,13 +1467,15 @@ impl Engine { Stmt::Let(name, Some(expr), _) => { let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? - scope.push_dynamic_value(name.clone(), ScopeEntryType::Normal, val, false); + let var_name = name.as_ref().clone(); + scope.push_dynamic_value(var_name, ScopeEntryType::Normal, val, false); Ok(Default::default()) } Stmt::Let(name, None, _) => { // TODO - avoid copying variable name in inner block? - scope.push(name.clone(), ()); + let var_name = name.as_ref().clone(); + scope.push(var_name, ()); Ok(Default::default()) } @@ -1480,7 +1483,8 @@ impl Engine { Stmt::Const(name, expr, _) if expr.is_constant() => { let val = self.eval_expr(scope, state, fn_lib, expr, level)?; // TODO - avoid copying variable name in inner block? - scope.push_dynamic_value(name.clone(), ScopeEntryType::Constant, val, true); + let var_name = name.as_ref().clone(); + scope.push_dynamic_value(var_name, ScopeEntryType::Constant, val, true); Ok(Default::default()) } diff --git a/src/optimize.rs b/src/optimize.rs index a63075a3..2a8fab4c 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -558,18 +558,18 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { }, // Do not call some special keywords - Expr::FunctionCall(id, args, def_value, pos) if DONT_EVAL_KEYWORDS.contains(&id.as_ref())=> - Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), + Expr::FnCall(id, args, def_value, pos) if DONT_EVAL_KEYWORDS.contains(&id.as_ref().as_ref())=> + Expr::FnCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), // Eagerly call functions - Expr::FunctionCall(id, args, def_value, pos) + Expr::FnCall(id, args, def_value, pos) if state.optimization_level == OptimizationLevel::Full // full optimizations && args.iter().all(|expr| expr.is_constant()) // all arguments are constants => { // First search in script-defined functions (can override built-in) - if state.fn_lib.iter().find(|(name, len)| name == &id && *len == args.len()).is_some() { + if state.fn_lib.iter().find(|(name, len)| name == id.as_ref() && *len == args.len()).is_some() { // A script-defined function overrides the built-in function - do not make the call - return Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos); + return Expr::FnCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos); } let mut arg_values: Vec<_> = args.iter().map(Expr::get_constant_value).collect(); @@ -577,7 +577,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { // Save the typename of the first argument if it is `type_of()` // This is to avoid `call_args` being passed into the closure - let arg_for_type_of = if id == KEYWORD_TYPE_OF && call_args.len() == 1 { + let arg_for_type_of = if *id == KEYWORD_TYPE_OF && call_args.len() == 1 { state.engine.map_type_name(call_args[0].type_name()) } else { "" @@ -600,13 +600,13 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { }) ).unwrap_or_else(|| // Optimize function call arguments - Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos) + Expr::FnCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos) ) } // id(args ..) -> optimize function call arguments - Expr::FunctionCall(id, args, def_value, pos) => - Expr::FunctionCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), + Expr::FnCall(id, args, def_value, pos) => + Expr::FnCall(id, Box::new(args.into_iter().map(|a| optimize_expr(a, state)).collect()), def_value, pos), // constant-name Expr::Variable(name, _, pos) if state.contains_constant(&name) => { diff --git a/src/parser.rs b/src/parser.rs index dccad46e..e682ee12 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -231,11 +231,11 @@ pub enum Stmt { /// loop { stmt } Loop(Box), /// for id in expr { stmt } - For(String, Box, Box), + For(Box, Box, Box), /// let id = expr - Let(String, Option>, Position), + Let(Box, Option>, Position), /// const id = expr - Const(String, Box, Position), + Const(Box, Box, Position), /// { stmt; ... } Block(Vec, Position), /// { stmt } @@ -317,7 +317,7 @@ pub enum Expr { /// String constant. StringConstant(String, Position), /// Variable access. - Variable(String, Option, Position), + Variable(Box, Option, Position), /// Property access. Property(String, Position), /// { stmt } @@ -325,8 +325,8 @@ pub enum Expr { /// func(expr, ... ) /// Use `Cow<'static, str>` because a lot of operators (e.g. `==`, `>=`) are implemented as function calls /// and the function names are predictable, so no need to allocate a new `String`. - FunctionCall( - Cow<'static, str>, + FnCall( + Box>, Box>, Option>, Position, @@ -427,7 +427,7 @@ impl Expr { | Self::Variable(_, _, pos) | Self::Property(_, pos) | Self::Stmt(_, pos) - | Self::FunctionCall(_, _, _, pos) + | Self::FnCall(_, _, _, pos) | Self::And(_, _, pos) | Self::Or(_, _, pos) | Self::In(_, _, pos) @@ -453,7 +453,7 @@ impl Expr { | Self::Variable(_, _, pos) | Self::Property(_, pos) | Self::Stmt(_, pos) - | Self::FunctionCall(_, _, _, pos) + | Self::FnCall(_, _, _, pos) | Self::And(_, _, pos) | Self::Or(_, _, pos) | Self::In(_, _, pos) @@ -530,7 +530,7 @@ impl Expr { Self::StringConstant(_, _) | Self::Stmt(_, _) - | Self::FunctionCall(_, _, _, _) + | Self::FnCall(_, _, _, _) | Self::Assignment(_, _, _) | Self::Dot(_, _, _) | Self::Index(_, _, _) @@ -550,7 +550,7 @@ impl Expr { /// Convert a `Variable` into a `Property`. All other variants are untouched. pub(crate) fn into_property(self) -> Self { match self { - Self::Variable(id, _, pos) => Self::Property(id, pos), + Self::Variable(id, _, pos) => Self::Property(*id, pos), _ => self, } } @@ -633,7 +633,12 @@ fn parse_call_expr<'a>( // id() (Token::RightParen, _) => { eat_token(input, Token::RightParen); - return Ok(Expr::FunctionCall(id.into(), Box::new(args), None, begin)); + return Ok(Expr::FnCall( + Box::new(id.into()), + Box::new(args), + None, + begin, + )); } // id... _ => (), @@ -645,7 +650,12 @@ fn parse_call_expr<'a>( match input.peek().unwrap() { (Token::RightParen, _) => { eat_token(input, Token::RightParen); - return Ok(Expr::FunctionCall(id.into(), Box::new(args), None, begin)); + return Ok(Expr::FnCall( + Box::new(id.into()), + Box::new(args), + None, + begin, + )); } (Token::Comma, _) => { eat_token(input, Token::Comma); @@ -968,7 +978,7 @@ fn parse_primary<'a>( Token::StringConst(s) => Expr::StringConstant(s, pos), Token::Identifier(s) => { let index = stack.find(&s); - Expr::Variable(s, index, pos) + Expr::Variable(Box::new(s), index, pos) } Token::LeftParen => parse_paren_expr(input, stack, pos, allow_stmt_expr)?, #[cfg(not(feature = "no_index"))] @@ -995,8 +1005,10 @@ fn parse_primary<'a>( root_expr = match (root_expr, token) { // Function call - (Expr::Variable(id, _, pos), Token::LeftParen) - | (Expr::Property(id, pos), Token::LeftParen) => { + (Expr::Variable(id, _, pos), Token::LeftParen) => { + parse_call_expr(input, stack, *id, pos, allow_stmt_expr)? + } + (Expr::Property(id, pos), Token::LeftParen) => { parse_call_expr(input, stack, id, pos, allow_stmt_expr)? } // Indexing @@ -1055,7 +1067,12 @@ fn parse_unary<'a>( Expr::FloatConstant(f, pos) => Ok(Expr::FloatConstant(-f, pos)), // Call negative function - e => Ok(Expr::FunctionCall("-".into(), Box::new(vec![e]), None, pos)), + e => Ok(Expr::FnCall( + Box::new("-".into()), + Box::new(vec![e]), + None, + pos, + )), } } // +expr @@ -1066,8 +1083,8 @@ fn parse_unary<'a>( // !expr (Token::Bang, _) => { let pos = eat_token(input, Token::Bang); - Ok(Expr::FunctionCall( - "!".into(), + Ok(Expr::FnCall( + Box::new("!".into()), Box::new(vec![parse_primary(input, stack, allow_stmt_expr)?]), Some(Box::new(false.into())), // NOT operator, when operating on invalid operand, defaults to false pos, @@ -1120,7 +1137,8 @@ fn parse_op_assignment_stmt<'a>( let rhs = parse_expr(input, stack, allow_stmt_expr)?; // lhs op= rhs -> lhs = op(lhs, rhs) - let rhs_expr = Expr::FunctionCall(op.into(), Box::new(vec![lhs_copy, rhs]), None, pos); + let args = vec![lhs_copy, rhs]; + let rhs_expr = Expr::FnCall(Box::new(op.into()), Box::new(args), None, pos); Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs_expr), pos)) } @@ -1337,65 +1355,89 @@ fn parse_binary_op<'a>( let cmp_default = Some(Box::new(false.into())); current_lhs = match op_token { - Token::Plus => { - Expr::FunctionCall("+".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::Minus => { - Expr::FunctionCall("-".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::Multiply => { - Expr::FunctionCall("*".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::Divide => { - Expr::FunctionCall("/".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } + Token::Plus => Expr::FnCall( + Box::new("+".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::Minus => Expr::FnCall( + Box::new("-".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::Multiply => Expr::FnCall( + Box::new("*".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::Divide => Expr::FnCall( + Box::new("/".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), - Token::LeftShift => { - Expr::FunctionCall("<<".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::RightShift => { - Expr::FunctionCall(">>".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::Modulo => { - Expr::FunctionCall("%".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::PowerOf => { - Expr::FunctionCall("~".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } + Token::LeftShift => Expr::FnCall( + Box::new("<<".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::RightShift => Expr::FnCall( + Box::new(">>".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::Modulo => Expr::FnCall( + Box::new("%".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::PowerOf => Expr::FnCall( + Box::new("~".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), // Comparison operators default to false when passed invalid operands - Token::EqualsTo => Expr::FunctionCall( - "==".into(), + Token::EqualsTo => Expr::FnCall( + Box::new("==".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, ), - Token::NotEqualsTo => Expr::FunctionCall( - "!=".into(), + Token::NotEqualsTo => Expr::FnCall( + Box::new("!=".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, ), - Token::LessThan => Expr::FunctionCall( - "<".into(), + Token::LessThan => Expr::FnCall( + Box::new("<".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, ), - Token::LessThanEqualsTo => Expr::FunctionCall( - "<=".into(), + Token::LessThanEqualsTo => Expr::FnCall( + Box::new("<=".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, ), - Token::GreaterThan => Expr::FunctionCall( - ">".into(), + Token::GreaterThan => Expr::FnCall( + Box::new(">".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, ), - Token::GreaterThanEqualsTo => Expr::FunctionCall( - ">=".into(), + Token::GreaterThanEqualsTo => Expr::FnCall( + Box::new(">=".into()), Box::new(vec![current_lhs, rhs]), cmp_default, pos, @@ -1403,15 +1445,24 @@ fn parse_binary_op<'a>( Token::Or => Expr::Or(Box::new(current_lhs), Box::new(rhs), pos), Token::And => Expr::And(Box::new(current_lhs), Box::new(rhs), pos), - Token::Ampersand => { - Expr::FunctionCall("&".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::Pipe => { - Expr::FunctionCall("|".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } - Token::XOr => { - Expr::FunctionCall("^".into(), Box::new(vec![current_lhs, rhs]), None, pos) - } + Token::Ampersand => Expr::FnCall( + Box::new("&".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::Pipe => Expr::FnCall( + Box::new("|".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), + Token::XOr => Expr::FnCall( + Box::new("^".into()), + Box::new(vec![current_lhs, rhs]), + None, + pos, + ), Token::In => make_in_expr(current_lhs, rhs, pos)?, @@ -1590,7 +1641,7 @@ fn parse_for<'a>( stack.rewind(prev_len); - Ok(Stmt::For(name, Box::new(expr), Box::new(body))) + Ok(Stmt::For(Box::new(name), Box::new(expr), Box::new(body))) } /// Parse a variable definition statement. @@ -1619,12 +1670,12 @@ fn parse_let<'a>( // let name = expr ScopeEntryType::Normal => { stack.push(name.clone()); - Ok(Stmt::Let(name, Some(Box::new(init_value)), pos)) + Ok(Stmt::Let(Box::new(name), Some(Box::new(init_value)), pos)) } // const name = { expr:constant } ScopeEntryType::Constant if init_value.is_constant() => { stack.push(name.clone()); - Ok(Stmt::Const(name, Box::new(init_value), pos)) + Ok(Stmt::Const(Box::new(name), Box::new(init_value), pos)) } // const name = expr - error ScopeEntryType::Constant => { @@ -1633,7 +1684,7 @@ fn parse_let<'a>( } } else { // let name - Ok(Stmt::Let(name, None, pos)) + Ok(Stmt::Let(Box::new(name), None, pos)) } } From de4120b3bc539ef7a8052f1352aadd6755895b96 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 1 May 2020 17:57:59 +0800 Subject: [PATCH 8/9] Undelete benchmark.yml --- .github/workflows/benchmark.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/benchmark.yml diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 00000000..df310705 --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,29 @@ +name: Benchmark +on: + push: + branches: + - master + +jobs: + benchmark: + name: Run Rust benchmark + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - run: rustup toolchain update nightly && rustup default nightly + - name: Run benchmark + run: cargo +nightly bench | tee output.txt + - name: Store benchmark result + uses: rhysd/github-action-benchmark@v1 + with: + name: Rust Benchmark + tool: 'cargo' + output-file-path: output.txt + # Use personal access token instead of GITHUB_TOKEN due to https://github.community/t5/GitHub-Actions/Github-action-not-triggering-gh-pages-upon-push/td-p/26869/highlight/false + github-token: ${{ secrets.RHAI }} + auto-push: true + # Show alert with commit comment on detecting possible performance regression + alert-threshold: '200%' + comment-on-alert: true + fail-on-alert: true + alert-comment-cc-users: '@schungx' From fc99b981a184a7a408cadc0793e23a0e6bd7e747 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 1 May 2020 23:39:55 +0800 Subject: [PATCH 9/9] Fix panic when string character index is OOB. --- src/engine.rs | 22 ++++++++++++++++------ src/optimize.rs | 10 ++-------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index cb92a5a9..c24b924c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1057,15 +1057,25 @@ impl Engine { .as_int() .map_err(|_| EvalAltResult::ErrorNumericIndexExpr(idx_pos))?; - let num_chars = s.chars().count(); - if index >= 0 { - let index = index as usize; - let ch = s.chars().nth(index).unwrap(); - Ok(Target::StringChar(Box::new((val, index, ch.into())))) + let ch = s.chars().nth(index as usize).ok_or_else(|| { + Box::new(EvalAltResult::ErrorStringBounds( + s.chars().count(), + index, + idx_pos, + )) + })?; + + Ok(Target::StringChar(Box::new(( + val, + index as usize, + ch.into(), + )))) } else { Err(Box::new(EvalAltResult::ErrorStringBounds( - num_chars, index, idx_pos, + s.chars().count(), + index, + idx_pos, ))) } } diff --git a/src/optimize.rs b/src/optimize.rs index 2a8fab4c..268c504a 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -435,9 +435,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { .unwrap_or_else(|| Expr::Unit(pos)) } // string[int] - (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, _)) - if i >= 0 && (i as usize) < s.chars().count() => - { + (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, _)) if i >= 0 && (i as usize) < s.chars().count() => { // String literal indexing - get the character state.set_dirty(); Expr::CharConstant(s.chars().nth(i as usize).expect("should get char"), pos) @@ -550,11 +548,7 @@ fn optimize_expr<'a>(expr: Expr, state: &mut State<'a>) -> Expr { optimize_expr(lhs, state) } // lhs || rhs - (lhs, rhs) => Expr::Or( - Box::new(optimize_expr(lhs, state)), - Box::new(optimize_expr(rhs, state)), - pos - ), + (lhs, rhs) => Expr::Or(Box::new(optimize_expr(lhs, state)), Box::new(optimize_expr(rhs, state)), pos), }, // Do not call some special keywords