From cdc8005146c4f39057003a0e96d356d2edad29b8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 15 Feb 2022 11:38:35 +0800 Subject: [PATCH 01/17] Bump version. --- CHANGELOG.md | 6 ++---- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17c0f8fa..d517f055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,9 @@ Version 1.5.0 This version adds a debugging interface, which can be used to integrate a debugger. -Based on popular demand, an option is added to throw exceptions when invalid properties are accessed -on object maps (default is to return `()`). +Based on popular demand, an option is added to throw exceptions when invalid properties are accessed on object maps (default is to return `()`). -Also based on popular demand, the `REPL` tool now uses -[`rustyline`](https://crates.io/crates/rustyline) for line editing and history. +Also based on popular demand, the `REPL` tool now uses a slightly-enhanced version of [`rustyline`](https://crates.io/crates/rustyline) for line editing and history. Bug fixes --------- diff --git a/Cargo.toml b/Cargo.toml index 7aa8be11..027d135f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = [".", "codegen"] [package] name = "rhai" -version = "1.5.0" +version = "1.6.0" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] description = "Embedded scripting for Rust" From 84face341acae0102c3a57a1fb6b454f252e48ea Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 15 Feb 2022 15:49:03 +0800 Subject: [PATCH 02/17] Fix doc comments. --- src/ast/flags.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ast/flags.rs b/src/ast/flags.rs index 15b664e2..1e0fdc63 100644 --- a/src/ast/flags.rs +++ b/src/ast/flags.rs @@ -13,8 +13,8 @@ pub enum FnAccess { Private, } -/// A type that holds a configuration option with bit-flags. Exported under the `internals` feature -/// only. +/// _(internals)_ A type that holds a configuration option with bit-flags. +/// Exported under the `internals` feature only. /// /// Functionality-wise, this type is a naive and simplistic implementation of /// [`bit_flags`](https://crates.io/crates/bitflags). It is re-implemented to avoid pulling in yet @@ -113,7 +113,8 @@ impl BitAndAssign for OptionFlags { } } -/// Option bit-flags for [`AST`][super::AST] nodes. +/// _(internals)_ Option bit-flags for [`AST`][super::AST] nodes. +/// Exported under the `internals` feature only. #[allow(non_snake_case)] pub mod AST_OPTION_FLAGS { use super::OptionFlags; From 3db918e89a860b700652fbef764e7057d0805897 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 15 Feb 2022 19:56:42 +0800 Subject: [PATCH 03/17] Doc fixup. --- README.md | 2 +- src/bin/README.md | 23 ++++++++++++++--------- src/bin/rhai-dbg.rs | 1 + 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 257072c6..f64d937b 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Standard features * Fairly efficient evaluation (1 million iterations in 0.3 sec on a single-core, 2.3 GHz Linux VM). * Tight integration with native Rust [functions](https://rhai.rs/book/rust/functions.html) and [types]([#custom-types-and-methods](https://rhai.rs/book/rust/custom.html)), including [getters/setters](https://rhai.rs/book/rust/getters-setters.html), [methods](https://rhai.rs/book/rust/custom.html) and [indexers](https://rhai.rs/book/rust/indexers.html). * Freely pass Rust values into a script as [variables](https://rhai.rs/book/language/variables.html)/[constants](https://rhai.rs/book/language/constants.html) via an external [`Scope`](https://rhai.rs/book/rust/scope.html) - all clonable Rust types are supported; no need to implement any special trait. Or tap directly into the [variable resolution process](https://rhai.rs/book/engine/var.html). -* Built-in support for most common [data types](https://rhai.rs/book/language/values-and-types.html) including booleans, [integers](https://rhai.rs/book/language/numbers.html), [floating-point numbers](https://rhai.rs/book/language/numbers.html) (including [`Decimal`](https://crates.io/crates/rust_decimal)), [strings](https://rhai.rs/book/language/strings-chars.html), [Unicode characters](https://rhai.rs/book/language/strings-chars.html), [arrays](https://rhai.rs/book/language/arrays.html) and [object maps](https://rhai.rs/book/language/object-maps.html). +* Built-in support for most common [data types](https://rhai.rs/book/language/values-and-types.html) including booleans, [integers](https://rhai.rs/book/language/numbers.html), [floating-point numbers](https://rhai.rs/book/language/numbers.html) (including [`Decimal`](https://crates.io/crates/rust_decimal)), [strings](https://rhai.rs/book/language/strings-chars.html), [Unicode characters](https://rhai.rs/book/language/strings-chars.html), [arrays](https://rhai.rs/book/language/arrays.html) (including packed [byte arrays](https://rhai.rs/book/language/blobs.html)) and [object maps](https://rhai.rs/book/language/object-maps.html). * Easily [call a script-defined function](https://rhai.rs/book/engine/call-fn.html) from Rust. * Relatively little `unsafe` code (yes there are some for performance reasons). * Few dependencies - currently only [`smallvec`](https://crates.io/crates/smallvec), [`num-traits`](https://crates.io/crates/num-traits), [`ahash`](https://crates.io/crates/ahash) and [`smartstring`](https://crates.io/crates/smartstring). diff --git a/src/bin/README.md b/src/bin/README.md index 32ba31ae..1480589c 100644 --- a/src/bin/README.md +++ b/src/bin/README.md @@ -1,16 +1,21 @@ Rhai Tools ========== -Tools for running Rhai scripts. +Tools for working with Rhai scripts. -| Tool | Required feature(s) | Description | -| -------------------------------------------------------------------------------- | :-----------------: | --------------------------------------------------- | -| [`rhai-run`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-run.rs) | | runs each filename passed to it as a Rhai script | -| [`rhai-repl`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-repl.rs) | `rustyline` | simple REPL that interactively evaluates statements | -| [`rhai-dbg`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-dbg.rs) | `debugging` | the _Rhai Debugger_ | +| Tool | Required feature(s) | Description | +| -------------------------------------------------------------------------------- | :-----------------: | ----------------------------------------------------- | +| [`rhai-run`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-run.rs) | | runs each filename passed to it as a Rhai script | +| [`rhai-repl`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-repl.rs) | `rustyline` | a simple REPL that interactively evaluates statements | +| [`rhai-dbg`](https://github.com/rhaiscript/rhai/blob/main/src/bin/rhai-dbg.rs) | `debugging` | the _Rhai Debugger_ | -There is a feature called `bin-features` which automatically includes all the necessary features -required for building these tools. +For convenience, a feature named `bin-features` is available which is a combination of the following: + +* `decimal` – support for decimal numbers +* `metadata` – access functions metadata +* `serde` – export functions metadata to JSON +* `debugging` – required by `rhai-dbg` +* `rustyline` – required by `rhai-repl` How to Run @@ -33,5 +38,5 @@ cargo install --path . --bins --features bin-features or specifically: ```sh -cargo install --path . --bin rhai-run --features bin-features +cargo install --path . --bin sample_app_to_run --features bin-features ``` diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index c4ee88ca..2fa211aa 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -635,6 +635,7 @@ fn main() { // Hook up debugger let lines: Vec<_> = script.trim().split('\n').map(|s| s.to_string()).collect(); + #[allow(deprecated)] engine.register_debugger( // Store the current source in the debugger state || "".into(), From 487d523e67ac05c57002969dd0a6849a1d261633 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 16 Feb 2022 09:12:34 +0800 Subject: [PATCH 04/17] Expand callback example. --- examples/callback.rs | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/examples/callback.rs b/examples/callback.rs index 40791bcd..ce27f77c 100644 --- a/examples/callback.rs +++ b/examples/callback.rs @@ -2,33 +2,41 @@ use rhai::{Engine, EvalAltResult, FnPtr}; +// To call a Rhai closure at a later time, you'd need three things: +// 1) an `Engine` (with all needed functions registered), +// 2) a compiled `AST`, +// 3) the closure (of type `FnPtr`). fn main() -> Result<(), Box> { - // This script creates a closure which may capture variables. - let script = " - let x = 20; - - // The following closure captures 'x' - return |a, b| (x + a) * b; - "; - - // To call a Rhai closure at a later time, you'd need three things: - // 1) an `Engine` (with all needed functions registered), - // 2) a compiled `AST`, - // 3) the closure (of type `FnPtr`). let engine = Engine::new(); - let ast = engine.compile(script)?; + // This script creates a closure which captures a variable and returns it. + let ast = engine.compile( + " + let x = 18; + + // The following closure captures 'x' + return |a, b| { + x += 1; // x is incremented each time + (x + a) * b + }; + ", + )?; let closure = engine.eval_ast::(&ast)?; - // Create a closure that we can call any time, encapsulating the - // `Engine`, `AST` and `FnPtr`. - let func = move |x: i64, y: i64| -> Result { closure.call(&engine, &ast, (x, y)) }; + // Create a closure by encapsulating the `Engine`, `AST` and `FnPtr`. + // In a real application, you'd be handling errors. + let func = move |x: i64, y: i64| -> i64 { closure.call(&engine, &ast, (x, y)).unwrap() }; // Now we can call `func` anywhere just like a normal function! - let result = func(1, 2)?; + let r1 = func(1, 2); - println!("The Answer: {}", result); // prints 42 + // Notice that each call to `func` returns a different value + // because the captured `x` is always changing! + let r2 = func(1, 2); + let r3 = func(1, 2); + + println!("The Answers: {}, {}, {}", r1, r2, r3); // prints 40, 42, 44 Ok(()) } From cf0660e36b5c41ac0e5014fca97ab575cef2168f Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 16 Feb 2022 12:57:26 +0800 Subject: [PATCH 05/17] Expand StmtBlock inline size. --- src/ast/ast.rs | 6 +++--- src/ast/mod.rs | 5 ++++- src/ast/stmt.rs | 16 +++++++++------- src/optimizer.rs | 15 +++++++-------- src/parser.rs | 9 +++++---- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/ast/ast.rs b/src/ast/ast.rs index 06fe65ec..dcf43977 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -1,7 +1,7 @@ //! Module defining the AST (abstract syntax tree). -use super::{Expr, FnAccess, Stmt, StmtBlock, AST_OPTION_FLAGS::*}; -use crate::{Dynamic, FnNamespace, Identifier, Position, StaticVec}; +use super::{Expr, FnAccess, Stmt, StmtBlock, StmtBlockContainer, AST_OPTION_FLAGS::*}; +use crate::{Dynamic, FnNamespace, Identifier, Position}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -197,7 +197,7 @@ impl AST { #[allow(dead_code)] #[inline(always)] #[must_use] - pub(crate) fn take_statements(&mut self) -> StaticVec { + pub(crate) fn take_statements(&mut self) -> StmtBlockContainer { self.body.take_statements() } /// Does this [`AST`] contain script-defined functions? diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 009b1199..3bfe2389 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -16,7 +16,10 @@ pub use ident::Ident; pub use script_fn::EncapsulatedEnviron; #[cfg(not(feature = "no_function"))] pub use script_fn::{ScriptFnDef, ScriptFnMetadata}; -pub use stmt::{ConditionalStmtBlock, OpAssignment, Stmt, StmtBlock, SwitchCases, TryCatchBlock}; +pub use stmt::{ + ConditionalStmtBlock, OpAssignment, Stmt, StmtBlock, StmtBlockContainer, SwitchCases, + TryCatchBlock, +}; #[cfg(not(feature = "no_float"))] pub use expr::FloatWrapper; diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 27962878..08ab0f21 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -131,10 +131,12 @@ pub struct TryCatchBlock { pub catch_block: StmtBlock, } +pub type StmtBlockContainer = smallvec::SmallVec<[Stmt; 8]>; + /// _(internals)_ A scoped block of statements. /// Exported under the `internals` feature only. #[derive(Clone, Hash, Default)] -pub struct StmtBlock(StaticVec, Span); +pub struct StmtBlock(StmtBlockContainer, Span); impl StmtBlock { /// A [`StmtBlock`] that does not exist. @@ -147,7 +149,7 @@ impl StmtBlock { start_pos: Position, end_pos: Position, ) -> Self { - let mut statements: StaticVec<_> = statements.into_iter().collect(); + let mut statements: smallvec::SmallVec<_> = statements.into_iter().collect(); statements.shrink_to_fit(); Self(statements, Span::new(start_pos, end_pos)) } @@ -155,7 +157,7 @@ impl StmtBlock { #[inline(always)] #[must_use] pub const fn empty(pos: Position) -> Self { - Self(StaticVec::new_const(), Span::new(pos, pos)) + Self(smallvec::SmallVec::new_const(), Span::new(pos, pos)) } /// Is this statements block empty? #[inline(always)] @@ -178,7 +180,7 @@ impl StmtBlock { /// Extract the statements. #[inline(always)] #[must_use] - pub(crate) fn take_statements(&mut self) -> StaticVec { + pub(crate) fn take_statements(&mut self) -> StmtBlockContainer { mem::take(&mut self.0) } /// Get an iterator over the statements of this statements block. @@ -223,7 +225,7 @@ impl StmtBlock { } impl Deref for StmtBlock { - type Target = StaticVec; + type Target = StmtBlockContainer; #[inline(always)] fn deref(&self) -> &Self::Target { @@ -268,7 +270,7 @@ impl From for StmtBlock { fn from(stmt: Stmt) -> Self { match stmt { Stmt::Block(mut block, span) => Self(block.iter_mut().map(mem::take).collect(), span), - Stmt::Noop(pos) => Self(StaticVec::new_const(), Span::new(pos, pos)), + Stmt::Noop(pos) => Self(smallvec::SmallVec::new_const(), Span::new(pos, pos)), _ => { let pos = stmt.position(); Self(vec![stmt].into(), Span::new(pos, Position::NONE)) @@ -279,7 +281,7 @@ impl From for StmtBlock { impl IntoIterator for StmtBlock { type Item = Stmt; - type IntoIter = smallvec::IntoIter<[Stmt; 3]>; + type IntoIter = smallvec::IntoIter<[Stmt; 8]>; #[inline(always)] fn into_iter(self) -> Self::IntoIter { diff --git a/src/optimizer.rs b/src/optimizer.rs index 220fae23..e60a9922 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1,7 +1,7 @@ //! Module implementing the [`AST`] optimizer. #![cfg(not(feature = "no_optimize"))] -use crate::ast::{Expr, OpAssignment, Stmt, AST_OPTION_FLAGS::*}; +use crate::ast::{Expr, OpAssignment, Stmt, StmtBlockContainer, AST_OPTION_FLAGS::*}; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::eval::{EvalState, GlobalRuntimeState}; use crate::func::builtin::get_builtin_binary_op_fn; @@ -184,12 +184,12 @@ fn has_native_fn_override( /// Optimize a block of [statements][Stmt]. fn optimize_stmt_block( - mut statements: StaticVec, + mut statements: StmtBlockContainer, state: &mut OptimizerState, preserve_result: bool, is_internal: bool, reduce_return: bool, -) -> StaticVec { +) -> StmtBlockContainer { if statements.is_empty() { return statements; } @@ -1155,12 +1155,12 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, chaining: bool) { /// /// Constants and variables from the scope are added. fn optimize_top_level( - statements: StaticVec, + statements: StmtBlockContainer, engine: &Engine, scope: &Scope, #[cfg(not(feature = "no_function"))] lib: &[&crate::Module], optimization_level: OptimizationLevel, -) -> StaticVec { +) -> StmtBlockContainer { let mut statements = statements; // If optimization level is None then skip optimizing @@ -1186,15 +1186,14 @@ fn optimize_top_level( } } - statements = optimize_stmt_block(statements, &mut state, true, false, true); - statements + optimize_stmt_block(statements, &mut state, true, false, true) } /// Optimize an [`AST`]. pub fn optimize_into_ast( engine: &Engine, scope: &Scope, - statements: StaticVec, + statements: StmtBlockContainer, #[cfg(not(feature = "no_function"))] functions: StaticVec< crate::Shared, >, diff --git a/src/parser.rs b/src/parser.rs index 74f97c2a..e43c0fd1 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -5,7 +5,8 @@ use crate::api::events::VarDefInfo; use crate::api::options::LanguageOptions; use crate::ast::{ BinaryExpr, ConditionalStmtBlock, CustomExpr, Expr, FnCallExpr, FnCallHashes, Ident, - OpAssignment, ScriptFnDef, Stmt, SwitchCases, TryCatchBlock, AST_OPTION_FLAGS::*, + OpAssignment, ScriptFnDef, Stmt, StmtBlockContainer, SwitchCases, TryCatchBlock, + AST_OPTION_FLAGS::*, }; use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS}; use crate::eval::{EvalState, GlobalRuntimeState}; @@ -3481,7 +3482,7 @@ impl Engine { } } - let mut statements = StaticVec::new_const(); + let mut statements = smallvec::SmallVec::new_const(); statements.push(Stmt::Expr(expr)); #[cfg(not(feature = "no_optimize"))] @@ -3507,8 +3508,8 @@ impl Engine { &self, input: &mut TokenStream, state: &mut ParseState, - ) -> ParseResult<(StaticVec, StaticVec>)> { - let mut statements = StaticVec::new_const(); + ) -> ParseResult<(StmtBlockContainer, StaticVec>)> { + let mut statements = smallvec::SmallVec::new_const(); let mut functions = BTreeMap::new(); while !input.peek().expect(NEVER_ENDS).0.is_eof() { From 0d2e3d82f37baf0d539dd2620593cc85dde94c25 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 16 Feb 2022 17:51:14 +0800 Subject: [PATCH 06/17] Reduce size of Stmt. --- src/api/compile.rs | 15 +- src/ast/ast.rs | 3 +- src/ast/stmt.rs | 269 ++++++++++++++++++++--------------- src/eval/debugger.rs | 27 ++-- src/eval/stmt.rs | 68 +++++---- src/optimizer.rs | 331 +++++++++++++++++++++++++------------------ src/parser.rs | 80 +++++------ src/tests.rs | 4 +- tests/optimizer.rs | 2 +- 9 files changed, 457 insertions(+), 342 deletions(-) diff --git a/src/api/compile.rs b/src/api/compile.rs index 39595a8d..6d491da6 100644 --- a/src/api/compile.rs +++ b/src/api/compile.rs @@ -99,12 +99,15 @@ impl Engine { ) { ast.walk(&mut |path| match path.last().unwrap() { // Collect all `import` statements with a string constant path - ASTNode::Stmt(Stmt::Import(Expr::StringConstant(s, ..), ..)) - if !resolver.contains_path(s) && !imports.contains(s.as_str()) => - { - imports.insert(s.clone().into()); - true - } + ASTNode::Stmt(Stmt::Import(x, ..)) => match x.0 { + Expr::StringConstant(ref s, ..) + if !resolver.contains_path(s) && !imports.contains(s.as_str()) => + { + imports.insert(s.clone().into()); + true + } + _ => true, + }, _ => true, }); } diff --git a/src/ast/ast.rs b/src/ast/ast.rs index dcf43977..b7603111 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -744,10 +744,11 @@ impl AST { include_variables: bool, ) -> impl Iterator { self.statements().iter().filter_map(move |stmt| match stmt { - Stmt::Var(expr, name, options, ..) + Stmt::Var(x, options, ..) if options.contains(AST_OPTION_CONSTANT) && include_constants || !options.contains(AST_OPTION_CONSTANT) && include_variables => { + let (name, expr) = x.as_ref(); if let Some(value) = expr.get_literal_value() { Some((name.as_str(), options.contains(AST_OPTION_CONSTANT), value)) } else { diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 08ab0f21..26c880fe 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -114,9 +114,9 @@ pub struct SwitchCases { /// Dictionary mapping value hashes to [`ConditionalStmtBlock`]'s. pub cases: BTreeMap>, /// Statements block for the default case (there can be no condition for the default case). - pub def_case: StmtBlock, + pub def_case: Box, /// List of range cases. - pub ranges: StaticVec<(INT, INT, bool, ConditionalStmtBlock)>, + pub ranges: StaticVec<(INT, INT, bool, Box)>, } /// _(internals)_ A `try-catch` block. @@ -131,8 +131,20 @@ pub struct TryCatchBlock { pub catch_block: StmtBlock, } +/// _(internals)_ The underlying container type for [`StmtBlock`]. +/// Exported under the `internals` feature only. +/// +/// A [`SmallVec`](https://crates.io/crates/smallvec) containing up to 8 items inline is used to +/// hold a statements block, with the assumption that most program blocks would container fewer than +/// 8 statements, and those that do have a lot more statements. +#[cfg(not(feature = "no_std"))] pub type StmtBlockContainer = smallvec::SmallVec<[Stmt; 8]>; +/// _(internals)_ The underlying container type for [`StmtBlock`]. +/// Exported under the `internals` feature only. +#[cfg(feature = "no_std")] +pub type StmtBlockContainer = StaticVec; + /// _(internals)_ A scoped block of statements. /// Exported under the `internals` feature only. #[derive(Clone, Hash, Default)] @@ -143,21 +155,27 @@ impl StmtBlock { pub const NONE: Self = Self::empty(Position::NONE); /// Create a new [`StmtBlock`]. + #[inline(always)] #[must_use] pub fn new( statements: impl IntoIterator, start_pos: Position, end_pos: Position, ) -> Self { + Self::new_with_span(statements, Span::new(start_pos, end_pos)) + } + /// Create a new [`StmtBlock`]. + #[must_use] + pub fn new_with_span(statements: impl IntoIterator, span: Span) -> Self { let mut statements: smallvec::SmallVec<_> = statements.into_iter().collect(); statements.shrink_to_fit(); - Self(statements, Span::new(start_pos, end_pos)) + Self(statements, span) } /// Create an empty [`StmtBlock`]. #[inline(always)] #[must_use] pub const fn empty(pos: Position) -> Self { - Self(smallvec::SmallVec::new_const(), Span::new(pos, pos)) + Self(StmtBlockContainer::new_const(), Span::new(pos, pos)) } /// Is this statements block empty? #[inline(always)] @@ -269,8 +287,8 @@ impl From for StmtBlock { #[inline] fn from(stmt: Stmt) -> Self { match stmt { - Stmt::Block(mut block, span) => Self(block.iter_mut().map(mem::take).collect(), span), - Stmt::Noop(pos) => Self(smallvec::SmallVec::new_const(), Span::new(pos, pos)), + Stmt::Block(block) => *block, + Stmt::Noop(pos) => Self(StmtBlockContainer::new_const(), Span::new(pos, pos)), _ => { let pos = stmt.position(); Self(vec![stmt].into(), Span::new(pos, Position::NONE)) @@ -303,7 +321,7 @@ pub enum Stmt { /// No-op. Noop(Position), /// `if` expr `{` stmt `}` `else` `{` stmt `}` - If(Expr, Box<(StmtBlock, StmtBlock)>, Position), + If(Box<(Expr, StmtBlock, StmtBlock)>, Position), /// `switch` expr `{` literal or range or _ `if` condition `=>` stmt `,` ... `}` /// /// ### Data Structure @@ -311,27 +329,27 @@ pub enum Stmt { /// 0) Hash table for (condition, block) /// 1) Default block /// 2) List of ranges: (start, end, inclusive, condition, statement) - Switch(Expr, Box, Position), + Switch(Box<(Expr, SwitchCases)>, Position), /// `while` expr `{` stmt `}` | `loop` `{` stmt `}` /// /// If the guard expression is [`UNIT`][Expr::Unit], then it is a `loop` statement. - While(Expr, Box, Position), + While(Box<(Expr, StmtBlock)>, Position), /// `do` `{` stmt `}` `while`|`until` expr /// /// ### Option Flags /// /// * [`AST_OPTION_NONE`] = `while` /// * [`AST_OPTION_NEGATED`] = `until` - Do(Box, Expr, OptionFlags, Position), + Do(Box<(Expr, StmtBlock)>, OptionFlags, Position), /// `for` `(` id `,` counter `)` `in` expr `{` stmt `}` - For(Expr, Box<(Ident, Option, StmtBlock)>, Position), + For(Box<(Ident, Expr, Option, StmtBlock)>, Position), /// \[`export`\] `let`|`const` id `=` expr /// /// ### Option Flags /// /// * [`AST_OPTION_EXPORTED`] = `export` /// * [`AST_OPTION_CONSTANT`] = `const` - Var(Expr, Box, OptionFlags, Position), + Var(Box<(Ident, Expr)>, OptionFlags, Position), /// expr op`=` expr Assignment(Box<(Option>, BinaryExpr)>, Position), /// func `(` expr `,` ... `)` @@ -340,11 +358,11 @@ pub enum Stmt { /// function call forming one statement. FnCall(Box, Position), /// `{` stmt`;` ... `}` - Block(Box<[Stmt]>, Span), + Block(Box), /// `try` `{` stmt; ... `}` `catch` `(` var `)` `{` stmt; ... `}` TryCatch(Box, Position), /// [expression][Expr] - Expr(Expr), + Expr(Box), /// `continue`/`break` /// /// ### Option Flags @@ -358,12 +376,12 @@ pub enum Stmt { /// /// * [`AST_OPTION_NONE`] = `return` /// * [`AST_OPTION_BREAK`] = `throw` - Return(OptionFlags, Option, Position), + Return(Option>, OptionFlags, Position), /// `import` expr `as` var /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] - Import(Expr, Option>, Position), + Import(Box<(Expr, Option)>, Position), /// `export` var `as` var /// /// Not available under `no_module`. @@ -378,7 +396,7 @@ pub enum Stmt { /// This variant does not map to any language structure. It is currently only used only to /// convert a normal variable into a shared variable when the variable is _captured_ by a closure. #[cfg(not(feature = "no_closure"))] - Share(crate::Identifier, Position), + Share(Box, Position), } impl Default for Stmt { @@ -391,7 +409,21 @@ impl Default for Stmt { impl From for Stmt { #[inline(always)] fn from(block: StmtBlock) -> Self { - Self::Block(block.0.into_boxed_slice(), block.1) + Self::Block(block.into()) + } +} + +impl> From<(T, Position, Position)> for Stmt { + #[inline(always)] + fn from(value: (T, Position, Position)) -> Self { + StmtBlock::new(value.0, value.1, value.2).into() + } +} + +impl> From<(T, Span)> for Stmt { + #[inline(always)] + fn from(value: (T, Span)) -> Self { + StmtBlock::new_with_span(value.0, value.1).into() } } @@ -419,7 +451,7 @@ impl Stmt { | Self::Var(.., pos) | Self::TryCatch(.., pos) => *pos, - Self::Block(.., span) => span.start(), + Self::Block(x) => x.position(), Self::Expr(x) => x.start_position(), @@ -448,7 +480,7 @@ impl Stmt { | Self::Var(.., pos) | Self::TryCatch(.., pos) => *pos = new_pos, - Self::Block(.., span) => *span = Span::new(new_pos, span.end()), + Self::Block(x) => x.set_position(new_pos, x.end_position()), Self::Expr(x) => { x.set_position(new_pos); @@ -504,11 +536,13 @@ impl Stmt { // A No-op requires a semicolon in order to know it is an empty statement! Self::Noop(..) => false, - Self::Expr(Expr::Custom(x, ..)) if x.is_self_terminated() => true, + Self::Expr(e) => match &**e { + Expr::Custom(x, ..) if x.is_self_terminated() => true, + _ => false, + }, Self::Var(..) | Self::Assignment(..) - | Self::Expr(..) | Self::FnCall(..) | Self::Do(..) | Self::BreakLoop(..) @@ -529,38 +563,37 @@ impl Stmt { match self { Self::Noop(..) => true, Self::Expr(expr) => expr.is_pure(), - Self::If(condition, x, ..) => { - condition.is_pure() - && x.0.iter().all(Stmt::is_pure) - && x.1.iter().all(Stmt::is_pure) + Self::If(x, ..) => { + x.0.is_pure() && x.1.iter().all(Stmt::is_pure) && x.2.iter().all(Stmt::is_pure) } - Self::Switch(expr, x, ..) => { - expr.is_pure() - && x.cases.values().all(|block| { + Self::Switch(x, ..) => { + x.0.is_pure() + && x.1.cases.values().all(|block| { block.condition.as_ref().map(Expr::is_pure).unwrap_or(true) && block.statements.iter().all(Stmt::is_pure) }) - && x.ranges.iter().all(|(.., block)| { + && x.1.ranges.iter().all(|(.., block)| { block.condition.as_ref().map(Expr::is_pure).unwrap_or(true) && block.statements.iter().all(Stmt::is_pure) }) - && x.def_case.iter().all(Stmt::is_pure) + && x.1.def_case.iter().all(Stmt::is_pure) } // Loops that exit can be pure because it can never be infinite. - Self::While(Expr::BoolConstant(false, ..), ..) => true, - Self::Do(body, Expr::BoolConstant(x, ..), options, ..) - if *x == options.contains(AST_OPTION_NEGATED) => - { - body.iter().all(Stmt::is_pure) - } + Self::While(x, ..) if matches!(x.0, Expr::BoolConstant(false, ..)) => true, + Self::Do(x, options, ..) if matches!(x.0, Expr::BoolConstant(..)) => match x.0 { + Expr::BoolConstant(cond, ..) if cond == options.contains(AST_OPTION_NEGATED) => { + x.1.iter().all(Stmt::is_pure) + } + _ => false, + }, // Loops are never pure since they can be infinite - and that's a side effect. Self::While(..) | Self::Do(..) => false, // For loops can be pure because if the iterable is pure, it is finite, // so infinite loops can never occur. - Self::For(iterable, x, ..) => iterable.is_pure() && x.2.iter().all(Stmt::is_pure), + Self::For(x, ..) => x.1.is_pure() && x.3.iter().all(Stmt::is_pure), Self::Var(..) | Self::Assignment(..) | Self::FnCall(..) => false, Self::Block(block, ..) => block.iter().all(|stmt| stmt.is_pure()), @@ -591,11 +624,13 @@ impl Stmt { match self { Self::Var(..) => true, - Self::Expr(Expr::Stmt(s)) => s.iter().all(Stmt::is_block_dependent), + Self::Expr(e) => match &**e { + Expr::Stmt(s) => s.iter().all(Stmt::is_block_dependent), + Expr::FnCall(x, ..) => !x.is_qualified() && x.name == KEYWORD_EVAL, + _ => false, + }, - Self::FnCall(x, ..) | Self::Expr(Expr::FnCall(x, ..)) => { - !x.is_qualified() && x.name == KEYWORD_EVAL - } + Self::FnCall(x, ..) => !x.is_qualified() && x.name == KEYWORD_EVAL, #[cfg(not(feature = "no_module"))] Self::Import(..) | Self::Export(..) => true, @@ -613,12 +648,15 @@ impl Stmt { #[must_use] pub fn is_internally_pure(&self) -> bool { match self { - Self::Var(expr, _, ..) => expr.is_pure(), + Self::Var(x, ..) => x.1.is_pure(), - Self::Expr(Expr::Stmt(s)) => s.iter().all(Stmt::is_internally_pure), + Self::Expr(e) => match e.as_ref() { + Expr::Stmt(s) => s.iter().all(Stmt::is_internally_pure), + _ => self.is_pure(), + }, #[cfg(not(feature = "no_module"))] - Self::Import(expr, ..) => expr.is_pure(), + Self::Import(x, ..) => x.0.is_pure(), #[cfg(not(feature = "no_module"))] Self::Export(..) => true, @@ -653,86 +691,86 @@ impl Stmt { } match self { - Self::Var(e, _, ..) => { - if !e.walk(path, on_node) { + Self::Var(x, ..) => { + if !x.1.walk(path, on_node) { return false; } } - Self::If(e, x, ..) => { - if !e.walk(path, on_node) { + Self::If(x, ..) => { + if !x.0.walk(path, on_node) { return false; } - for s in x.0.iter() { - if !s.walk(path, on_node) { - return false; - } - } for s in x.1.iter() { if !s.walk(path, on_node) { return false; } } - } - Self::Switch(e, x, ..) => { - if !e.walk(path, on_node) { - return false; - } - for b in x.cases.values() { - if !b - .condition - .as_ref() - .map(|e| e.walk(path, on_node)) - .unwrap_or(true) - { - return false; - } - for s in b.statements.iter() { - if !s.walk(path, on_node) { - return false; - } - } - } - for (.., b) in &x.ranges { - if !b - .condition - .as_ref() - .map(|e| e.walk(path, on_node)) - .unwrap_or(true) - { - return false; - } - for s in b.statements.iter() { - if !s.walk(path, on_node) { - return false; - } - } - } - for s in x.def_case.iter() { - if !s.walk(path, on_node) { - return false; - } - } - } - Self::While(e, s, ..) | Self::Do(s, e, ..) => { - if !e.walk(path, on_node) { - return false; - } - for s in &s.0 { - if !s.walk(path, on_node) { - return false; - } - } - } - Self::For(e, x, ..) => { - if !e.walk(path, on_node) { - return false; - } for s in x.2.iter() { if !s.walk(path, on_node) { return false; } } } + Self::Switch(x, ..) => { + if !x.0.walk(path, on_node) { + return false; + } + for b in x.1.cases.values() { + if !b + .condition + .as_ref() + .map(|e| e.walk(path, on_node)) + .unwrap_or(true) + { + return false; + } + for s in b.statements.iter() { + if !s.walk(path, on_node) { + return false; + } + } + } + for (.., b) in &x.1.ranges { + if !b + .condition + .as_ref() + .map(|e| e.walk(path, on_node)) + .unwrap_or(true) + { + return false; + } + for s in b.statements.iter() { + if !s.walk(path, on_node) { + return false; + } + } + } + for s in x.1.def_case.iter() { + if !s.walk(path, on_node) { + return false; + } + } + } + Self::While(x, ..) | Self::Do(x, ..) => { + if !x.0.walk(path, on_node) { + return false; + } + for s in x.1.statements() { + if !s.walk(path, on_node) { + return false; + } + } + } + Self::For(x, ..) => { + if !x.1.walk(path, on_node) { + return false; + } + for s in x.3.iter() { + if !s.walk(path, on_node) { + return false; + } + } + } Self::Assignment(x, ..) => { if !x.1.lhs.walk(path, on_node) { return false; @@ -749,7 +787,7 @@ impl Stmt { } } Self::Block(x, ..) => { - for s in x.iter() { + for s in x.statements() { if !s.walk(path, on_node) { return false; } @@ -767,14 +805,19 @@ impl Stmt { } } } - Self::Expr(e) | Self::Return(_, Some(e), _) => { + Self::Expr(e) => { + if !e.walk(path, on_node) { + return false; + } + } + Self::Return(Some(e), ..) => { if !e.walk(path, on_node) { return false; } } #[cfg(not(feature = "no_module"))] - Self::Import(e, ..) => { - if !e.walk(path, on_node) { + Self::Import(x, ..) => { + if !x.0.walk(path, on_node) { return false; } } diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index d6f10ca0..5fc04e47 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -361,17 +361,23 @@ impl Debugger { node.position() == *pos && _src == source } BreakPoint::AtFunctionName { name, .. } => match node { - ASTNode::Expr(Expr::FnCall(x, ..)) - | ASTNode::Stmt(Stmt::FnCall(x, ..)) - | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(x, ..))) => x.name == *name, + ASTNode::Expr(Expr::FnCall(x, ..)) | ASTNode::Stmt(Stmt::FnCall(x, ..)) => { + x.name == *name + } + ASTNode::Stmt(Stmt::Expr(e)) => match e.as_ref() { + Expr::FnCall(x, ..) => x.name == *name, + _ => false, + }, _ => false, }, BreakPoint::AtFunctionCall { name, args, .. } => match node { - ASTNode::Expr(Expr::FnCall(x, ..)) - | ASTNode::Stmt(Stmt::FnCall(x, ..)) - | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(x, ..))) => { + ASTNode::Expr(Expr::FnCall(x, ..)) | ASTNode::Stmt(Stmt::FnCall(x, ..)) => { x.args.len() == *args && x.name == *name } + ASTNode::Stmt(Stmt::Expr(e)) => match e.as_ref() { + Expr::FnCall(x, ..) => x.args.len() == *args && x.name == *name, + _ => false, + }, _ => false, }, #[cfg(not(feature = "no_object"))] @@ -548,9 +554,12 @@ impl Engine { DebuggerCommand::FunctionExit => { // Bump a level if it is a function call let level = match node { - ASTNode::Expr(Expr::FnCall(..)) - | ASTNode::Stmt(Stmt::FnCall(..)) - | ASTNode::Stmt(Stmt::Expr(Expr::FnCall(..))) => context.call_level() + 1, + ASTNode::Expr(Expr::FnCall(..)) | ASTNode::Stmt(Stmt::FnCall(..)) => { + context.call_level() + 1 + } + ASTNode::Stmt(Stmt::Expr(e)) if matches!(e.as_ref(), Expr::FnCall(..)) => { + context.call_level() + 1 + } _ => context.call_level(), }; global.debugger.status = DebuggerStatus::FunctionExit(level); diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 294705ac..8d7d7358 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -338,7 +338,9 @@ impl Engine { } // If statement - Stmt::If(expr, x, ..) => { + Stmt::If(x, ..) => { + let (expr, if_block, else_block) = x.as_ref(); + let guard_val = self .eval_expr(scope, global, state, lib, this_ptr, expr, level) .and_then(|v| { @@ -349,18 +351,18 @@ impl Engine { match guard_val { Ok(true) => { - if !x.0.is_empty() { + if !if_block.is_empty() { self.eval_stmt_block( - scope, global, state, lib, this_ptr, &x.0, true, level, + scope, global, state, lib, this_ptr, if_block, true, level, ) } else { Ok(Dynamic::UNIT) } } Ok(false) => { - if !x.1.is_empty() { + if !else_block.is_empty() { self.eval_stmt_block( - scope, global, state, lib, this_ptr, &x.1, true, level, + scope, global, state, lib, this_ptr, else_block, true, level, ) } else { Ok(Dynamic::UNIT) @@ -371,15 +373,17 @@ impl Engine { } // Switch statement - Stmt::Switch(match_expr, x, ..) => { - let SwitchCases { - cases, - def_case, - ranges, - } = x.as_ref(); + Stmt::Switch(x, ..) => { + let ( + expr, + SwitchCases { + cases, + def_case, + ranges, + }, + ) = x.as_ref(); - let value_result = - self.eval_expr(scope, global, state, lib, this_ptr, match_expr, level); + let value_result = self.eval_expr(scope, global, state, lib, this_ptr, expr, level); if let Ok(value) = value_result { let stmt_block_result = if value.is_hashable() { @@ -484,7 +488,9 @@ impl Engine { } // Loop - Stmt::While(Expr::Unit(..), body, ..) => loop { + Stmt::While(x, ..) if matches!(x.0, Expr::Unit(..)) => loop { + let (.., body) = x.as_ref(); + if !body.is_empty() { match self .eval_stmt_block(scope, global, state, lib, this_ptr, body, true, level) @@ -503,7 +509,9 @@ impl Engine { }, // While loop - Stmt::While(expr, body, ..) => loop { + Stmt::While(x, ..) => loop { + let (expr, body) = x.as_ref(); + let condition = self .eval_expr(scope, global, state, lib, this_ptr, expr, level) .and_then(|v| { @@ -532,7 +540,8 @@ impl Engine { }, // Do loop - Stmt::Do(body, expr, options, ..) => loop { + Stmt::Do(x, options, ..) => loop { + let (expr, body) = x.as_ref(); let is_while = !options.contains(AST_OPTION_NEGATED); if !body.is_empty() { @@ -549,7 +558,7 @@ impl Engine { } let condition = self - .eval_expr(scope, global, state, lib, this_ptr, expr, level) + .eval_expr(scope, global, state, lib, this_ptr, &expr, level) .and_then(|v| { v.as_bool().map_err(|typ| { self.make_type_mismatch_err::(typ, expr.position()) @@ -564,8 +573,8 @@ impl Engine { }, // For loop - Stmt::For(expr, x, ..) => { - let (Ident { name: var_name, .. }, counter, statements) = x.as_ref(); + Stmt::For(x, ..) => { + let (Ident { name: var_name, .. }, expr, counter, statements) = x.as_ref(); let iter_result = self .eval_expr(scope, global, state, lib, this_ptr, expr, level) @@ -786,30 +795,31 @@ impl Engine { } // Throw value - Stmt::Return(options, Some(expr), pos) if options.contains(AST_OPTION_BREAK) => self + Stmt::Return(Some(expr), options, pos) if options.contains(AST_OPTION_BREAK) => self .eval_expr(scope, global, state, lib, this_ptr, expr, level) .and_then(|v| Err(ERR::ErrorRuntime(v.flatten(), *pos).into())), // Empty throw - Stmt::Return(options, None, pos) if options.contains(AST_OPTION_BREAK) => { + Stmt::Return(None, options, pos) if options.contains(AST_OPTION_BREAK) => { Err(ERR::ErrorRuntime(Dynamic::UNIT, *pos).into()) } // Return value - Stmt::Return(.., Some(expr), pos) => self + Stmt::Return(Some(expr), .., pos) => self .eval_expr(scope, global, state, lib, this_ptr, expr, level) .and_then(|v| Err(ERR::Return(v.flatten(), *pos).into())), // Empty return - Stmt::Return(.., None, pos) => Err(ERR::Return(Dynamic::UNIT, *pos).into()), + Stmt::Return(None, .., pos) => Err(ERR::Return(Dynamic::UNIT, *pos).into()), // Let/const statement - shadowing disallowed - Stmt::Var(.., x, _, pos) if !self.allow_shadowing() && scope.contains(&x.name) => { - Err(ERR::ErrorVariableExists(x.name.to_string(), *pos).into()) + Stmt::Var(x, .., pos) if !self.allow_shadowing() && scope.contains(&x.0.name) => { + Err(ERR::ErrorVariableExists(x.0.name.to_string(), *pos).into()) } // Let/const statement - Stmt::Var(expr, x, options, pos) => { - let var_name = &x.name; + Stmt::Var(x, options, pos) => { + let var_name = &x.0.name; + let expr = &x.1; let entry_type = if options.contains(AST_OPTION_CONSTANT) { AccessMode::ReadOnly @@ -902,7 +912,9 @@ impl Engine { // Import statement #[cfg(not(feature = "no_module"))] - Stmt::Import(expr, export, _pos) => { + Stmt::Import(x, _pos) => { + let (expr, export) = x.as_ref(); + // Guard against too many modules #[cfg(not(feature = "unchecked"))] if global.num_modules_loaded >= self.max_modules() { diff --git a/src/optimizer.rs b/src/optimizer.rs index e60a9922..daad6b3d 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -1,7 +1,9 @@ //! Module implementing the [`AST`] optimizer. #![cfg(not(feature = "no_optimize"))] -use crate::ast::{Expr, OpAssignment, Stmt, StmtBlockContainer, AST_OPTION_FLAGS::*}; +use crate::ast::{ + Expr, OpAssignment, Stmt, StmtBlock, StmtBlockContainer, SwitchCases, AST_OPTION_FLAGS::*, +}; use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::eval::{EvalState, GlobalRuntimeState}; use crate::func::builtin::get_builtin_binary_op_fn; @@ -252,22 +254,22 @@ fn optimize_stmt_block( // Optimize each statement in the block for stmt in statements.iter_mut() { match stmt { - Stmt::Var(value_expr, x, options, ..) => { + Stmt::Var(x, options, ..) => { if options.contains(AST_OPTION_CONSTANT) { // Add constant literals into the state - optimize_expr(value_expr, state, false); + optimize_expr(&mut x.1, state, false); - if value_expr.is_constant() { + if x.1.is_constant() { state.push_var( - x.name.as_str(), + x.0.name.as_str(), AccessMode::ReadOnly, - value_expr.get_literal_value(), + x.1.get_literal_value(), ); } } else { // Add variables into the state - optimize_expr(value_expr, state, false); - state.push_var(x.name.as_str(), AccessMode::ReadWrite, None); + optimize_expr(&mut x.1, state, false); + state.push_var(x.0.name.as_str(), AccessMode::ReadWrite, None); } } // Optimize the statement @@ -284,10 +286,11 @@ fn optimize_stmt_block( .find_map(|(i, stmt)| match stmt { stmt if !is_pure(stmt) => Some(i), - Stmt::Var(e, _, ..) | Stmt::Expr(e) if !e.is_constant() => Some(i), + Stmt::Var(x, ..) if x.1.is_constant() => Some(i), + Stmt::Expr(e) if !e.is_constant() => Some(i), #[cfg(not(feature = "no_module"))] - Stmt::Import(e, ..) if !e.is_constant() => Some(i), + Stmt::Import(x, ..) if !x.0.is_constant() => Some(i), _ => None, }) @@ -320,7 +323,7 @@ fn optimize_stmt_block( loop { match statements[..] { // { return; } -> {} - [Stmt::Return(options, None, ..)] + [Stmt::Return(None, options, ..)] if reduce_return && !options.contains(AST_OPTION_BREAK) => { state.set_dirty(); @@ -331,7 +334,7 @@ fn optimize_stmt_block( statements.clear(); } // { ...; return; } -> { ... } - [.., ref last_stmt, Stmt::Return(options, None, ..)] + [.., ref last_stmt, Stmt::Return(None, options, ..)] if reduce_return && !options.contains(AST_OPTION_BREAK) && !last_stmt.returns_value() => @@ -340,7 +343,7 @@ fn optimize_stmt_block( statements.pop().unwrap(); } // { ...; return val; } -> { ...; val } - [.., Stmt::Return(options, ref mut expr, pos)] + [.., Stmt::Return(ref mut expr, options, pos)] if reduce_return && !options.contains(AST_OPTION_BREAK) => { state.set_dirty(); @@ -377,14 +380,14 @@ fn optimize_stmt_block( statements.clear(); } // { ...; return; } -> { ... } - [.., Stmt::Return(options, None, ..)] + [.., Stmt::Return(None, options, ..)] if reduce_return && !options.contains(AST_OPTION_BREAK) => { state.set_dirty(); statements.pop().unwrap(); } // { ...; return pure_val; } -> { ... } - [.., Stmt::Return(options, Some(ref expr), ..)] + [.., Stmt::Return(Some(ref expr), options, ..)] if reduce_return && !options.contains(AST_OPTION_BREAK) && expr.is_pure() => @@ -460,7 +463,8 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } // if expr {} - Stmt::If(condition, x, ..) if x.0.is_empty() && x.1.is_empty() => { + Stmt::If(x, ..) if x.1.is_empty() && x.2.is_empty() => { + let condition = &mut x.0; state.set_dirty(); let pos = condition.start_position(); @@ -469,56 +473,72 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b *stmt = if preserve_result { // -> { expr, Noop } - Stmt::Block( - [Stmt::Expr(expr), Stmt::Noop(pos)].into(), - Span::new(pos, Position::NONE), + ( + [Stmt::Expr(expr.into()), Stmt::Noop(pos)], + pos, + Position::NONE, ) + .into() } else { // -> expr - Stmt::Expr(expr) + Stmt::Expr(expr.into()) }; } // if false { if_block } -> Noop - Stmt::If(Expr::BoolConstant(false, pos), x, ..) if x.1.is_empty() => { - state.set_dirty(); - *stmt = Stmt::Noop(*pos); + Stmt::If(x, ..) if matches!(x.0, Expr::BoolConstant(false, ..)) && x.2.is_empty() => { + if let Expr::BoolConstant(false, pos) = x.0 { + state.set_dirty(); + *stmt = Stmt::Noop(pos); + } else { + unreachable!("`Expr::BoolConstant`"); + } } // if false { if_block } else { else_block } -> else_block - Stmt::If(Expr::BoolConstant(false, ..), x, ..) => { + Stmt::If(x, ..) if matches!(x.0, Expr::BoolConstant(false, ..)) => { + state.set_dirty(); + *stmt = + match optimize_stmt_block(mem::take(&mut *x.2), state, preserve_result, true, false) + { + statements if statements.is_empty() => Stmt::Noop(x.2.position()), + statements => (statements, x.2.span()).into(), + } + } + // if true { if_block } else { else_block } -> if_block + Stmt::If(x, ..) if matches!(x.0, Expr::BoolConstant(true, ..)) => { state.set_dirty(); *stmt = match optimize_stmt_block(mem::take(&mut *x.1), state, preserve_result, true, false) { statements if statements.is_empty() => Stmt::Noop(x.1.position()), - statements => Stmt::Block(statements.into_boxed_slice(), x.1.span()), - } - } - // if true { if_block } else { else_block } -> if_block - Stmt::If(Expr::BoolConstant(true, ..), x, ..) => { - state.set_dirty(); - *stmt = - match optimize_stmt_block(mem::take(&mut *x.0), state, preserve_result, true, false) - { - statements if statements.is_empty() => Stmt::Noop(x.0.position()), - statements => Stmt::Block(statements.into_boxed_slice(), x.0.span()), + statements => (statements, x.1.span()).into(), } } // if expr { if_block } else { else_block } - Stmt::If(condition, x, ..) => { + Stmt::If(x, ..) => { + let (condition, body, other) = x.as_mut(); optimize_expr(condition, state, false); - *x.0 = optimize_stmt_block(mem::take(&mut *x.0), state, preserve_result, true, false); - *x.1 = optimize_stmt_block(mem::take(&mut *x.1), state, preserve_result, true, false); + **body = + optimize_stmt_block(mem::take(&mut **body), state, preserve_result, true, false); + **other = + optimize_stmt_block(mem::take(&mut **other), state, preserve_result, true, false); } // switch const { ... } - Stmt::Switch(match_expr, x, pos) if match_expr.is_constant() => { + Stmt::Switch(x, pos) if x.0.is_constant() => { + let ( + match_expr, + SwitchCases { + cases, + ranges, + def_case, + }, + ) = x.as_mut(); + let value = match_expr.get_literal_value().unwrap(); let hasher = &mut get_hasher(); value.hash(hasher); let hash = hasher.finish(); - let cases = &mut x.cases; - // First check hashes if let Some(block) = cases.get_mut(&hash) { if let Some(mut condition) = mem::take(&mut block.condition) { @@ -526,18 +546,18 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b optimize_expr(&mut condition, state, false); let def_stmt = - optimize_stmt_block(mem::take(&mut x.def_case), state, true, true, false); + optimize_stmt_block(mem::take(def_case), state, true, true, false); *stmt = Stmt::If( - condition, - Box::new(( + ( + condition, mem::take(&mut block.statements), - Stmt::Block( - def_stmt.into_boxed_slice(), - x.def_case.span_or_else(*pos, Position::NONE), - ) + StmtBlock::new_with_span( + def_stmt, + def_case.span_or_else(*pos, Position::NONE), + ), + ) .into(), - )), match_expr.start_position(), ); } else { @@ -549,7 +569,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b true, false, ); - *stmt = Stmt::Block(statements.into_boxed_slice(), block.statements.span()); + *stmt = (statements, block.statements.span()).into(); } state.set_dirty(); @@ -557,8 +577,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } // Then check ranges - let ranges = &mut x.ranges; - if value.is::() && !ranges.is_empty() { let value = value.as_int().expect("`INT`"); @@ -576,23 +594,18 @@ 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 def_stmt = optimize_stmt_block( - mem::take(&mut x.def_case), - state, - true, - true, - false, - ); + let def_stmt = + optimize_stmt_block(mem::take(def_case), state, true, true, false); *stmt = Stmt::If( - condition, - Box::new(( + ( + condition, mem::take(&mut block.statements), - Stmt::Block( - def_stmt.into_boxed_slice(), - x.def_case.span_or_else(*pos, Position::NONE), - ) + StmtBlock::new_with_span( + def_stmt, + def_case.span_or_else(*pos, Position::NONE), + ), + ) .into(), - )), match_expr.start_position(), ); } else { @@ -600,8 +613,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b let statements = mem::take(&mut *block.statements); let statements = optimize_stmt_block(statements, state, true, true, false); - *stmt = - Stmt::Block(statements.into_boxed_slice(), block.statements.span()); + *stmt = (statements, block.statements.span()).into(); } state.set_dirty(); @@ -644,17 +656,25 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // Promote the default case state.set_dirty(); - let def_stmt = - optimize_stmt_block(mem::take(&mut x.def_case), state, true, true, false); - *stmt = Stmt::Block( - def_stmt.into_boxed_slice(), - x.def_case.span_or_else(*pos, Position::NONE), - ); + let def_stmt = optimize_stmt_block(mem::take(def_case), state, true, true, false); + *stmt = (def_stmt, def_case.span_or_else(*pos, Position::NONE)).into(); } // switch - Stmt::Switch(match_expr, x, ..) => { + Stmt::Switch(x, ..) => { + let ( + match_expr, + SwitchCases { + cases, + ranges, + def_case, + .. + }, + ) = x.as_mut(); + optimize_expr(match_expr, state, false); - for block in x.cases.values_mut() { + + // Optimize cases + for block in cases.values_mut() { let statements = mem::take(&mut *block.statements); *block.statements = optimize_stmt_block(statements, state, preserve_result, true, false); @@ -669,30 +689,58 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } // Remove false cases - while let Some((&key, ..)) = x.cases.iter().find(|(.., block)| match block.condition { - Some(Expr::BoolConstant(false, ..)) => true, - _ => false, - }) { - state.set_dirty(); - x.cases.remove(&key); + cases.retain(|_, block| match block.condition { + Some(Expr::BoolConstant(false, ..)) => { + state.set_dirty(); + false + } + _ => true, + }); + + // Optimize ranges + for (.., block) in ranges.iter_mut() { + let statements = mem::take(&mut *block.statements); + *block.statements = + optimize_stmt_block(statements, state, preserve_result, true, false); + + if let Some(mut condition) = mem::take(&mut block.condition) { + optimize_expr(&mut condition, state, false); + match condition { + Expr::Unit(..) | Expr::BoolConstant(true, ..) => state.set_dirty(), + _ => block.condition = Some(condition), + } + } } - let def_block = mem::take(&mut *x.def_case); - *x.def_case = optimize_stmt_block(def_block, state, preserve_result, true, false); + // Remove false ranges + ranges.retain(|(.., block)| match block.condition { + Some(Expr::BoolConstant(false, ..)) => { + state.set_dirty(); + false + } + _ => true, + }); + + let def_block = mem::take(&mut ***def_case); + ***def_case = optimize_stmt_block(def_block, state, preserve_result, true, false); } // while false { block } -> Noop - Stmt::While(Expr::BoolConstant(false, pos), ..) => { - state.set_dirty(); - *stmt = Stmt::Noop(*pos) - } + Stmt::While(x, ..) if matches!(x.0, Expr::BoolConstant(false, ..)) => match x.0 { + Expr::BoolConstant(false, pos) => { + state.set_dirty(); + *stmt = Stmt::Noop(pos) + } + _ => unreachable!("`Expr::BoolConstant"), + }, // while expr { block } - Stmt::While(condition, body, ..) => { + Stmt::While(x, ..) => { + let (condition, body) = x.as_mut(); optimize_expr(condition, state, false); if let Expr::BoolConstant(true, pos) = condition { *condition = Expr::Unit(*pos); } - ***body = optimize_stmt_block(mem::take(&mut **body), state, false, true, false); + **body = optimize_stmt_block(mem::take(&mut **body), state, false, true, false); if body.len() == 1 { match body[0] { @@ -701,14 +749,11 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b // Only a single break statement - turn into running the guard expression once state.set_dirty(); if !condition.is_unit() { - let mut statements = vec![Stmt::Expr(mem::take(condition))]; + let mut statements = vec![Stmt::Expr(mem::take(condition).into())]; if preserve_result { statements.push(Stmt::Noop(pos)) } - *stmt = Stmt::Block( - statements.into_boxed_slice(), - Span::new(pos, Position::NONE), - ); + *stmt = (statements, Span::new(pos, Position::NONE)).into(); } else { *stmt = Stmt::Noop(pos); }; @@ -717,37 +762,51 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } } } - // do { block } while false | do { block } until true -> { block } - Stmt::Do(body, Expr::BoolConstant(x, ..), options, ..) - if *x == options.contains(AST_OPTION_NEGATED) => + // do { block } until true -> { block } + Stmt::Do(x, options, ..) + if matches!(x.0, Expr::BoolConstant(true, ..)) + && options.contains(AST_OPTION_NEGATED) => { state.set_dirty(); - *stmt = Stmt::Block( - optimize_stmt_block(mem::take(&mut **body), state, false, true, false) - .into_boxed_slice(), - body.span(), - ); + *stmt = ( + optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false), + x.1.span(), + ) + .into(); + } + // do { block } while false -> { block } + Stmt::Do(x, options, ..) + if matches!(x.0, Expr::BoolConstant(false, ..)) + && !options.contains(AST_OPTION_NEGATED) => + { + state.set_dirty(); + *stmt = ( + optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false), + x.1.span(), + ) + .into(); } // do { block } while|until expr - Stmt::Do(body, condition, ..) => { - optimize_expr(condition, state, false); - ***body = optimize_stmt_block(mem::take(&mut **body), state, false, true, false); + Stmt::Do(x, ..) => { + optimize_expr(&mut x.0, state, false); + *x.1 = optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false); } // for id in expr { block } - Stmt::For(iterable, x, ..) => { - optimize_expr(iterable, state, false); - *x.2 = optimize_stmt_block(mem::take(&mut *x.2), state, false, true, false); + Stmt::For(x, ..) => { + optimize_expr(&mut x.1, state, false); + *x.3 = optimize_stmt_block(mem::take(&mut *x.3), state, false, true, false); } // let id = expr; - Stmt::Var(expr, _, options, ..) if !options.contains(AST_OPTION_CONSTANT) => { - optimize_expr(expr, state, false) + Stmt::Var(x, options, ..) if !options.contains(AST_OPTION_CONSTANT) => { + optimize_expr(&mut x.1, state, false) } // import expr as var; #[cfg(not(feature = "no_module"))] - Stmt::Import(expr, ..) => optimize_expr(expr, state, false), + Stmt::Import(x, ..) => optimize_expr(&mut x.0, state, false), // { block } - Stmt::Block(statements, span) => { - let statements = mem::take(statements).into_vec().into(); + Stmt::Block(block) => { + let span = block.span(); + let statements = block.take_statements().into_vec().into(); let mut block = optimize_stmt_block(statements, state, preserve_result, true, false); match block.as_mut_slice() { @@ -760,18 +819,18 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b state.set_dirty(); *stmt = mem::take(s); } - _ => *stmt = Stmt::Block(block.into_boxed_slice(), *span), + _ => *stmt = (block, span).into(), } } // try { pure try_block } catch ( var ) { catch_block } -> try_block Stmt::TryCatch(x, ..) if x.try_block.iter().all(Stmt::is_pure) => { // If try block is pure, there will never be any exceptions state.set_dirty(); - *stmt = Stmt::Block( - optimize_stmt_block(mem::take(&mut *x.try_block), state, false, true, false) - .into_boxed_slice(), + *stmt = ( + optimize_stmt_block(mem::take(&mut *x.try_block), state, false, true, false), x.try_block.span(), - ); + ) + .into(); } // try { try_block } catch ( var ) { catch_block } Stmt::TryCatch(x, ..) => { @@ -780,31 +839,33 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b *x.catch_block = optimize_stmt_block(mem::take(&mut *x.catch_block), state, false, true, false); } - // func(...) - Stmt::Expr(expr @ Expr::FnCall(..)) => { + + Stmt::Expr(expr) => { optimize_expr(expr, state, false); - match expr { + + match expr.as_mut() { + // func(...) Expr::FnCall(x, pos) => { state.set_dirty(); *stmt = Stmt::FnCall(mem::take(x), *pos); } + // {...}; + Expr::Stmt(x) => { + if x.is_empty() { + state.set_dirty(); + *stmt = Stmt::Noop(x.position()); + } else { + state.set_dirty(); + *stmt = mem::take(&mut **x).into(); + } + } + // expr; _ => (), } } - // {} - Stmt::Expr(Expr::Stmt(x)) if x.is_empty() => { - state.set_dirty(); - *stmt = Stmt::Noop(x.position()); - } - // {...}; - Stmt::Expr(Expr::Stmt(x)) => { - state.set_dirty(); - *stmt = mem::take(&mut **x).into(); - } - // expr; - Stmt::Expr(expr) => optimize_expr(expr, state, false), + // return expr; - Stmt::Return(_, Some(ref mut expr), _) => optimize_expr(expr, state, false), + Stmt::Return(Some(ref mut expr), ..) => optimize_expr(expr, state, false), // All other statements - skip _ => (), diff --git a/src/parser.rs b/src/parser.rs index e43c0fd1..3d1acfce 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -5,14 +5,14 @@ use crate::api::events::VarDefInfo; use crate::api::options::LanguageOptions; use crate::ast::{ BinaryExpr, ConditionalStmtBlock, CustomExpr, Expr, FnCallExpr, FnCallHashes, Ident, - OpAssignment, ScriptFnDef, Stmt, StmtBlockContainer, SwitchCases, TryCatchBlock, + OpAssignment, ScriptFnDef, Stmt, StmtBlock, StmtBlockContainer, SwitchCases, TryCatchBlock, AST_OPTION_FLAGS::*, }; use crate::engine::{Precedence, KEYWORD_THIS, OP_CONTAINS}; use crate::eval::{EvalState, GlobalRuntimeState}; use crate::func::hashing::get_hasher; use crate::tokenizer::{ - is_keyword_function, is_valid_function_name, is_valid_identifier, Span, Token, TokenStream, + is_keyword_function, is_valid_function_name, is_valid_identifier, Token, TokenStream, TokenizerControl, }; use crate::types::dynamic::AccessMode; @@ -1007,7 +1007,7 @@ fn parse_switch( } let mut cases = BTreeMap::>::new(); - let mut ranges = StaticVec::<(INT, INT, bool, ConditionalStmtBlock)>::new(); + let mut ranges = StaticVec::<(INT, INT, bool, Box)>::new(); let mut def_pos = Position::NONE; let mut def_stmt = None; @@ -1119,7 +1119,10 @@ fn parse_switch( }); } // Other range - _ => ranges.push((range.0, range.1, range.2, (condition, stmt).into())), + _ => { + let block: ConditionalStmtBlock = (condition, stmt).into(); + ranges.push((range.0, range.1, range.2, block.into())) + } } } None @@ -1129,7 +1132,7 @@ fn parse_switch( cases.insert(hash, block.into()); None } - (None, None) => Some(stmt.into()), + (None, None) => Some(Box::new(stmt.into())), _ => unreachable!("both hash and range in switch statement case"), }; @@ -1156,18 +1159,13 @@ fn parse_switch( } } - let def_case = def_stmt.unwrap_or_else(|| Stmt::Noop(Position::NONE).into()); + let cases = SwitchCases { + cases, + def_case: def_stmt.unwrap_or_else(|| StmtBlock::NONE.into()), + ranges, + }; - Ok(Stmt::Switch( - item, - SwitchCases { - cases, - def_case, - ranges, - } - .into(), - settings.pos, - )) + Ok(Stmt::Switch((item, cases).into(), settings.pos)) } /// Parse a primary expression. @@ -1877,7 +1875,7 @@ fn parse_op_assignment_stmt( .map(|(op, pos)| (Some(op), pos)) .expect(NEVER_ENDS), // Not op-assignment - _ => return Ok(Stmt::Expr(lhs)), + _ => return Ok(Stmt::Expr(lhs.into())), }; let mut settings = settings; @@ -2419,8 +2417,7 @@ fn parse_if( }; Ok(Stmt::If( - guard, - (if_body.into(), else_body.into()).into(), + (guard, if_body.into(), else_body.into()).into(), settings.pos, )) } @@ -2453,7 +2450,7 @@ fn parse_while_loop( let body = parse_block(input, state, lib, settings.level_up())?; - Ok(Stmt::While(guard, Box::new(body.into()), settings.pos)) + Ok(Stmt::While((guard, body.into()).into(), settings.pos)) } /// Parse a do loop. @@ -2491,12 +2488,7 @@ fn parse_do( let guard = parse_expr(input, state, lib, settings.level_up())?.ensure_bool_expr()?; ensure_not_assignment(input)?; - Ok(Stmt::Do( - Box::new(body.into()), - guard, - negated, - settings.pos, - )) + Ok(Stmt::Do((guard, body.into()).into(), negated, settings.pos)) } /// Parse a for loop. @@ -2584,8 +2576,7 @@ fn parse_for( state.stack.rewind(prev_stack_len); Ok(Stmt::For( - expr, - Box::new((loop_var, counter_var, body.into())), + Box::new((loop_var, expr, counter_var, body.into())), settings.pos, )) } @@ -2669,14 +2660,13 @@ fn parse_let( // let name = expr AccessMode::ReadWrite => { state.stack.push(name, ()); - Ok(Stmt::Var(expr, var_def.into(), export, settings.pos)) + Ok(Stmt::Var((var_def, expr).into(), export, settings.pos)) } // const name = { expr:constant } AccessMode::ReadOnly => { state.stack.push_constant(name, ()); Ok(Stmt::Var( - expr, - var_def.into(), + (var_def, expr).into(), AST_OPTION_CONSTANT + export, settings.pos, )) @@ -2704,7 +2694,7 @@ fn parse_import( // import expr as ... if !match_token(input, Token::As).0 { - return Ok(Stmt::Import(expr, None, settings.pos)); + return Ok(Stmt::Import((expr, None).into(), settings.pos)); } // import expr as name ... @@ -2713,8 +2703,7 @@ fn parse_import( state.imports.push(name.clone()); Ok(Stmt::Import( - expr, - Some(Ident { name, pos }.into()), + (expr, Some(Ident { name, pos })).into(), settings.pos, )) } @@ -2865,10 +2854,7 @@ fn parse_block( #[cfg(not(feature = "no_module"))] state.imports.truncate(orig_imports_len); - Ok(Stmt::Block( - statements.into_boxed_slice(), - Span::new(settings.pos, end_pos), - )) + Ok((statements, settings.pos, end_pos).into()) } /// Parse an expression as a statement. @@ -3071,17 +3057,17 @@ fn parse_stmt( match input.peek().expect(NEVER_ENDS) { // `return`/`throw` at - (Token::EOF, ..) => Ok(Stmt::Return(return_type, None, token_pos)), + (Token::EOF, ..) => Ok(Stmt::Return(None, return_type, token_pos)), // `return`/`throw` at end of block (Token::RightBrace, ..) if !settings.is_global => { - Ok(Stmt::Return(return_type, None, token_pos)) + Ok(Stmt::Return(None, return_type, token_pos)) } // `return;` or `throw;` - (Token::SemiColon, ..) => Ok(Stmt::Return(return_type, None, token_pos)), + (Token::SemiColon, ..) => Ok(Stmt::Return(None, return_type, token_pos)), // `return` or `throw` with expression _ => { let expr = parse_expr(input, state, lib, settings.level_up())?; - Ok(Stmt::Return(return_type, Some(expr), token_pos)) + Ok(Stmt::Return(Some(expr.into()), return_type, token_pos)) } } } @@ -3327,9 +3313,9 @@ fn make_curry_from_externals( statements.extend( externals .into_iter() - .map(|crate::ast::Ident { name, pos }| Stmt::Share(name, pos)), + .map(|crate::ast::Ident { name, pos }| Stmt::Share(name.into(), pos)), ); - statements.push(Stmt::Expr(expr)); + statements.push(Stmt::Expr(expr.into())); Expr::Stmt(crate::ast::StmtBlock::new(statements, pos, Position::NONE).into()) } @@ -3482,8 +3468,8 @@ impl Engine { } } - let mut statements = smallvec::SmallVec::new_const(); - statements.push(Stmt::Expr(expr)); + let mut statements = StmtBlockContainer::new_const(); + statements.push(Stmt::Expr(expr.into())); #[cfg(not(feature = "no_optimize"))] return Ok(crate::optimizer::optimize_into_ast( @@ -3509,7 +3495,7 @@ impl Engine { input: &mut TokenStream, state: &mut ParseState, ) -> ParseResult<(StmtBlockContainer, StaticVec>)> { - let mut statements = smallvec::SmallVec::new_const(); + let mut statements = StmtBlockContainer::new_const(); let mut functions = BTreeMap::new(); while !input.peek().expect(NEVER_ENDS).0.is_eof() { diff --git a/src/tests.rs b/src/tests.rs index e96d8dd5..f9e099e2 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -21,8 +21,8 @@ fn check_struct_sizes() { ); assert_eq!(size_of::(), if PACKED { 12 } else { 16 }); assert_eq!(size_of::>(), if PACKED { 12 } else { 16 }); - assert_eq!(size_of::(), if PACKED { 24 } else { 32 }); - assert_eq!(size_of::>(), if PACKED { 24 } else { 32 }); + assert_eq!(size_of::(), if PACKED { 12 } else { 16 }); + assert_eq!(size_of::>(), if PACKED { 12 } else { 16 }); #[cfg(target_pointer_width = "64")] { diff --git a/tests/optimizer.rs b/tests/optimizer.rs index af9754f4..cac15cf2 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -84,7 +84,7 @@ fn test_optimizer_parse() -> Result<(), Box> { assert_eq!( format!("{:?}", ast), - r#"AST { body: [Var(false @ 1:18, "DECISION" @ 1:7, (Constant), 1:1), Expr(123 @ 1:51)] }"# + r#"AST { body: [Var(("DECISION" @ 1:7, false @ 1:18), (Constant), 1:1), Expr(123 @ 1:51)] }"# ); let ast = engine.compile("if 1 == 2 { 42 }")?; From 1011602cf60574ba86ade2b5d5a745d8412f4cf4 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 16 Feb 2022 18:05:09 +0800 Subject: [PATCH 07/17] Fix no-std build. --- src/ast/stmt.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 26c880fe..88aee7e0 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -299,7 +299,10 @@ impl From for StmtBlock { impl IntoIterator for StmtBlock { type Item = Stmt; + #[cfg(not(feature = "no_std"))] type IntoIter = smallvec::IntoIter<[Stmt; 8]>; + #[cfg(feature = "no_std")] + type IntoIter = smallvec::IntoIter<[Stmt; 3]>; #[inline(always)] fn into_iter(self) -> Self::IntoIter { From 83786c992bdd152fb785c88be68dae137a05d6f6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 18 Feb 2022 11:05:58 +0800 Subject: [PATCH 08/17] Fix parser bug. --- CHANGELOG.md | 9 ++++++ src/parser.rs | 85 +++++++++++++++++++++++++++++---------------------- 2 files changed, 57 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d517f055..01267a2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ Rhai Release Notes ================== +Version 1.6.0 +============= + +Bug fixes +--------- + +* Invalid property or method access such as `a.b::c.d` or `a.b::func()` no longer panics but properly returns a syntax error. + + Version 1.5.0 ============= diff --git a/src/parser.rs b/src/parser.rs index 3d1acfce..bcee4392 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1908,7 +1908,7 @@ fn make_dot_expr( // lhs.module::id - syntax error #[cfg(not(feature = "no_module"))] (.., Expr::Variable(.., x)) => { - Err(PERR::PropertyExpected.into_err(x.1.expect("`Some`").0[0].pos)) + Err(PERR::PropertyExpected.into_err(x.1.expect("`Some`").0.position())) } #[cfg(feature = "no_module")] (.., Expr::Variable(..)) => unreachable!("qualified property name"), @@ -1918,6 +1918,41 @@ fn make_dot_expr( false, op_pos, )), + // lhs.nnn::func(...) - syntax error + (.., Expr::FnCall(func, ..)) if func.is_qualified() => { + Err(PERR::PropertyExpected.into_err(func.namespace.expect("`Some`").position())) + } + // lhs.Fn() or lhs.eval() + (.., Expr::FnCall(func, func_pos)) + if func.args.is_empty() + && [crate::engine::KEYWORD_FN_PTR, crate::engine::KEYWORD_EVAL] + .contains(&func.name.as_ref()) => + { + let err_msg = format!( + "'{}' should not be called in method style. Try {}(...);", + func.name, func.name + ); + Err(LexError::ImproperSymbol(func.name.to_string(), err_msg).into_err(func_pos)) + } + // lhs.func!(...) + (.., Expr::FnCall(func, func_pos)) if func.capture_parent_scope => { + Err(PERR::MalformedCapture( + "method-call style does not support running within the caller's scope".into(), + ) + .into_err(func_pos)) + } + // lhs.func(...) + (lhs, Expr::FnCall(mut func, func_pos)) => { + // Recalculate hash + func.hashes = FnCallHashes::from_all( + #[cfg(not(feature = "no_function"))] + calc_fn_hash(&func.name, func.args.len()), + calc_fn_hash(&func.name, func.args.len() + 1), + ); + + let rhs = Expr::FnCall(func, func_pos); + Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), false, op_pos)) + } // lhs.dot_lhs.dot_rhs or lhs.dot_lhs[idx_rhs] (lhs, rhs @ Expr::Dot(..)) | (lhs, rhs @ Expr::Index(..)) => { let (x, term, pos, is_dot) = match rhs { @@ -1927,6 +1962,17 @@ fn make_dot_expr( }; match x.lhs { + // lhs.module::id.dot_rhs or lhs.module::id[idx_rhs] - syntax error + #[cfg(not(feature = "no_module"))] + Expr::Variable(.., x) if x.1.is_some() => { + Err(PERR::PropertyExpected.into_err(x.1.expect("`Some`").0.position())) + } + // lhs.module::func().dot_rhs or lhs.module::func()[idx_rhs] - syntax error + #[cfg(not(feature = "no_module"))] + Expr::FnCall(func, ..) if func.is_qualified() => { + Err(PERR::PropertyExpected.into_err(func.namespace.expect("`Some`").position())) + } + // lhs.id.dot_rhs or lhs.id[idx_rhs] Expr::Variable(..) | Expr::Property(..) => { let new_lhs = BinaryExpr { lhs: x.lhs.into_property(state), @@ -1941,6 +1987,7 @@ fn make_dot_expr( }; Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), false, op_pos)) } + // lhs.func().dot_rhs or lhs.func()[idx_rhs] Expr::FnCall(mut func, func_pos) => { // Recalculate hash func.hashes = FnCallHashes::from_all( @@ -1965,42 +2012,6 @@ fn make_dot_expr( expr => unreachable!("invalid dot expression: {:?}", expr), } } - // lhs.nnn::func(...) - (.., Expr::FnCall(x, ..)) if x.is_qualified() => { - unreachable!("method call should not be namespace-qualified") - } - // lhs.Fn() or lhs.eval() - (.., Expr::FnCall(x, pos)) - if x.args.is_empty() - && [crate::engine::KEYWORD_FN_PTR, crate::engine::KEYWORD_EVAL] - .contains(&x.name.as_ref()) => - { - Err(LexError::ImproperSymbol( - x.name.to_string(), - format!( - "'{}' should not be called in method style. Try {}(...);", - x.name, x.name - ), - ) - .into_err(pos)) - } - // lhs.func!(...) - (.., Expr::FnCall(x, pos)) if x.capture_parent_scope => Err(PERR::MalformedCapture( - "method-call style does not support running within the caller's scope".into(), - ) - .into_err(pos)), - // lhs.func(...) - (lhs, Expr::FnCall(mut func, func_pos)) => { - // Recalculate hash - func.hashes = FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] - calc_fn_hash(&func.name, func.args.len()), - calc_fn_hash(&func.name, func.args.len() + 1), - ); - - let rhs = Expr::FnCall(func, func_pos); - Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), false, op_pos)) - } // lhs.rhs (.., rhs) => Err(PERR::PropertyExpected.into_err(rhs.start_position())), } From bb04fab0110abff552d9aaf167e5b7770707b5f3 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 18 Feb 2022 15:04:46 +0800 Subject: [PATCH 09/17] Optimize variable shadowing. --- CHANGELOG.md | 5 ++++ src/ast/ast.rs | 2 +- src/ast/stmt.rs | 17 ++++++++----- src/eval/stmt.rs | 23 +++++++++++------ src/optimizer.rs | 2 +- src/parser.rs | 62 ++++++++++++++++++++++++++-------------------- src/types/scope.rs | 16 ++++++------ tests/optimizer.rs | 2 +- tests/options.rs | 8 ++++-- tests/var_scope.rs | 58 ++++++++++++++++++++++++++++++++++++++++--- 10 files changed, 137 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01267a2e..ab32c6e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ Bug fixes * Invalid property or method access such as `a.b::c.d` or `a.b::func()` no longer panics but properly returns a syntax error. +Enhancements +------------ + +* Variable definitions are optimized so that shadowed variables are reused as much as possible to reduce memory consumption. + Version 1.5.0 ============= diff --git a/src/ast/ast.rs b/src/ast/ast.rs index b7603111..40119fe5 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -748,7 +748,7 @@ impl AST { if options.contains(AST_OPTION_CONSTANT) && include_constants || !options.contains(AST_OPTION_CONSTANT) && include_variables => { - let (name, expr) = x.as_ref(); + let (name, expr, ..) = x.as_ref(); if let Some(value) = expr.get_literal_value() { Some((name.as_str(), options.contains(AST_OPTION_CONSTANT), value)) } else { diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 88aee7e0..fc45062f 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -11,6 +11,7 @@ use std::{ fmt, hash::Hash, mem, + num::NonZeroUsize, ops::{Deref, DerefMut}, }; @@ -345,14 +346,18 @@ pub enum Stmt { /// * [`AST_OPTION_NEGATED`] = `until` Do(Box<(Expr, StmtBlock)>, OptionFlags, Position), /// `for` `(` id `,` counter `)` `in` expr `{` stmt `}` - For(Box<(Ident, Expr, Option, StmtBlock)>, Position), + For(Box<(Ident, Option, Expr, StmtBlock)>, Position), /// \[`export`\] `let`|`const` id `=` expr /// /// ### Option Flags /// /// * [`AST_OPTION_EXPORTED`] = `export` /// * [`AST_OPTION_CONSTANT`] = `const` - Var(Box<(Ident, Expr)>, OptionFlags, Position), + Var( + Box<(Ident, Expr, Option)>, + OptionFlags, + Position, + ), /// expr op`=` expr Assignment(Box<(Option>, BinaryExpr)>, Position), /// func `(` expr `,` ... `)` @@ -380,12 +385,12 @@ pub enum Stmt { /// * [`AST_OPTION_NONE`] = `return` /// * [`AST_OPTION_BREAK`] = `throw` Return(Option>, OptionFlags, Position), - /// `import` expr `as` var + /// `import` expr `as` alias /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] Import(Box<(Expr, Option)>, Position), - /// `export` var `as` var + /// `export` var `as` alias /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] @@ -596,7 +601,7 @@ impl Stmt { // For loops can be pure because if the iterable is pure, it is finite, // so infinite loops can never occur. - Self::For(x, ..) => x.1.is_pure() && x.3.iter().all(Stmt::is_pure), + Self::For(x, ..) => x.2.is_pure() && x.3.iter().all(Stmt::is_pure), Self::Var(..) | Self::Assignment(..) | Self::FnCall(..) => false, Self::Block(block, ..) => block.iter().all(|stmt| stmt.is_pure()), @@ -765,7 +770,7 @@ impl Stmt { } } Self::For(x, ..) => { - if !x.1.walk(path, on_node) { + if !x.2.walk(path, on_node) { return false; } for s in x.3.iter() { diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 8d7d7358..9ac8c048 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -574,7 +574,7 @@ impl Engine { // For loop Stmt::For(x, ..) => { - let (Ident { name: var_name, .. }, expr, counter, statements) = x.as_ref(); + let (Ident { name: var_name, .. }, counter, expr, statements) = x.as_ref(); let iter_result = self .eval_expr(scope, global, state, lib, this_ptr, expr, level) @@ -818,20 +818,20 @@ impl Engine { } // Let/const statement Stmt::Var(x, options, pos) => { - let var_name = &x.0.name; - let expr = &x.1; + let (Ident { name: var_name, .. }, expr, index) = x.as_ref(); - let entry_type = if options.contains(AST_OPTION_CONSTANT) { + let access = if options.contains(AST_OPTION_CONSTANT) { AccessMode::ReadOnly } else { AccessMode::ReadWrite }; let export = options.contains(AST_OPTION_EXPORTED); + // Check variable definition filter let result = if let Some(ref filter) = self.def_var_filter { let will_shadow = scope.contains(var_name); let nesting_level = state.scope_level; - let is_const = entry_type == AccessMode::ReadOnly; + let is_const = access == AccessMode::ReadOnly; let info = VarDefInfo { name: var_name, is_const, @@ -864,16 +864,18 @@ impl Engine { if let Some(result) = result { result.map(|_| Dynamic::UNIT) } else { + // Evaluate initial value let value_result = self .eval_expr(scope, global, state, lib, this_ptr, expr, level) .map(Dynamic::flatten); - if let Ok(value) = value_result { + if let Ok(mut value) = value_result { let _alias = if !rewind_scope { + // Put global constants into global module #[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_module"))] if state.scope_level == 0 - && entry_type == AccessMode::ReadOnly + && access == AccessMode::ReadOnly && lib.iter().any(|&m| !m.is_empty()) { if global.constants.is_none() { @@ -896,7 +898,12 @@ impl Engine { None }; - scope.push_dynamic_value(var_name.clone(), entry_type, value); + if let Some(index) = index { + value.set_access_mode(access); + *scope.get_mut_by_index(scope.len() - index.get()) = value; + } else { + scope.push_entry(var_name.clone(), access, value); + } #[cfg(not(feature = "no_module"))] if let Some(alias) = _alias { diff --git a/src/optimizer.rs b/src/optimizer.rs index daad6b3d..412c0f39 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -793,7 +793,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b } // for id in expr { block } Stmt::For(x, ..) => { - optimize_expr(&mut x.1, state, false); + optimize_expr(&mut x.2, state, false); *x.3 = optimize_stmt_block(mem::take(&mut *x.3), state, false, true, false); } // let id = expr; diff --git a/src/parser.rs b/src/parser.rs index bcee4392..9365a817 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -53,7 +53,7 @@ pub struct ParseState<'e> { /// Encapsulates a local stack with variable names to simulate an actual runtime scope. pub stack: Scope<'e>, /// Size of the local variables stack upon entry of the current block scope. - pub entry_stack_len: usize, + pub block_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] pub external_vars: Vec, @@ -94,7 +94,7 @@ impl<'e> ParseState<'e> { allow_capture: true, interned_strings: StringsInterner::new(), stack: Scope::new(), - entry_stack_len: 0, + block_stack_len: 0, #[cfg(not(feature = "no_module"))] imports: StaticVec::new_const(), } @@ -2587,7 +2587,7 @@ fn parse_for( state.stack.rewind(prev_stack_len); Ok(Stmt::For( - Box::new((loop_var, expr, counter_var, body.into())), + Box::new((loop_var, counter_var, expr, body.into())), settings.pos, )) } @@ -2597,7 +2597,7 @@ fn parse_let( input: &mut TokenStream, state: &mut ParseState, lib: &mut FnLib, - var_type: AccessMode, + access: AccessMode, is_export: bool, settings: ParseSettings, ) -> ParseResult { @@ -2620,7 +2620,7 @@ fn parse_let( if let Some(ref filter) = state.engine.def_var_filter { let will_shadow = state.stack.iter().any(|(v, ..)| v == name.as_ref()); let level = settings.level; - let is_const = var_type == AccessMode::ReadOnly; + let is_const = access == AccessMode::ReadOnly; let info = VarDefInfo { name: &name, is_const, @@ -2648,10 +2648,6 @@ fn parse_let( } let name = state.get_identifier("", name); - let var_def = Ident { - name: name.clone(), - pos, - }; // let name = ... let expr = if match_token(input, Token::Equals).0 { @@ -2667,22 +2663,34 @@ fn parse_let( AST_OPTION_NONE }; - match var_type { + let existing = state.stack.get_index(&name).and_then(|(n, a)| { + if n < state.block_stack_len { + // Defined in parent block + None + } else if a == AccessMode::ReadOnly && access != AccessMode::ReadOnly { + // Overwrite constant + None + } else { + Some(n) + } + }); + + let idx = if let Some(n) = existing { + state.stack.get_mut_by_index(n).set_access_mode(access); + Some(NonZeroUsize::new(state.stack.len() - n).unwrap()) + } else { + state.stack.push_entry(name.as_str(), access, Dynamic::UNIT); + None + }; + + let var_def = (Ident { name, pos }, expr, idx).into(); + + Ok(match access { // let name = expr - AccessMode::ReadWrite => { - state.stack.push(name, ()); - Ok(Stmt::Var((var_def, expr).into(), export, settings.pos)) - } + AccessMode::ReadWrite => Stmt::Var(var_def, export, settings.pos), // const name = { expr:constant } - AccessMode::ReadOnly => { - state.stack.push_constant(name, ()); - Ok(Stmt::Var( - (var_def, expr).into(), - AST_OPTION_CONSTANT + export, - settings.pos, - )) - } - } + AccessMode::ReadOnly => Stmt::Var(var_def, AST_OPTION_CONSTANT + export, settings.pos), + }) } /// Parse an import statement. @@ -2798,8 +2806,8 @@ fn parse_block( let mut statements = Vec::with_capacity(8); - let prev_entry_stack_len = state.entry_stack_len; - state.entry_stack_len = state.stack.len(); + let prev_entry_stack_len = state.block_stack_len; + state.block_stack_len = state.stack.len(); #[cfg(not(feature = "no_module"))] let orig_imports_len = state.imports.len(); @@ -2859,8 +2867,8 @@ fn parse_block( } }; - state.stack.rewind(state.entry_stack_len); - state.entry_stack_len = prev_entry_stack_len; + state.stack.rewind(state.block_stack_len); + state.block_stack_len = prev_entry_stack_len; #[cfg(not(feature = "no_module"))] state.imports.truncate(orig_imports_len); diff --git a/src/types/scope.rs b/src/types/scope.rs index 85fd70d4..728470f9 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -37,7 +37,7 @@ const SCOPE_ENTRIES_INLINED: usize = 8; /// /// my_scope.push("z", 40_i64); /// -/// engine.eval_with_scope::<()>(&mut my_scope, "let x = z + 1; z = 0;")?; +/// engine.run(&mut my_scope, "let x = z + 1; z = 0;")?; /// /// assert_eq!(engine.eval_with_scope::(&mut my_scope, "x + 1")?, 42); /// @@ -185,7 +185,7 @@ impl Scope<'_> { /// ``` #[inline(always)] pub fn push(&mut self, name: impl Into, value: impl Variant + Clone) -> &mut Self { - self.push_dynamic_value(name, AccessMode::ReadWrite, Dynamic::from(value)) + self.push_entry(name, AccessMode::ReadWrite, Dynamic::from(value)) } /// Add (push) a new [`Dynamic`] entry to the [`Scope`]. /// @@ -201,7 +201,7 @@ impl Scope<'_> { /// ``` #[inline(always)] pub fn push_dynamic(&mut self, name: impl Into, value: Dynamic) -> &mut Self { - self.push_dynamic_value(name, value.access_mode(), value) + self.push_entry(name, value.access_mode(), value) } /// Add (push) a new constant to the [`Scope`]. /// @@ -224,7 +224,7 @@ impl Scope<'_> { name: impl Into, value: impl Variant + Clone, ) -> &mut Self { - self.push_dynamic_value(name, AccessMode::ReadOnly, Dynamic::from(value)) + self.push_entry(name, AccessMode::ReadOnly, Dynamic::from(value)) } /// Add (push) a new constant with a [`Dynamic`] value to the Scope. /// @@ -247,11 +247,11 @@ impl Scope<'_> { name: impl Into, value: Dynamic, ) -> &mut Self { - self.push_dynamic_value(name, AccessMode::ReadOnly, value) + self.push_entry(name, AccessMode::ReadOnly, value) } /// Add (push) a new entry with a [`Dynamic`] value to the [`Scope`]. #[inline] - pub(crate) fn push_dynamic_value( + pub(crate) fn push_entry( &mut self, name: impl Into, access: AccessMode, @@ -622,7 +622,7 @@ impl> Extend<(K, Dynamic)> for Scope<'_> { #[inline] fn extend>(&mut self, iter: T) { for (name, value) in iter { - self.push_dynamic_value(name, AccessMode::ReadWrite, value); + self.push_entry(name, AccessMode::ReadWrite, value); } } } @@ -640,7 +640,7 @@ impl> Extend<(K, bool, Dynamic)> for Scope<'_> { #[inline] fn extend>(&mut self, iter: T) { for (name, is_constant, value) in iter { - self.push_dynamic_value( + self.push_entry( name, if is_constant { AccessMode::ReadOnly diff --git a/tests/optimizer.rs b/tests/optimizer.rs index cac15cf2..886b4fff 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -84,7 +84,7 @@ fn test_optimizer_parse() -> Result<(), Box> { assert_eq!( format!("{:?}", ast), - r#"AST { body: [Var(("DECISION" @ 1:7, false @ 1:18), (Constant), 1:1), Expr(123 @ 1:51)] }"# + r#"AST { body: [Var(("DECISION" @ 1:7, false @ 1:18, None), (Constant), 1:1), Expr(123 @ 1:51)] }"# ); let ast = engine.compile("if 1 == 2 { 42 }")?; diff --git a/tests/options.rs b/tests/options.rs index d7e45780..803a5802 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -25,11 +25,15 @@ fn test_options_allow() -> Result<(), Box> { assert!(engine.compile("let x = || 42;").is_err()); } - engine.compile("while x > y { foo(z); }")?; + let ast = engine.compile("let x = 0; while x < 10 { x += 1; }")?; engine.set_allow_looping(false); - assert!(engine.compile("while x > y { foo(z); }").is_err()); + engine.run_ast(&ast)?; + + assert!(engine + .compile("let x = 0; while x < 10 { x += 1; }") + .is_err()); engine.compile("let x = 42; let x = 123;")?; diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 84678747..813dfe5a 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -5,17 +5,67 @@ fn test_var_scope() -> Result<(), Box> { let engine = Engine::new(); let mut scope = Scope::new(); - engine.eval_with_scope::<()>(&mut scope, "let x = 4 + 5")?; + engine.run_with_scope(&mut scope, "let x = 4 + 5")?; assert_eq!(engine.eval_with_scope::(&mut scope, "x")?, 9); - engine.eval_with_scope::<()>(&mut scope, "x += 1; x += 2;")?; + engine.run_with_scope(&mut scope, "x += 1; x += 2;")?; assert_eq!(engine.eval_with_scope::(&mut scope, "x")?, 12); scope.set_value("x", 42 as INT); assert_eq!(engine.eval_with_scope::(&mut scope, "x")?, 42); - engine.eval_with_scope::<()>(&mut scope, "{let x = 3}")?; + engine.run_with_scope(&mut scope, "{ let x = 3 }")?; assert_eq!(engine.eval_with_scope::(&mut scope, "x")?, 42); + #[cfg(not(feature = "no_optimize"))] + if engine.optimization_level() != rhai::OptimizationLevel::None { + scope.clear(); + engine.run_with_scope(&mut scope, "let x = 3; let x = 42; let x = 123;")?; + assert_eq!(scope.len(), 1); + assert_eq!(scope.get_value::("x").unwrap(), 123); + + scope.clear(); + engine.run_with_scope( + &mut scope, + "let x = 3; let y = 0; let x = 42; let y = 999; let x = 123;", + )?; + assert_eq!(scope.len(), 2); + assert_eq!(scope.get_value::("x").unwrap(), 123); + assert_eq!(scope.get_value::("y").unwrap(), 999); + + scope.clear(); + engine.run_with_scope( + &mut scope, + "const x = 3; let y = 0; let x = 42; let y = 999; const x = 123;", + )?; + assert_eq!(scope.len(), 3); + assert_eq!(scope.get_value::("x").unwrap(), 123); + assert_eq!(scope.get_value::("y").unwrap(), 999); + + scope.clear(); + engine.run_with_scope( + &mut scope, + "let x = 3; let y = 0; { let x = 42; let y = 999; } let x = 123;", + )?; + + assert_eq!( + engine.eval::( + " + let sum = 0; + for x in 0..10 { + let x = 42; + sum += x; + } + sum + ", + )?, + 420 + ); + } + + assert_eq!(scope.len(), 2); + assert_eq!(scope.get_value::("x").unwrap(), 123); + assert_eq!(scope.get_value::("y").unwrap(), 0); + Ok(()) } @@ -60,7 +110,7 @@ fn test_scope_eval() -> Result<(), Box> { // First invocation engine - .eval_with_scope::<()>(&mut scope, " let x = 4 + 5 - y + z; y = 1;") + .run_with_scope(&mut scope, " let x = 4 + 5 - y + z; y = 1;") .expect("variables y and z should exist"); // Second invocation using the same state From ef87ae48c10827b239ec1c0c2ae08c39bdd544e9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 18 Feb 2022 16:21:20 +0800 Subject: [PATCH 10/17] Fix doc-test. --- src/types/scope.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/types/scope.rs b/src/types/scope.rs index 728470f9..7114f8a3 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -37,10 +37,11 @@ const SCOPE_ENTRIES_INLINED: usize = 8; /// /// my_scope.push("z", 40_i64); /// -/// engine.run(&mut my_scope, "let x = z + 1; z = 0;")?; +/// engine.run_with_scope(&mut my_scope, "let x = z + 1; z = 0;")?; /// -/// assert_eq!(engine.eval_with_scope::(&mut my_scope, "x + 1")?, 42); +/// let result: i64 = engine.eval_with_scope(&mut my_scope, "x + 1")?; /// +/// assert_eq!(result, 42); /// assert_eq!(my_scope.get_value::("x").expect("x should exist"), 41); /// assert_eq!(my_scope.get_value::("z").expect("z should exist"), 0); /// # Ok(()) @@ -52,11 +53,11 @@ const SCOPE_ENTRIES_INLINED: usize = 8; // // # Implementation Notes // -// [`Scope`] is implemented as two [`Vec`]'s of exactly the same length. Variables data (name, -// type, etc.) is manually split into two equal-length arrays. That's because variable names take -// up the most space, with [`Identifier`] being four words long, but in the vast majority of -// cases the name is NOT used to look up a variable. Variable lookup is usually via direct -// indexing, by-passing the name altogether. +// [`Scope`] is implemented as two arrays of exactly the same length. Variables data (name, type, +// etc.) is manually split into two equal-length arrays. That's because variable names take up the +// most space, with [`Identifier`] being three words long, but in the vast majority of cases the +// name is NOT used to look up a variable. Variable lookup is usually via direct indexing, +// by-passing the name altogether. // // Since [`Dynamic`] is reasonably small, packing it tightly improves cache locality when variables // are accessed. From 78b5c9fd4ee99a2a3c74aaa62e3a2c5680b4b711 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 18 Feb 2022 19:13:09 +0800 Subject: [PATCH 11/17] Fix bug in Scope::is_constant. --- CHANGELOG.md | 1 + src/types/scope.rs | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab32c6e4..513329e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Bug fixes --------- * Invalid property or method access such as `a.b::c.d` or `a.b::func()` no longer panics but properly returns a syntax error. +* `Scope::is_constant` now returns the correct value. Enhancements ------------ diff --git a/src/types/scope.rs b/src/types/scope.rs index 7114f8a3..310877c7 100644 --- a/src/types/scope.rs +++ b/src/types/scope.rs @@ -375,9 +375,11 @@ impl Scope<'_> { /// ``` #[inline] pub fn is_constant(&self, name: &str) -> Option { - self.get_index(name).and_then(|(.., access)| match access { - AccessMode::ReadWrite => None, - AccessMode::ReadOnly => Some(true), + self.get_index(name).and_then(|(.., access)| { + Some(match access { + AccessMode::ReadWrite => false, + AccessMode::ReadOnly => true, + }) }) } /// Update the value of the named entry in the [`Scope`] if it already exists and is not constant. From 67a663881861370e8d60f1eb975caa7a86741564 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 18 Feb 2022 19:14:42 +0800 Subject: [PATCH 12/17] Allow variable to overwrite constant when shadowing. --- src/parser.rs | 5 +---- tests/var_scope.rs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 9365a817..6c39bf57 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2663,13 +2663,10 @@ fn parse_let( AST_OPTION_NONE }; - let existing = state.stack.get_index(&name).and_then(|(n, a)| { + let existing = state.stack.get_index(&name).and_then(|(n, ..)| { if n < state.block_stack_len { // Defined in parent block None - } else if a == AccessMode::ReadOnly && access != AccessMode::ReadOnly { - // Overwrite constant - None } else { Some(n) } diff --git a/tests/var_scope.rs b/tests/var_scope.rs index 813dfe5a..d7998379 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -32,14 +32,27 @@ fn test_var_scope() -> Result<(), Box> { assert_eq!(scope.get_value::("x").unwrap(), 123); assert_eq!(scope.get_value::("y").unwrap(), 999); + scope.clear(); + engine.run_with_scope( + &mut scope, + "const x = 3; let y = 0; let x = 42; let y = 999;", + )?; + assert_eq!(scope.len(), 2); + assert_eq!(scope.get_value::("x").unwrap(), 42); + assert_eq!(scope.get_value::("y").unwrap(), 999); + assert!(!scope.is_constant("x").unwrap()); + assert!(!scope.is_constant("y").unwrap()); + scope.clear(); engine.run_with_scope( &mut scope, "const x = 3; let y = 0; let x = 42; let y = 999; const x = 123;", )?; - assert_eq!(scope.len(), 3); + assert_eq!(scope.len(), 2); assert_eq!(scope.get_value::("x").unwrap(), 123); assert_eq!(scope.get_value::("y").unwrap(), 999); + assert!(scope.is_constant("x").unwrap()); + assert!(!scope.is_constant("y").unwrap()); scope.clear(); engine.run_with_scope( From dd566ed1e1a1b4b610c68df36bbaea41d2e27aa2 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 19 Feb 2022 12:26:17 +0800 Subject: [PATCH 13/17] Fix builds. --- src/parser.rs | 17 ++++++++--------- tests/var_scope.rs | 8 ++++---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 6c39bf57..b61653e2 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -271,7 +271,7 @@ impl Expr { fn into_property(self, state: &mut ParseState) -> Self { match self { #[cfg(not(feature = "no_module"))] - Self::Variable(.., ref x) if x.1.is_some() => self, + Self::Variable(.., x) if x.1.is_some() => unreachable!("qualified property"), Self::Variable(.., pos, x) => { let ident = x.2; let getter = state.get_identifier(crate::engine::FN_GET, &ident); @@ -1900,18 +1900,16 @@ fn make_dot_expr( x.rhs = make_dot_expr(state, x.rhs, term || terminate_chaining, rhs, op_pos)?; Ok(Expr::Index(x, false, pos)) } + // lhs.module::id - syntax error + #[cfg(not(feature = "no_module"))] + (.., Expr::Variable(.., x)) if x.1.is_some() => { + Err(PERR::PropertyExpected.into_err(x.1.expect("`Some`").0.position())) + } // lhs.id - (lhs, var_expr @ Expr::Variable(..)) if var_expr.is_variable_access(true) => { + (lhs, var_expr @ Expr::Variable(..)) => { let rhs = var_expr.into_property(state); Ok(Expr::Dot(BinaryExpr { lhs, rhs }.into(), false, op_pos)) } - // lhs.module::id - syntax error - #[cfg(not(feature = "no_module"))] - (.., Expr::Variable(.., x)) => { - Err(PERR::PropertyExpected.into_err(x.1.expect("`Some`").0.position())) - } - #[cfg(feature = "no_module")] - (.., Expr::Variable(..)) => unreachable!("qualified property name"), // lhs.prop (lhs, prop @ Expr::Property(..)) => Ok(Expr::Dot( BinaryExpr { lhs, rhs: prop }.into(), @@ -1919,6 +1917,7 @@ fn make_dot_expr( op_pos, )), // lhs.nnn::func(...) - syntax error + #[cfg(not(feature = "no_module"))] (.., Expr::FnCall(func, ..)) if func.is_qualified() => { Err(PERR::PropertyExpected.into_err(func.namespace.expect("`Some`").position())) } diff --git a/tests/var_scope.rs b/tests/var_scope.rs index d7998379..2f50c457 100644 --- a/tests/var_scope.rs +++ b/tests/var_scope.rs @@ -60,6 +60,10 @@ fn test_var_scope() -> Result<(), Box> { "let x = 3; let y = 0; { let x = 42; let y = 999; } let x = 123;", )?; + assert_eq!(scope.len(), 2); + assert_eq!(scope.get_value::("x").unwrap(), 123); + assert_eq!(scope.get_value::("y").unwrap(), 0); + assert_eq!( engine.eval::( " @@ -75,10 +79,6 @@ fn test_var_scope() -> Result<(), Box> { ); } - assert_eq!(scope.len(), 2); - assert_eq!(scope.get_value::("x").unwrap(), 123); - assert_eq!(scope.get_value::("y").unwrap(), 0); - Ok(()) } From fa8e2e638b69370e227a4297d91f60344f2bc659 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 23 Feb 2022 15:43:27 +0800 Subject: [PATCH 14/17] Fix bug. --- src/func/register.rs | 62 +++++++++++++++++++++++--------------------- src/module/mod.rs | 9 ++++--- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/func/register.rs b/src/func/register.rs index eab4000d..289dd8db 100644 --- a/src/func/register.rs +++ b/src/func/register.rs @@ -85,22 +85,32 @@ pub trait RegisterNativeFunction { fn return_type_name() -> &'static str; } -#[inline] -#[must_use] -fn is_setter(_fn_name: &str) -> bool { - #[cfg(not(feature = "no_object"))] - if _fn_name.starts_with(crate::engine::FN_SET) { - return true; - } - #[cfg(not(feature = "no_index"))] - if _fn_name.starts_with(crate::engine::FN_IDX_SET) { - return true; - } - false -} - const EXPECT_ARGS: &str = "arguments"; +macro_rules! check_constant { + ($ctx:ident, $args:ident) => { + #[cfg(any(not(feature = "no_object"), not(feature = "no_index")))] + { + let args_len = $args.len(); + + if args_len > 0 && $args[0].is_read_only() { + let deny = match $ctx.fn_name() { + #[cfg(not(feature = "no_object"))] + f if args_len == 2 && f.starts_with(crate::engine::FN_SET) => true, + #[cfg(not(feature = "no_index"))] + crate::engine::FN_IDX_SET if args_len == 3 => true, + _ => false, + }; + if deny { + return Err( + ERR::ErrorAssignmentToConstant(String::new(), Position::NONE).into(), + ); + } + } + } + }; +} + macro_rules! def_register { () => { def_register!(imp from_pure :); @@ -124,11 +134,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { - if args.len() == 2 && args[0].is_read_only() && is_setter(ctx.fn_name()) { - return Err(ERR::ErrorAssignmentToConstant(String::new(), Position::NONE).into()); - } - // The arguments are assumed to be of the correct number and types! + check_constant!(ctx, args); + let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* @@ -152,11 +160,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { - if args.len() == 2 && args[0].is_read_only() && is_setter(ctx.fn_name()) { - return Err(ERR::ErrorAssignmentToConstant(String::new(), Position::NONE).into()); - } - // The arguments are assumed to be of the correct number and types! + check_constant!(ctx, args); + let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* @@ -180,11 +186,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::>() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { - if args.len() == 2 && args[0].is_read_only() && is_setter(ctx.fn_name()) { - return Err(ERR::ErrorAssignmentToConstant(String::new(), Position::NONE).into()); - } - // The arguments are assumed to be of the correct number and types! + check_constant!(ctx, args); + let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* @@ -205,11 +209,9 @@ macro_rules! def_register { #[cfg(feature = "metadata")] #[inline(always)] fn return_type_name() -> &'static str { std::any::type_name::>() } #[inline(always)] fn into_callable_function(self) -> CallableFunction { CallableFunction::$abi(Box::new(move |ctx: NativeCallContext, args: &mut FnCallArgs| { - if args.len() == 2 && args[0].is_read_only() && is_setter(ctx.fn_name()) { - return Err(ERR::ErrorAssignmentToConstant(String::new(), Position::NONE).into()); - } - // The arguments are assumed to be of the correct number and types! + check_constant!(ctx, args); + let mut _drain = args.iter_mut(); $($let $par = ($clone)(_drain.next().expect(EXPECT_ARGS)); )* diff --git a/src/module/mod.rs b/src/module/mod.rs index 4b241288..8139a155 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -1670,9 +1670,12 @@ impl Module { } /// Create a new [`Module`] by evaluating an [`AST`][crate::AST]. /// - /// The entire [`AST`][crate::AST] is encapsulated into each function, allowing functions - /// to cross-call each other. Functions in the global namespace, plus all functions - /// defined in the [`Module`], are _merged_ into a _unified_ namespace before each call. + /// The entire [`AST`][crate::AST] is encapsulated into each function, allowing functions to + /// cross-call each other. + /// + /// Functions in the global namespace, plus all functions defined in the [`Module`], are + /// _merged_ into a _unified_ namespace before each call. + /// /// Therefore, all functions will be found. #[cfg(not(feature = "no_module"))] pub(crate) fn eval_ast_as_new_raw( From 7263896776993d523dde5b4a64bb7e9364e18408 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 24 Feb 2022 08:54:37 +0800 Subject: [PATCH 15/17] Bump minimum compiler version. --- CHANGELOG.md | 5 +++++ Cargo.toml | 1 + README.md | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 513329e7..08fc72b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ Rhai Release Notes Version 1.6.0 ============= +Compiler version +---------------- + +* Minimum compiler version is now `1.57` due to [`smartstring`](https://crates.io/crates/smartstring) dependency. + Bug fixes --------- diff --git a/Cargo.toml b/Cargo.toml index 027d135f..4cc1409d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [".", "codegen"] [package] name = "rhai" version = "1.6.0" +rust-version = "1.57" edition = "2018" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] description = "Embedded scripting for Rust" diff --git a/README.md b/README.md index f64d937b..cf33fd5d 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Targets and builds * All CPU and O/S targets supported by Rust, including: * WebAssembly (WASM) * `no-std` -* Minimum Rust version 1.51 +* Minimum Rust version 1.57 Standard features From 2f5ce2fe5b1910f5cdc42954097518373f3961c0 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 24 Feb 2022 09:08:10 +0800 Subject: [PATCH 16/17] Deprecate Position::new_const. --- src/api/deprecated.rs | 26 +++++++++++++++++++++++++- src/tokenizer.rs | 22 +--------------------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/api/deprecated.rs b/src/api/deprecated.rs index dd8ebd7a..62d7b52c 100644 --- a/src/api/deprecated.rs +++ b/src/api/deprecated.rs @@ -2,7 +2,7 @@ use crate::{ Dynamic, Engine, EvalAltResult, Expression, FnPtr, ImmutableString, NativeCallContext, - RhaiResult, RhaiResultOf, Scope, AST, + Position, RhaiResult, RhaiResultOf, Scope, AST, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -314,3 +314,27 @@ impl Expression<'_> { self.get_string_value() } } + +impl Position { + /// Create a new [`Position`]. + /// + /// If `line` is zero, then [`None`] is returned. + /// + /// If `position` is zero, then it is at the beginning of a line. + /// + /// # Deprecated + /// + /// This function is deprecated. Use [`new`][Position::new] (which panics when `line` is zero) instead. + /// + /// This method will be removed in the next major version. + #[deprecated(since = "1.6.0", note = "use `new` instead")] + #[inline(always)] + #[must_use] + pub const fn new_const(line: u16, position: u16) -> Option { + if line == 0 { + None + } else { + Some(Self::new(line, position)) + } + } +} diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 8d3c70b2..bf8326ac 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -94,7 +94,7 @@ impl Position { /// Panics if `line` is zero. #[inline(always)] #[must_use] - pub fn new(line: u16, position: u16) -> Self { + pub const fn new(line: u16, position: u16) -> Self { assert!(line != 0, "line cannot be zero"); let _pos = position; @@ -106,26 +106,6 @@ impl Position { pos: _pos, } } - /// Create a new [`Position`]. - /// - /// If `line` is zero, then [`None`] is returned. - /// - /// If `position` is zero, then it is at the beginning of a line. - #[inline] - #[must_use] - pub const fn new_const(line: u16, position: u16) -> Option { - if line == 0 { - return None; - } - let _pos = position; - - Some(Self { - #[cfg(not(feature = "no_position"))] - line, - #[cfg(not(feature = "no_position"))] - pos: _pos, - }) - } /// Get the line number (1-based), or [`None`] if there is no position. #[inline] #[must_use] From 73f10b8adca5acac66eeacdded12d17f058f628e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 24 Feb 2022 10:36:20 +0800 Subject: [PATCH 17/17] Minor housekeeping. --- src/api/call_fn.rs | 8 ++-- src/api/custom_syntax.rs | 1 + src/api/eval.rs | 5 ++- src/api/files.rs | 15 ++++--- src/api/mod.rs | 27 ++++++++----- src/api/options.rs | 16 ++++---- src/ast/script_fn.rs | 4 +- src/ast/stmt.rs | 4 +- src/eval/chaining.rs | 86 ++++++++++++++++------------------------ src/eval/expr.rs | 8 +--- src/eval/global_state.rs | 15 ++++--- src/eval/stmt.rs | 19 ++++----- src/func/args.rs | 6 +-- src/func/call.rs | 14 ++++--- src/types/dynamic.rs | 35 ++++++++-------- 15 files changed, 128 insertions(+), 135 deletions(-) diff --git a/src/api/call_fn.rs b/src/api/call_fn.rs index ca41552d..32b91c96 100644 --- a/src/api/call_fn.rs +++ b/src/api/call_fn.rs @@ -77,8 +77,8 @@ impl Engine { .into() }) } - /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments - /// and the following options: + /// Call a script function defined in an [`AST`] with multiple [`Dynamic`] arguments and the + /// following options: /// /// * whether to evaluate the [`AST`] to load necessary modules before calling the function /// * whether to rewind the [`Scope`] after the function call @@ -95,8 +95,8 @@ impl Engine { /// All the arguments are _consumed_, meaning that they're replaced by `()`. /// This is to avoid unnecessarily cloning the arguments. /// - /// Do not use the arguments after this call. If they are needed afterwards, - /// clone them _before_ calling this function. + /// Do not use the arguments after this call. If they are needed afterwards, clone them _before_ + /// calling this function. /// /// # Example /// diff --git a/src/api/custom_syntax.rs b/src/api/custom_syntax.rs index 01e0d523..cf5713a7 100644 --- a/src/api/custom_syntax.rs +++ b/src/api/custom_syntax.rs @@ -85,6 +85,7 @@ impl Expression<'_> { self.0.position() } /// Get the value of this expression if it is a literal constant. + /// /// Supports [`INT`][crate::INT], [`FLOAT`][crate::FLOAT], `()`, `char`, `bool` and /// [`ImmutableString`][crate::ImmutableString]. /// diff --git a/src/api/eval.rs b/src/api/eval.rs index ff5c7254..fc7d1bd7 100644 --- a/src/api/eval.rs +++ b/src/api/eval.rs @@ -34,8 +34,9 @@ impl Engine { /// ## Constants Propagation /// /// If not [`OptimizationLevel::None`][crate::OptimizationLevel::None], constants defined within - /// the scope are propagated throughout the script _including_ functions. This allows functions - /// to be optimized based on dynamic global constants. + /// the scope are propagated throughout the script _including_ functions. + /// + /// This allows functions to be optimized based on dynamic global constants. /// /// # Example /// diff --git a/src/api/files.rs b/src/api/files.rs index 8c058f27..1cfd82a9 100644 --- a/src/api/files.rs +++ b/src/api/files.rs @@ -72,8 +72,9 @@ impl Engine { /// ## Constants Propagation /// /// If not [`OptimizationLevel::None`][crate::OptimizationLevel::None], constants defined within - /// the scope are propagated throughout the script _including_ functions. This allows functions - /// to be optimized based on dynamic global constants. + /// the scope are propagated throughout the script _including_ functions. + /// + /// This allows functions to be optimized based on dynamic global constants. /// /// # Example /// @@ -134,8 +135,9 @@ impl Engine { /// ## Constants Propagation /// /// If not [`OptimizationLevel::None`][crate::OptimizationLevel::None], constants defined within - /// the scope are propagated throughout the script _including_ functions. This allows functions - /// to be optimized based on dynamic global constants. + /// the scope are propagated throughout the script _including_ functions. + /// + /// This allows functions to be optimized based on dynamic global constants. /// /// # Example /// @@ -176,8 +178,9 @@ impl Engine { /// ## Constants Propagation /// /// If not [`OptimizationLevel::None`][crate::OptimizationLevel::None], constants defined within - /// the scope are propagated throughout the script _including_ functions. This allows functions - /// to be optimized based on dynamic global constants. + /// the scope are propagated throughout the script _including_ functions. + /// + /// This allows functions to be optimized based on dynamic global constants. #[inline] pub fn run_file_with_scope( &self, diff --git a/src/api/mod.rs b/src/api/mod.rs index 417ff39d..17730cce 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -82,21 +82,26 @@ impl Engine { pub const fn optimization_level(&self) -> crate::OptimizationLevel { self.optimization_level } - /// Optimize the [`AST`][crate::AST] with constants defined in an external Scope. An optimized - /// copy of the [`AST`][crate::AST] is returned while the original [`AST`][crate::AST] is consumed. + /// Optimize the [`AST`][crate::AST] with constants defined in an external Scope. + /// An optimized copy of the [`AST`][crate::AST] is returned while the original [`AST`][crate::AST] + /// is consumed. /// /// Not available under `no_optimize`. /// /// Although optimization is performed by default during compilation, sometimes it is necessary - /// to _re_-optimize an [`AST`][crate::AST]. For example, when working with constants that are - /// passed in via an external scope, it will be more efficient to optimize the - /// [`AST`][crate::AST] once again to take advantage of the new constants. + /// to _re_-optimize an [`AST`][crate::AST]. + /// + /// For example, when working with constants that are passed in via an external scope, it will + /// be more efficient to optimize the [`AST`][crate::AST] once again to take advantage of the + /// new constants. /// /// With this method, it is no longer necessary to recompile a large script. The script - /// [`AST`][crate::AST] can be compiled just once. Before evaluation, constants are passed into - /// the [`Engine`] via an external scope (i.e. with - /// [`Scope::push_constant`][crate::Scope::push_constant]). Then, the [`AST`][crate::AST] is - /// cloned and the copy re-optimized before running. + /// [`AST`][crate::AST] can be compiled just once. + /// + /// Before evaluation, constants are passed into the [`Engine`] via an external scope (i.e. with + /// [`Scope::push_constant`][crate::Scope::push_constant]). + /// + /// Then, the [`AST`][crate::AST] is cloned and the copy re-optimized before running. #[inline] #[must_use] pub fn optimize_ast( @@ -148,8 +153,8 @@ impl Engine { /// /// # Examples /// - /// The following will raise an error during parsing because the `if` keyword is disabled - /// and is recognized as a reserved symbol! + /// The following will raise an error during parsing because the `if` keyword is disabled and is + /// recognized as a reserved symbol! /// /// ```rust,should_panic /// # fn main() -> Result<(), rhai::ParseError> { diff --git a/src/api/options.rs b/src/api/options.rs index 230df25c..619be34c 100644 --- a/src/api/options.rs +++ b/src/api/options.rs @@ -57,7 +57,7 @@ impl Engine { /// Is `if`-expression allowed? /// Default is `true`. #[inline(always)] - pub fn allow_if_expression(&self) -> bool { + pub const fn allow_if_expression(&self) -> bool { self.options.allow_if_expr } /// Set whether `if`-expression is allowed. @@ -68,7 +68,7 @@ impl Engine { /// Is `switch` expression allowed? /// Default is `true`. #[inline(always)] - pub fn allow_switch_expression(&self) -> bool { + pub const fn allow_switch_expression(&self) -> bool { self.options.allow_switch_expr } /// Set whether `switch` expression is allowed. @@ -79,7 +79,7 @@ impl Engine { /// Is statement-expression allowed? /// Default is `true`. #[inline(always)] - pub fn allow_statement_expression(&self) -> bool { + pub const fn allow_statement_expression(&self) -> bool { self.options.allow_stmt_expr } /// Set whether statement-expression is allowed. @@ -93,7 +93,7 @@ impl Engine { /// Not available under `no_function`. #[cfg(not(feature = "no_function"))] #[inline(always)] - pub fn allow_anonymous_fn(&self) -> bool { + pub const fn allow_anonymous_fn(&self) -> bool { self.options.allow_anonymous_fn } /// Set whether anonymous function is allowed. @@ -107,7 +107,7 @@ impl Engine { /// Is looping allowed? /// Default is `true`. #[inline(always)] - pub fn allow_looping(&self) -> bool { + pub const fn allow_looping(&self) -> bool { self.options.allow_looping } /// Set whether looping is allowed. @@ -118,7 +118,7 @@ impl Engine { /// Is variables shadowing allowed? /// Default is `true`. #[inline(always)] - pub fn allow_shadowing(&self) -> bool { + pub const fn allow_shadowing(&self) -> bool { self.options.allow_shadowing } /// Set whether variables shadowing is allowed. @@ -129,7 +129,7 @@ impl Engine { /// Is strict variables mode enabled? /// Default is `false`. #[inline(always)] - pub fn strict_variables(&self) -> bool { + pub const fn strict_variables(&self) -> bool { self.options.strict_var } /// Set whether strict variables mode is enabled. @@ -143,7 +143,7 @@ impl Engine { /// Not available under `no_object`. #[cfg(not(feature = "no_object"))] #[inline(always)] - pub fn fail_on_invalid_map_property(&self) -> bool { + pub const fn fail_on_invalid_map_property(&self) -> bool { self.options.fail_on_invalid_map_property } /// Set whether to raise error if an object map property does not exist. diff --git a/src/ast/script_fn.rs b/src/ast/script_fn.rs index 0daf388f..f01bc8eb 100644 --- a/src/ast/script_fn.rs +++ b/src/ast/script_fn.rs @@ -84,8 +84,8 @@ pub struct ScriptFnMetadata<'a> { /// /// Line doc-comments are kept in one string slice per line without the termination line-break. /// - /// Leading white-spaces are stripped, and each string slice always starts with the corresponding - /// doc-comment leader: `///` or `/**`. + /// Leading white-spaces are stripped, and each string slice always starts with the + /// corresponding doc-comment leader: `///` or `/**`. #[cfg(feature = "metadata")] pub comments: Vec<&'a str>, /// Function access mode. diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index fc45062f..2a4e60bc 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -621,8 +621,8 @@ impl Stmt { } /// Does this statement's behavior depend on its containing block? /// - /// A statement that depends on its containing block behaves differently when promoted - /// to an upper block. + /// A statement that depends on its containing block behaves differently when promoted to an + /// upper block. /// /// Currently only variable definitions (i.e. `let` and `const`), `import`/`export` statements, /// and `eval` calls (which may in turn call define variables) fall under this category. diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index f80cf737..dcf50ab1 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -187,9 +187,9 @@ impl Engine { self.call_indexer_set( global, state, lib, target, idx, new_val, is_ref_mut, level, ) - .or_else(|idx_err| match *idx_err { + .or_else(|e| match *e { ERR::ErrorIndexingType(..) => Ok((Dynamic::UNIT, false)), - _ => Err(idx_err), + _ => Err(e), })?; } @@ -333,28 +333,25 @@ impl Engine { self.call_indexer_get( global, state, lib, target, &mut prop, level, ) - .map_err( - |idx_err| match *idx_err { + .map_err(|e| { + match *e { ERR::ErrorIndexingType(..) => err, - _ => idx_err, - }, - ) + _ => e, + } + }) } _ => Err(err), })?; - self.eval_op_assignment( - global, - state, - lib, - op_info, - op_pos, - &mut (&mut orig_val).into(), - root, - new_val, - level, - ) - .map_err(|err| err.fill_position(new_pos))?; + { + let orig_val = &mut (&mut orig_val).into(); + + self.eval_op_assignment( + global, state, lib, op_info, op_pos, orig_val, root, new_val, + level, + ) + .map_err(|err| err.fill_position(new_pos))?; + } new_val = orig_val; } @@ -373,12 +370,10 @@ impl Engine { self.call_indexer_set( global, state, lib, target, idx, new_val, is_ref_mut, level, ) - .map_err( - |idx_err| match *idx_err { - ERR::ErrorIndexingType(..) => err, - _ => idx_err, - }, - ) + .map_err(|e| match *e { + ERR::ErrorIndexingType(..) => err, + _ => e, + }) } _ => Err(err), }) @@ -403,11 +398,9 @@ impl Engine { self.call_indexer_get( global, state, lib, target, &mut prop, level, ) - .map_err(|idx_err| { - match *idx_err { - ERR::ErrorIndexingType(..) => err, - _ => idx_err, - } + .map_err(|e| match *e { + ERR::ErrorIndexingType(..) => err, + _ => e, }) } _ => Err(err), @@ -502,39 +495,28 @@ impl Engine { global, state, lib, target, &mut prop, level, ) .map_err( - |idx_err| match *idx_err { + |e| match *e { ERR::ErrorIndexingType(..) => err, - _ => idx_err, + _ => e, }, ) } _ => Err(err), })?; - let val = &mut val; + let val = &mut (&mut val).into(); let (result, may_be_changed) = self .eval_dot_index_chain_helper( - global, - state, - lib, - this_ptr, - &mut val.into(), - root, - rhs, - &x.rhs, - *term, - idx_values, - rhs_chain, - level, - new_val, + global, state, lib, this_ptr, val, root, rhs, &x.rhs, + *term, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(*x_pos))?; // Feed the value back via a setter just in case it has been updated if may_be_changed { // Re-use args because the first &mut parameter will not be consumed - let mut arg_values = [target.as_mut(), val]; + let mut arg_values = [target.as_mut(), val.as_mut()]; let args = &mut arg_values; self.exec_fn_call( None, global, state, lib, setter, hash_set, args, @@ -550,13 +532,13 @@ impl Engine { global, state, lib, target, idx, new_val, is_ref_mut, level, ) - .or_else(|idx_err| match *idx_err { + .or_else(|e| match *e { // If there is no setter, no need to feed it // back because the property is read-only ERR::ErrorIndexingType(..) => { Ok((Dynamic::UNIT, false)) } - _ => Err(idx_err), + _ => Err(e), }) } _ => Err(err), @@ -584,11 +566,11 @@ impl Engine { #[cfg(feature = "debugging")] global.debugger.reset_status(reset_debugger); - let val = &mut result?.0; - let target = &mut val.into(); + let (val, _) = &mut result?; + let val = &mut val.into(); self.eval_dot_index_chain_helper( - global, state, lib, this_ptr, target, root, rhs, &x.rhs, *term, + global, state, lib, this_ptr, val, root, rhs, &x.rhs, *term, idx_values, rhs_chain, level, new_val, ) .map_err(|err| err.fill_position(pos)) diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 6a9004e8..d6ea6025 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -419,17 +419,13 @@ impl Engine { }; #[cfg(not(feature = "unchecked"))] - let val_sizes = Self::calc_data_sizes(&value, true); + let delta = Self::calc_data_sizes(&value, true); *map.get_mut(key).unwrap() = value; #[cfg(not(feature = "unchecked"))] if self.has_data_size_limit() { - sizes = ( - sizes.0 + val_sizes.0, - sizes.1 + val_sizes.1, - sizes.2 + val_sizes.2, - ); + sizes = (sizes.0 + delta.0, sizes.1 + delta.1, sizes.2 + delta.2); self.raise_err_if_over_data_size_limit(sizes, value_expr.position())?; } } diff --git a/src/eval/global_state.rs b/src/eval/global_state.rs index 22f2574f..b15fa6ab 100644 --- a/src/eval/global_state.rs +++ b/src/eval/global_state.rs @@ -17,8 +17,10 @@ pub type GlobalConstants = // # Implementation Notes // // This implementation for imported [modules][crate::Module] splits the module names from the shared -// modules to improve data locality. Most usage will be looking up a particular key from the list -// and then getting the module that corresponds to that key. +// modules to improve data locality. +// +// Most usage will be looking up a particular key from the list and then getting the module that +// corresponds to that key. #[derive(Clone)] pub struct GlobalRuntimeState<'a> { /// Stack of module names. @@ -103,7 +105,8 @@ impl GlobalRuntimeState<'_> { pub fn get_shared_import(&self, index: usize) -> Option> { self.modules.get(index).cloned() } - /// Get a mutable reference to the globally-imported [crate::Module][crate::Module] at a particular index. + /// Get a mutable reference to the globally-imported [crate::Module][crate::Module] at a + /// particular index. /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] @@ -190,7 +193,8 @@ impl GlobalRuntimeState<'_> { ) -> impl Iterator)> { self.keys.iter().zip(self.modules.iter()) } - /// Does the specified function hash key exist in the stack of globally-imported [modules][crate::Module]? + /// Does the specified function hash key exist in the stack of globally-imported + /// [modules][crate::Module]? /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] @@ -200,7 +204,8 @@ impl GlobalRuntimeState<'_> { pub fn contains_qualified_fn(&self, hash: u64) -> bool { self.modules.iter().any(|m| m.contains_qualified_fn(hash)) } - /// Get the specified function via its hash key from the stack of globally-imported [modules][crate::Module]. + /// Get the specified function via its hash key from the stack of globally-imported + /// [modules][crate::Module]. /// /// Not available under `no_module`. #[cfg(not(feature = "no_module"))] diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 9ac8c048..8079cb58 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -257,16 +257,11 @@ impl Engine { #[cfg(not(feature = "unchecked"))] self.inc_operations(&mut global.num_operations, pos)?; + let root = (var_name, pos); + let lhs_ptr = &mut lhs_ptr; + self.eval_op_assignment( - global, - state, - lib, - *op_info, - *op_pos, - &mut lhs_ptr, - (var_name, pos), - rhs_val, - level, + global, state, lib, *op_info, *op_pos, lhs_ptr, root, rhs_val, level, ) .map_err(|err| err.fill_position(rhs.start_position())) .map(|_| Dynamic::UNIT) @@ -392,8 +387,8 @@ impl Engine { let hash = hasher.finish(); // First check hashes - if let Some(t) = cases.get(&hash) { - let cond_result = t + if let Some(case_block) = cases.get(&hash) { + let cond_result = case_block .condition .as_ref() .map(|cond| { @@ -410,7 +405,7 @@ impl Engine { .unwrap_or(Ok(true)); match cond_result { - Ok(true) => Ok(Some(&t.statements)), + Ok(true) => Ok(Some(&case_block.statements)), Ok(false) => Ok(None), _ => cond_result.map(|_| None), } diff --git a/src/func/args.rs b/src/func/args.rs index d612ad3f..4d14280c 100644 --- a/src/func/args.rs +++ b/src/func/args.rs @@ -9,7 +9,8 @@ use std::prelude::v1::*; /// Trait that parses arguments to a function call. /// -/// Any data type can implement this trait in order to pass arguments to [`Engine::call_fn`][crate::Engine::call_fn]. +/// Any data type can implement this trait in order to pass arguments to +/// [`Engine::call_fn`][crate::Engine::call_fn]. pub trait FuncArgs { /// Parse function call arguments into a container. /// @@ -65,8 +66,7 @@ impl FuncArgs for Vec { } } -/// Macro to implement [`FuncArgs`] for tuples of standard types (each can be -/// converted into a [`Dynamic`]). +/// Macro to implement [`FuncArgs`] for tuples of standard types (each can be converted into a [`Dynamic`]). macro_rules! impl_args { ($($p:ident),*) => { impl<$($p: Variant + Clone),*> FuncArgs for ($($p,)*) diff --git a/src/func/call.rs b/src/func/call.rs index 5a13b03f..da327258 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -340,7 +340,8 @@ impl Engine { /// Function call arguments be _consumed_ when the function requires them to be passed by value. /// All function arguments not in the first position are always passed by value and thus consumed. /// - /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! + /// **DO NOT** reuse the argument values unless for the first `&mut` argument - + /// all others are silently replaced by `()`! pub(crate) fn call_native_fn( &self, global: &mut GlobalRuntimeState, @@ -591,10 +592,11 @@ impl Engine { /// /// # WARNING /// - /// Function call arguments may be _consumed_ when the function requires them to be passed by value. - /// All function arguments not in the first position are always passed by value and thus consumed. + /// Function call arguments may be _consumed_ when the function requires them to be passed by + /// value. All function arguments not in the first position are always passed by value and thus consumed. /// - /// **DO NOT** reuse the argument values unless for the first `&mut` argument - all others are silently replaced by `()`! + /// **DO NOT** reuse the argument values unless for the first `&mut` argument - + /// all others are silently replaced by `()`! pub(crate) fn exec_fn_call( &self, scope: Option<&mut Scope>, @@ -1301,8 +1303,8 @@ impl Engine { // No arguments } else { // See if the first argument is a variable (not namespace-qualified). - // If so, convert to method-call style in order to leverage potential - // &mut first argument and avoid cloning the value + // If so, convert to method-call style in order to leverage potential &mut first argument + // and avoid cloning the value if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { #[cfg(feature = "debugging")] self.run_debugger(scope, global, state, lib, this_ptr, &args_expr[0], level)?; diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index 08a89729..abf7f346 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -339,8 +339,8 @@ impl Dynamic { } self } - /// Does this [`Dynamic`] hold a variant data type - /// instead of one of the supported system primitive types? + /// Does this [`Dynamic`] hold a variant data type instead of one of the supported system + /// primitive types? #[inline(always)] #[must_use] pub const fn is_variant(&self) -> bool { @@ -360,8 +360,7 @@ impl Dynamic { } /// Is the value held by this [`Dynamic`] a particular type? /// - /// If the [`Dynamic`] is a shared variant checking is performed on - /// top of its internal value. + /// If the [`Dynamic`] is a shared variant checking is performed on top of its internal value. #[inline] #[must_use] pub fn is(&self) -> bool { @@ -1072,11 +1071,14 @@ impl Dynamic { } /// Is this [`Dynamic`] read-only? /// - /// Constant [`Dynamic`] values are read-only. If a [`&mut Dynamic`][Dynamic] to such a constant - /// is passed to a Rust function, the function can use this information to return an error of - /// [`ErrorAssignmentToConstant`][crate::EvalAltResult::ErrorAssignmentToConstant] - /// if its value is going to be modified. This safe-guards constant values from being modified - /// from within Rust functions. + /// Constant [`Dynamic`] values are read-only. + /// + /// If a [`&mut Dynamic`][Dynamic] to such a constant is passed to a Rust function, the function + /// can use this information to return an error of + /// [`ErrorAssignmentToConstant`][crate::EvalAltResult::ErrorAssignmentToConstant] if its value + /// is going to be modified. + /// + /// This safe-guards constant values from being modified from within Rust functions. #[must_use] pub fn is_read_only(&self) -> bool { #[cfg(not(feature = "no_closure"))] @@ -1138,15 +1140,16 @@ impl Dynamic { /// /// # Notes /// - /// Beware that you need to pass in an [`Array`][crate::Array] type for it to be recognized as an [`Array`][crate::Array]. - /// A [`Vec`][Vec] does not get automatically converted to an [`Array`][crate::Array], but - /// will be a custom type instead (stored as a trait object). Use `Into` to convert a - /// [`Vec`][Vec] into a [`Dynamic`] as an [`Array`][crate::Array] value. + /// Beware that you need to pass in an [`Array`][crate::Array] type for it to be recognized as + /// an [`Array`][crate::Array]. A [`Vec`][Vec] does not get automatically converted to an + /// [`Array`][crate::Array], but will be a custom type instead (stored as a trait object). Use + /// `Into` to convert a [`Vec`][Vec] into a [`Dynamic`] as an + /// [`Array`][crate::Array] value. /// /// Similarly, passing in a [`HashMap`][std::collections::HashMap] or - /// [`BTreeMap`][std::collections::BTreeMap] will not get a [`Map`][crate::Map] - /// but a custom type. Again, use `Into` to get a [`Dynamic`] with a - /// [`Map`][crate::Map] value. + /// [`BTreeMap`][std::collections::BTreeMap] will not get a [`Map`][crate::Map] but a + /// custom type. Again, use `Into` to get a [`Dynamic`] with a [`Map`][crate::Map] + /// value. /// /// # Examples ///