From aacb7f0b2473ca82dd8ee339ddba1f571473cce9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Apr 2021 15:37:59 +0800 Subject: [PATCH 01/15] Ensure no interpolation for normal strings. --- tests/string.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/string.rs b/tests/string.rs index 9c72b27d..abcea364 100644 --- a/tests/string.rs +++ b/tests/string.rs @@ -325,6 +325,16 @@ fn test_string_interpolated() -> Result<(), Box> { "hello 42 worlds!" ); + assert_eq!( + engine.eval::( + r#" + let x = 40; + "hello ${x+2} worlds!" + "# + )?, + "hello ${x+2} worlds!" + ); + assert_eq!( engine.eval::( r" From bc9c1ab850f6a2237accd6ffed7843bc6d7142ab Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Apr 2021 23:08:27 +0800 Subject: [PATCH 02/15] Add external control interface for tokenizer. --- src/engine_api.rs | 61 ++++++++--------------------------------------- src/parser.rs | 47 +++++++++++++----------------------- src/token.rs | 42 ++++++++++++++++++++++++-------- 3 files changed, 58 insertions(+), 92 deletions(-) diff --git a/src/engine_api.rs b/src/engine_api.rs index 3b4cdee9..403080a2 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -9,7 +9,6 @@ use crate::parser::ParseState; use crate::stdlib::{ any::{type_name, TypeId}, boxed::Box, - num::NonZeroUsize, string::String, }; use crate::{ @@ -1158,16 +1157,8 @@ impl Engine { scripts: &[&str], optimization_level: OptimizationLevel, ) -> Result { - let (stream, buffer) = self.lex_raw(scripts, None); - let mut state = ParseState::new( - self, - buffer, - #[cfg(not(feature = "unchecked"))] - NonZeroUsize::new(self.max_expr_depth()), - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - NonZeroUsize::new(self.max_function_expr_depth()), - ); + let (stream, tokenizer_control) = self.lex_raw(scripts, None); + let mut state = ParseState::new(self, tokenizer_control); self.parse( &mut stream.peekable(), &mut state, @@ -1347,7 +1338,7 @@ impl Engine { .into()); }; - let (stream, buffer) = self.lex_raw( + let (stream, tokenizer_control) = self.lex_raw( &scripts, Some(if has_null { |token| match token { @@ -1360,15 +1351,7 @@ impl Engine { }), ); - let mut state = ParseState::new( - self, - buffer, - #[cfg(not(feature = "unchecked"))] - NonZeroUsize::new(self.max_expr_depth()), - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - NonZeroUsize::new(self.max_function_expr_depth()), - ); + let mut state = ParseState::new(self, tokenizer_control); let ast = self.parse_global_expr( &mut stream.peekable(), @@ -1454,18 +1437,10 @@ impl Engine { script: &str, ) -> Result { let scripts = [script]; - let (stream, buffer) = self.lex_raw(&scripts, None); + let (stream, tokenizer_control) = self.lex_raw(&scripts, None); let mut peekable = stream.peekable(); - let mut state = ParseState::new( - self, - buffer, - #[cfg(not(feature = "unchecked"))] - NonZeroUsize::new(self.max_expr_depth()), - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - NonZeroUsize::new(self.max_function_expr_depth()), - ); + let mut state = ParseState::new(self, tokenizer_control); self.parse_global_expr(&mut peekable, &mut state, scope, self.optimization_level) } /// Evaluate a script file. @@ -1624,16 +1599,8 @@ impl Engine { script: &str, ) -> Result> { let scripts = [script]; - let (stream, buffer) = self.lex_raw(&scripts, None); - let mut state = ParseState::new( - self, - buffer, - #[cfg(not(feature = "unchecked"))] - NonZeroUsize::new(self.max_expr_depth()), - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - NonZeroUsize::new(self.max_function_expr_depth()), - ); + let (stream, tokenizer_control) = self.lex_raw(&scripts, None); + let mut state = ParseState::new(self, tokenizer_control); // No need to optimize a lone expression let ast = self.parse_global_expr( @@ -1779,16 +1746,8 @@ impl Engine { script: &str, ) -> Result<(), Box> { let scripts = [script]; - let (stream, buffer) = self.lex_raw(&scripts, None); - let mut state = ParseState::new( - self, - buffer, - #[cfg(not(feature = "unchecked"))] - NonZeroUsize::new(self.max_expr_depth()), - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - NonZeroUsize::new(self.max_function_expr_depth()), - ); + let (stream, tokenizer_control) = self.lex_raw(&scripts, None); + let mut state = ParseState::new(self, tokenizer_control); let ast = self.parse( &mut stream.peekable(), diff --git a/src/parser.rs b/src/parser.rs index 96482ca6..25622144 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -11,7 +11,6 @@ use crate::optimize::optimize_into_ast; use crate::optimize::OptimizationLevel; use crate::stdlib::{ boxed::Box, - cell::Cell, collections::BTreeMap, format, hash::{Hash, Hasher}, @@ -22,7 +21,9 @@ use crate::stdlib::{ vec::Vec, }; use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; -use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; +use crate::token::{ + is_keyword_function, is_valid_identifier, Token, TokenStream, TokenizerControl, +}; use crate::utils::{get_hasher, IdentifierBuilder}; use crate::{ calc_fn_hash, Dynamic, Engine, Identifier, LexError, ParseError, ParseErrorType, Position, @@ -45,7 +46,7 @@ pub struct ParseState<'e> { /// Reference to the scripting [`Engine`]. engine: &'e Engine, /// Input stream buffer containing the next character to read. - buffer: Shared>>, + tokenizer_control: TokenizerControl, /// Interned strings. interned_strings: IdentifierBuilder, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. @@ -76,22 +77,15 @@ pub struct ParseState<'e> { impl<'e> ParseState<'e> { /// Create a new [`ParseState`]. #[inline(always)] - pub fn new( - engine: &'e Engine, - buffer: Shared>>, - #[cfg(not(feature = "unchecked"))] max_expr_depth: Option, - #[cfg(not(feature = "unchecked"))] - #[cfg(not(feature = "no_function"))] - max_function_expr_depth: Option, - ) -> Self { + pub fn new(engine: &'e Engine, tokenizer_control: TokenizerControl) -> Self { Self { engine, - buffer, + tokenizer_control, #[cfg(not(feature = "unchecked"))] - max_expr_depth, + max_expr_depth: NonZeroUsize::new(engine.max_expr_depth()), #[cfg(not(feature = "unchecked"))] #[cfg(not(feature = "no_function"))] - max_function_expr_depth, + max_function_expr_depth: NonZeroUsize::new(engine.max_function_expr_depth()), #[cfg(not(feature = "no_closure"))] external_vars: Default::default(), #[cfg(not(feature = "no_closure"))] @@ -982,14 +976,8 @@ fn parse_primary( // | ... #[cfg(not(feature = "no_function"))] Token::Pipe | Token::Or if settings.allow_anonymous_fn => { - let mut new_state = ParseState::new( - state.engine, - state.buffer.clone(), - #[cfg(not(feature = "unchecked"))] - state.max_function_expr_depth, - #[cfg(not(feature = "unchecked"))] - state.max_function_expr_depth, - ); + let mut new_state = ParseState::new(state.engine, state.tokenizer_control.clone()); + new_state.max_expr_depth = new_state.max_function_expr_depth; let settings = ParseSettings { allow_if_expr: true, @@ -1034,7 +1022,9 @@ fn parse_primary( segments.push(expr); // Make sure to parse the following as text - state.buffer.set(Some('`')); + let mut control = state.tokenizer_control.get(); + control.is_within_text = true; + state.tokenizer_control.set(control); match input.next().unwrap() { (Token::StringConstant(s), pos) => { @@ -2540,14 +2530,9 @@ fn parse_stmt( match input.next().unwrap() { (Token::Fn, pos) => { - let mut new_state = ParseState::new( - state.engine, - state.buffer.clone(), - #[cfg(not(feature = "unchecked"))] - state.max_function_expr_depth, - #[cfg(not(feature = "unchecked"))] - state.max_function_expr_depth, - ); + let mut new_state = + ParseState::new(state.engine, state.tokenizer_control.clone()); + new_state.max_expr_depth = new_state.max_function_expr_depth; let settings = ParseSettings { allow_if_expr: true, diff --git a/src/token.rs b/src/token.rs index 3a64bd84..7dab498c 100644 --- a/src/token.rs +++ b/src/token.rs @@ -11,10 +11,11 @@ use crate::stdlib::{ iter::{FusedIterator, Peekable}, num::NonZeroUsize, ops::{Add, AddAssign}, + rc::Rc, str::{Chars, FromStr}, string::{String, ToString}, }; -use crate::{Engine, LexError, Shared, StaticVec, INT}; +use crate::{Engine, LexError, StaticVec, INT}; #[cfg(not(feature = "no_float"))] use crate::ast::FloatWrapper; @@ -25,6 +26,17 @@ use rust_decimal::Decimal; #[cfg(not(feature = "no_function"))] use crate::engine::KEYWORD_IS_DEF_FN; +/// A type containing commands to control the tokenizer. +#[derive(Debug, Clone, Eq, PartialEq, Hash, Copy, Default)] +pub struct TokenizeControlBlock { + /// Is the current tokenizer position within an interpolated text string? + /// This flag allows switching the tokenizer back to _text_ parsing after an interpolation stream. + pub is_within_text: bool, +} + +/// A shared object that allows control of the tokenizer from outside. +pub type TokenizerControl = Rc>; + type LERR = LexError; /// Separator character for numbers. @@ -849,6 +861,9 @@ pub trait InputStream { /// _(INTERNALS)_ Parse a string literal ended by `termination_char`. /// Exported under the `internals` feature only. /// +/// Returns the parsed string and a boolean indicating whether the string is +/// terminated by an interpolation `${`. +/// /// # Volatile API /// /// This function is volatile and may change. @@ -1840,8 +1855,8 @@ pub struct TokenIterator<'a> { state: TokenizeState, /// Current position. pos: Position, - /// Buffer containing the next character to read, if any. - buffer: Shared>>, + /// External buffer containing the next character to read, if any. + tokenizer_control: TokenizerControl, /// Input character stream. stream: MultiInputsStream<'a>, /// A processor function that maps a token to another. @@ -1852,9 +1867,16 @@ impl<'a> Iterator for TokenIterator<'a> { type Item = (Token, Position); fn next(&mut self) -> Option { - if let Some(ch) = self.buffer.take() { - self.stream.unget(ch); + let mut control = self.tokenizer_control.get(); + + if control.is_within_text { + // Push a back-tick into the stream + self.stream.unget('`'); + // Rewind the current position by one character self.pos.rewind(); + // Reset it + control.is_within_text = false; + self.tokenizer_control.set(control); } let (token, pos) = match get_next_token(&mut self.stream, &mut self.state, &mut self.pos) { @@ -1945,7 +1967,7 @@ impl Engine { pub fn lex<'a>( &'a self, input: impl IntoIterator, - ) -> (TokenIterator<'a>, Shared>>) { + ) -> (TokenIterator<'a>, ExternalBuffer) { self.lex_raw(input, None) } /// _(INTERNALS)_ Tokenize an input text stream with a mapping function. @@ -1956,7 +1978,7 @@ impl Engine { &'a self, input: impl IntoIterator, map: fn(Token) -> Token, - ) -> (TokenIterator<'a>, Shared>>) { + ) -> (TokenIterator<'a>, ExternalBuffer) { self.lex_raw(input, Some(map)) } /// Tokenize an input text stream with an optional mapping function. @@ -1965,8 +1987,8 @@ impl Engine { &'a self, input: impl IntoIterator, map: Option Token>, - ) -> (TokenIterator<'a>, Shared>>) { - let buffer: Shared>> = Cell::new(None).into(); + ) -> (TokenIterator<'a>, TokenizerControl) { + let buffer: TokenizerControl = Default::default(); let buffer2 = buffer.clone(); ( @@ -1984,7 +2006,7 @@ impl Engine { disable_doc_comments: self.disable_doc_comments, }, pos: Position::new(1, 0), - buffer, + tokenizer_control: buffer, stream: MultiInputsStream { buf: None, streams: input.into_iter().map(|s| s.chars().peekable()).collect(), From 0807c474a1dd86b5ffdb8e099c366303bfdcb80e Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Apr 2021 23:22:45 +0800 Subject: [PATCH 03/15] Revise using string interpolation. --- CHANGELOG.md | 2 +- benches/parsing.rs | 4 ++-- examples/threading.rs | 4 ++-- scripts/fibonacci.rhai | 4 ++-- scripts/for1.rhai | 2 +- scripts/for2.rhai | 6 +++--- scripts/mat_mul.rhai | 2 +- scripts/oop.rhai | 4 ++-- scripts/primes.rhai | 4 ++-- scripts/speed_test.rhai | 2 +- scripts/string.rhai | 7 ++++--- scripts/strings_map.rhai | 6 +++--- src/ast.rs | 12 ++++++------ src/fn_args.rs | 2 +- tests/call_fn.rs | 2 +- tests/modules.rs | 2 +- 16 files changed, 33 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99e9987f..e53f2148 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Rhai Release Notes ================== -This version adds string interpolation. +This version adds string interpolation with `` `... ${`` ... ``} ...` `` syntax. Version 0.19.16 =============== diff --git a/benches/parsing.rs b/benches/parsing.rs index 6bde9dd1..d0f7aced 100644 --- a/benches/parsing.rs +++ b/benches/parsing.rs @@ -93,8 +93,8 @@ fn bench_parse_primes(bench: &mut Bencher) { } } - print("Total " + total_primes_found + " primes."); - print("Run time = " + now.elapsed + " seconds."); + print(`Total ${total_primes_found} primes.`); + print(`Run time = ${now.elapsed} seconds.`); "#; let mut engine = Engine::new(); diff --git a/examples/threading.rs b/examples/threading.rs index 2e0aa9d3..ad80aedb 100644 --- a/examples/threading.rs +++ b/examples/threading.rs @@ -40,9 +40,9 @@ fn main() { loop { let x = get(); - print("Script Read: " + x); + print(`Script Read: ${x}`); x += 1; - print("Script Write: " + x); + print(`Script Write: ${x}`); put(x); } "#, diff --git a/scripts/fibonacci.rhai b/scripts/fibonacci.rhai index 14a35d5e..fb40c64a 100644 --- a/scripts/fibonacci.rhai +++ b/scripts/fibonacci.rhai @@ -21,9 +21,9 @@ for n in range(0, 5) { result = fib(target); } -print("Finished. Run time = " + now.elapsed + " seconds."); +print(`Finished. Run time = ${now.elapsed} seconds.`); -print("Fibonacci number #" + target + " = " + result); +print(`Fibonacci number #${target} = ${result}`); if result != 317_811 { print("The answer is WRONG! Should be 317,811!"); diff --git a/scripts/for1.rhai b/scripts/for1.rhai index 345d17a7..39ac7009 100644 --- a/scripts/for1.rhai +++ b/scripts/for1.rhai @@ -4,7 +4,7 @@ let arr = [1, 2, 3, 4]; for a in arr { for b in [10, 20] { - print(a + "," + b); + print(`${a}, ${b}`); } if a == 3 { break; } diff --git a/scripts/for2.rhai b/scripts/for2.rhai index 8547e7d5..c7b6b816 100644 --- a/scripts/for2.rhai +++ b/scripts/for2.rhai @@ -1,6 +1,6 @@ const MAX = 1_000_000; -print("Iterating an array with " + MAX + " items..."); +print(`Iterating an array with ${MAX} items...`); print("Ready... Go!"); @@ -18,5 +18,5 @@ for i in list { sum += i; } -print("Sum = " + sum); -print("Finished. Run time = " + now.elapsed + " seconds."); +print(`Sum = ${sum}`); +print(`Finished. Run time = ${now.elapsed} seconds.`); diff --git a/scripts/mat_mul.rhai b/scripts/mat_mul.rhai index 54bca636..9fed0d77 100644 --- a/scripts/mat_mul.rhai +++ b/scripts/mat_mul.rhai @@ -68,4 +68,4 @@ for i in range(0, SIZE) { } */ -print("Finished. Run time = " + now.elapsed + " seconds."); +print(`Finished. Run time = ${now.elapsed} seconds.`); diff --git a/scripts/oop.rhai b/scripts/oop.rhai index e90d690b..8caf6d87 100644 --- a/scripts/oop.rhai +++ b/scripts/oop.rhai @@ -7,7 +7,7 @@ let last_value = (); let obj1 = #{ _data: 42, // data field get_data: || this._data, // property getter - action: || print("Data=" + this._data), // method + action: || print(`Data=${this._data}`), // method update: |x| { // property setter this._data = x; last_value = this._data; // capture 'last_value' @@ -38,4 +38,4 @@ if obj2.get_data() > 0 { // property access obj2.update(42); // call method } -print("Should be 84: " + last_value); +print(`Should be 84: ${last_value}`); diff --git a/scripts/primes.rhai b/scripts/primes.rhai index 25fa0690..f5b65763 100644 --- a/scripts/primes.rhai +++ b/scripts/primes.rhai @@ -25,8 +25,8 @@ for p in range(2, MAX_NUMBER_TO_CHECK) { } } -print("Total " + total_primes_found + " primes <= " + MAX_NUMBER_TO_CHECK); -print("Run time = " + now.elapsed + " seconds."); +print(`Total ${total_primes_found} primes <= ${MAX_NUMBER_TO_CHECK}`); +print(`Run time = ${now.elapsed} seconds.`); if total_primes_found != 78_498 { print("The answer is WRONG! Should be 78,498!"); diff --git a/scripts/speed_test.rhai b/scripts/speed_test.rhai index edb9bd0e..d07dadf3 100644 --- a/scripts/speed_test.rhai +++ b/scripts/speed_test.rhai @@ -10,4 +10,4 @@ while x > 0 { x -= 1; } -print("Finished. Run time = " + now.elapsed + " seconds."); +print(`Finished. Run time = ${now.elapsed} seconds.`); diff --git a/scripts/string.rhai b/scripts/string.rhai index 0bb799c5..a507648c 100644 --- a/scripts/string.rhai +++ b/scripts/string.rhai @@ -11,17 +11,18 @@ print("foo" >= "bar"); // string comparison print("the answer is " + 42); // string building using non-string types let s = "\u2764 hello, world! \U0001F603"; // string variable -print("length=" + s.len); // should be 17 +print(`length=${s.len}`); // should be 17 s[s.len-3] = '?'; // change the string -print("Question: " + s); // should print 'Question: hello, world?' +print(`Question: ${s}`); // should print 'Question: hello, world?' // Line continuation: let s = "This is a long \ string constructed using \ line continuation"; -print("One string: " + s); +// String interpolation +print(`One string: ${s}`); // Multi-line literal string: let s = ` diff --git a/scripts/strings_map.rhai b/scripts/strings_map.rhai index 04cd81db..da0dbd87 100644 --- a/scripts/strings_map.rhai +++ b/scripts/strings_map.rhai @@ -75,7 +75,7 @@ let keys = []; for animal in animals { for adjective in adjectives { for adverb in adverbs { - keys.push(adverb + " " + adjective + " " + animal) + keys.push(`${adverb} ${adjective} ${animal}`) } } } @@ -99,5 +99,5 @@ for key in keys { map.remove(key); } -print("Sum = " + sum); -print("Finished. Run time = " + now.elapsed + " seconds."); +print(`Sum = ${sum}`); +print(`Finished. Run time = ${now.elapsed} seconds.`); diff --git a/src/ast.rs b/src/ast.rs index b5dd9518..6335ba31 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -384,7 +384,7 @@ impl AST { /// "#)?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { "hello" + n } + /// fn foo(n) { `hello${n}` } /// foo("!") /// "#)?; /// @@ -395,7 +395,7 @@ impl AST { /// /// // 'ast' is essentially: /// // - /// // fn foo(n) { "hello" + n } // <- definition of first 'foo' is overwritten + /// // fn foo(n) { `hello${n}` } // <- definition of first 'foo' is overwritten /// // foo(1) // <- notice this will be "hello1" instead of 43, /// // // but it is no longer the return value /// // foo("!") // returns "hello!" @@ -436,7 +436,7 @@ impl AST { /// "#)?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { "hello" + n } + /// fn foo(n) { `hello${n}` } /// foo("!") /// "#)?; /// @@ -447,7 +447,7 @@ impl AST { /// /// // 'ast1' is essentially: /// // - /// // fn foo(n) { "hello" + n } // <- definition of first 'foo' is overwritten + /// // fn foo(n) { `hello${n}` } // <- definition of first 'foo' is overwritten /// // foo(1) // <- notice this will be "hello1" instead of 43, /// // // but it is no longer the return value /// // foo("!") // returns "hello!" @@ -490,7 +490,7 @@ impl AST { /// "#)?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { "hello" + n } + /// fn foo(n) { `hello${n}` } /// fn error() { 0 } /// foo("!") /// "#)?; @@ -574,7 +574,7 @@ impl AST { /// "#)?; /// /// let ast2 = engine.compile(r#" - /// fn foo(n) { "hello" + n } + /// fn foo(n) { `hello${n}` } /// fn error() { 0 } /// foo("!") /// "#)?; diff --git a/src/fn_args.rs b/src/fn_args.rs index 13dabf2f..6b3baf78 100644 --- a/src/fn_args.rs +++ b/src/fn_args.rs @@ -41,7 +41,7 @@ pub trait FuncArgs { /// /// let ast = engine.compile(r#" /// fn hello(x, y, z) { - /// if x { "hello " + y } else { y + z } + /// if x { `hello ${y}` } else { y + z } /// } /// "#)?; /// diff --git a/tests/call_fn.rs b/tests/call_fn.rs index fdf82e88..07fdefda 100644 --- a/tests/call_fn.rs +++ b/tests/call_fn.rs @@ -81,7 +81,7 @@ fn test_call_fn_args() -> Result<(), Box> { let ast = engine.compile( r#" fn hello(x, y, z) { - if x { "hello " + y } else { y + z } + if x { `hello ${y}` } else { y + z } } "#, )?; diff --git a/tests/modules.rs b/tests/modules.rs index 64401696..03a2b477 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -315,7 +315,7 @@ fn test_module_from_ast() -> Result<(), Box> { // Final variable values become constant module variable values foo = calc(foo); - hello = "hello, " + foo + " worlds!"; + hello = `hello, ${foo} worlds!`; export x as abc, From 8956a77c8c4aa43b977dc2cfeb4030e74af30b22 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sun, 4 Apr 2021 23:23:10 +0800 Subject: [PATCH 04/15] Add new state in TokenizeState to switch back to text mode. --- src/token.rs | 113 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 45 deletions(-) diff --git a/src/token.rs b/src/token.rs index 7dab498c..55124fa6 100644 --- a/src/token.rs +++ b/src/token.rs @@ -9,6 +9,7 @@ use crate::stdlib::{ cell::Cell, char, fmt, format, iter::{FusedIterator, Peekable}, + mem, num::NonZeroUsize, ops::{Add, AddAssign}, rc::Rc, @@ -839,6 +840,8 @@ pub struct TokenizeState { pub include_comments: bool, /// Disable doc-comments? pub disable_doc_comments: bool, + /// Is the current tokenizer position within the text stream of an interpolated string? + pub is_within_text_terminated_by: Option, } /// _(INTERNALS)_ Trait that encapsulates a peekable character input stream. @@ -874,6 +877,7 @@ pub fn parse_string_literal( termination_char: char, continuation: bool, verbatim: bool, + skip_first_new_line: bool, allow_interpolation: bool, ) -> Result<(String, bool), (LexError, Position)> { let mut result: smallvec::SmallVec<[char; 16]> = Default::default(); @@ -883,6 +887,25 @@ pub fn parse_string_literal( let mut skip_whitespace_until = 0; let mut interpolated = false; + if skip_first_new_line { + // Start from the next line if at the end of line + match stream.peek_next() { + // `\r - start from next line + Some('\r') => { + eat_next(stream, pos); + // `\r\n + if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) { + eat_next(stream, pos); + } + } + // `\n - start from next line + Some('\n') => { + eat_next(stream, pos); + } + _ => (), + } + } + loop { let next_char = stream.get_next().ok_or((LERR::UnterminatedString, start))?; @@ -1163,6 +1186,22 @@ fn get_next_token_inner( } } + // Within text? + if let Some(ch) = mem::take(&mut state.is_within_text_terminated_by) { + let start_pos = *pos; + + return parse_string_literal(stream, state, pos, ch, false, true, true, true).map_or_else( + |(err, err_pos)| Some((Token::LexError(err), err_pos)), + |(result, interpolated)| { + if interpolated { + Some((Token::InterpolatedString(result), start_pos)) + } else { + Some((Token::StringConstant(result), start_pos)) + } + }, + ); + } + let mut negated = false; while let Some(c) = stream.get_next() { @@ -1327,41 +1366,25 @@ fn get_next_token_inner( // " - string literal ('"', _) => { - return parse_string_literal(stream, state, pos, c, true, false, false) + return parse_string_literal(stream, state, pos, c, true, false, false, false) .map_or_else( - |err| Some((Token::LexError(err.0), err.1)), + |(err, err_pos)| Some((Token::LexError(err), err_pos)), |(result, _)| Some((Token::StringConstant(result), start_pos)), ); } // ` - string literal ('`', _) => { - // Start from the next line if ` at the end of line - match stream.peek_next() { - // `\r - start from next line - Some('\r') => { - eat_next(stream, pos); - // `\r\n - if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) { - eat_next(stream, pos); - } - } - // `\n - start from next line - Some('\n') => { - eat_next(stream, pos); - } - _ => (), - } - - return parse_string_literal(stream, state, pos, c, false, true, true).map_or_else( - |err| Some((Token::LexError(err.0), err.1)), - |(result, interpolated)| { - if interpolated { - Some((Token::InterpolatedString(result), start_pos)) - } else { - Some((Token::StringConstant(result), start_pos)) - } - }, - ); + return parse_string_literal(stream, state, pos, c, false, true, true, true) + .map_or_else( + |(err, err_pos)| Some((Token::LexError(err), err_pos)), + |(result, interpolated)| { + if interpolated { + Some((Token::InterpolatedString(result), start_pos)) + } else { + Some((Token::StringConstant(result), start_pos)) + } + }, + ); } // ' - character literal @@ -1373,19 +1396,20 @@ fn get_next_token_inner( } ('\'', _) => { return Some( - parse_string_literal(stream, state, pos, c, false, false, false).map_or_else( - |err| (Token::LexError(err.0), err.1), - |(result, _)| { - let mut chars = result.chars(); - let first = chars.next().unwrap(); + parse_string_literal(stream, state, pos, c, false, false, false, false) + .map_or_else( + |(err, err_pos)| (Token::LexError(err), err_pos), + |(result, _)| { + let mut chars = result.chars(); + let first = chars.next().unwrap(); - if chars.next().is_some() { - (Token::LexError(LERR::MalformedChar(result)), start_pos) - } else { - (Token::CharConstant(first), start_pos) - } - }, - ), + if chars.next().is_some() { + (Token::LexError(LERR::MalformedChar(result)), start_pos) + } else { + (Token::CharConstant(first), start_pos) + } + }, + ), ) } @@ -1870,10 +1894,8 @@ impl<'a> Iterator for TokenIterator<'a> { let mut control = self.tokenizer_control.get(); if control.is_within_text { - // Push a back-tick into the stream - self.stream.unget('`'); - // Rewind the current position by one character - self.pos.rewind(); + // Switch to text mode terminated by back-tick + self.state.is_within_text_terminated_by = Some('`'); // Reset it control.is_within_text = false; self.tokenizer_control.set(control); @@ -2004,6 +2026,7 @@ impl Engine { end_with_none: false, include_comments: false, disable_doc_comments: self.disable_doc_comments, + is_within_text_terminated_by: None, }, pos: Position::new(1, 0), tokenizer_control: buffer, From e6ea006ac63637b451d7e29a3921cba2c51cd079 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 00:05:56 +0800 Subject: [PATCH 05/15] Fix builds. --- src/lib.rs | 5 ++++- src/parser.rs | 12 ++++++++++-- src/token.rs | 12 ++++++------ tests/functions.rs | 2 +- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 146a3717..f7e97d1d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -211,7 +211,10 @@ pub use dynamic::Variant; // Expose internal data structures. #[cfg(feature = "internals")] #[deprecated = "this type is volatile and may change"] -pub use token::{get_next_token, parse_string_literal, InputStream, Token, TokenizeState}; +pub use token::{ + get_next_token, parse_string_literal, InputStream, Token, TokenizeState, TokenizerControl, + TokenizerControlBlock, +}; #[cfg(feature = "internals")] #[deprecated = "this type is volatile and may change"] diff --git a/src/parser.rs b/src/parser.rs index 25622144..83f5df33 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -977,7 +977,11 @@ fn parse_primary( #[cfg(not(feature = "no_function"))] Token::Pipe | Token::Or if settings.allow_anonymous_fn => { let mut new_state = ParseState::new(state.engine, state.tokenizer_control.clone()); - new_state.max_expr_depth = new_state.max_function_expr_depth; + + #[cfg(not(feature = "unchecked"))] + { + new_state.max_expr_depth = new_state.max_function_expr_depth; + } let settings = ParseSettings { allow_if_expr: true, @@ -2532,7 +2536,11 @@ fn parse_stmt( (Token::Fn, pos) => { let mut new_state = ParseState::new(state.engine, state.tokenizer_control.clone()); - new_state.max_expr_depth = new_state.max_function_expr_depth; + + #[cfg(not(feature = "unchecked"))] + { + new_state.max_expr_depth = new_state.max_function_expr_depth; + } let settings = ParseSettings { allow_if_expr: true, diff --git a/src/token.rs b/src/token.rs index 55124fa6..389ad8f7 100644 --- a/src/token.rs +++ b/src/token.rs @@ -27,16 +27,16 @@ use rust_decimal::Decimal; #[cfg(not(feature = "no_function"))] use crate::engine::KEYWORD_IS_DEF_FN; -/// A type containing commands to control the tokenizer. +/// _(INTERNALS)_ A type containing commands to control the tokenizer. #[derive(Debug, Clone, Eq, PartialEq, Hash, Copy, Default)] -pub struct TokenizeControlBlock { +pub struct TokenizerControlBlock { /// Is the current tokenizer position within an interpolated text string? /// This flag allows switching the tokenizer back to _text_ parsing after an interpolation stream. pub is_within_text: bool, } -/// A shared object that allows control of the tokenizer from outside. -pub type TokenizerControl = Rc>; +/// _(INTERNALS)_ A shared object that allows control of the tokenizer from outside. +pub type TokenizerControl = Rc>; type LERR = LexError; @@ -1989,7 +1989,7 @@ impl Engine { pub fn lex<'a>( &'a self, input: impl IntoIterator, - ) -> (TokenIterator<'a>, ExternalBuffer) { + ) -> (TokenIterator<'a>, TokenizerControl) { self.lex_raw(input, None) } /// _(INTERNALS)_ Tokenize an input text stream with a mapping function. @@ -2000,7 +2000,7 @@ impl Engine { &'a self, input: impl IntoIterator, map: fn(Token) -> Token, - ) -> (TokenIterator<'a>, ExternalBuffer) { + ) -> (TokenIterator<'a>, TokenizerControl) { self.lex_raw(input, Some(map)) } /// Tokenize an input text stream with an optional mapping function. diff --git a/tests/functions.rs b/tests/functions.rs index e6b56245..05805a9e 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -1,5 +1,5 @@ #![cfg(not(feature = "no_function"))] -use rhai::{Engine, EvalAltResult, FnNamespace, Module, ParseErrorType, Shared, INT}; +use rhai::{Engine, EvalAltResult, FnNamespace, Module, Shared, INT}; #[cfg(not(feature = "no_object"))] #[test] From 26bb88974a111f7518ace6eb7c02f156ee44091d Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 00:10:08 +0800 Subject: [PATCH 06/15] Add function for string + char. --- src/packages/string_more.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index e2e533e3..109549fd 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -56,6 +56,10 @@ mod string_functions { pub fn add_append_str(string1: ImmutableString, string2: ImmutableString) -> ImmutableString { string1 + string2 } + #[rhai_fn(name = "+", name = "append")] + pub fn add_append_char(string: ImmutableString, ch: char) -> ImmutableString { + string + ch + } #[rhai_fn(name = "+", name = "append")] pub fn add_append_unit(string: ImmutableString, _item: ()) -> ImmutableString { From 00784d39ad9a990c77f5facae58f7db108328876 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 14:51:26 +0800 Subject: [PATCH 07/15] PropertyExpected for map literal with interpolated key. --- src/parser.rs | 1 + tests/maps.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index 83f5df33..1ba1b130 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -710,6 +710,7 @@ fn parse_map_literal( } (s, pos) } + (Token::InterpolatedString(_), pos) => return Err(PERR::PropertyExpected.into_err(pos)), (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); } diff --git a/tests/maps.rs b/tests/maps.rs index 895c7310..30dcf985 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -35,8 +35,21 @@ fn test_map_indexing() -> Result<(), Box> { engine.eval::("let y = #{a: 1, b: 2, c: 3}; y.a = 5; y.a")?, 5 ); + engine.eval::<()>("let y = #{a: 1, b: 2, c: 3}; y.z")?; + assert_eq!( + engine.eval::(r#"let y = #{`a\nb`: 1}; y["a\\nb"]"#)?, + 1 + ); + + assert!(matches!( + *engine + .eval::("let y = #{`a${1}`: 1}; y.a1") + .expect_err("should error"), + EvalAltResult::ErrorParsing(ParseErrorType::PropertyExpected, _) + )); + assert!(engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "c" in y"#)?); assert!(engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "b" in y"#)?); assert!(!engine.eval::(r#"let y = #{a: 1, b: 2, c: 3}; "z" in y"#)?); From f5d3a0ef4faa5f76077c24e74a865844b1e69d58 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 14:57:07 +0800 Subject: [PATCH 08/15] Promote expr block. --- src/optimize.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/optimize.rs b/src/optimize.rs index ea30d5cf..6d8edc6a 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -546,19 +546,21 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { Stmt::Import(expr, _, _) => optimize_expr(expr, state), // { block } Stmt::Block(statements, pos) => { - let block = mem::take(statements); - *stmt = match optimize_stmt_block(block, state, preserve_result, true, false) { - statements if statements.is_empty() => { + let mut block = + optimize_stmt_block(mem::take(statements), state, preserve_result, true, false); + + match block.as_mut_slice() { + [] => { state.set_dirty(); - Stmt::Noop(*pos) + *stmt = Stmt::Noop(*pos); } // Only one statement - promote - mut statements if statements.len() == 1 => { + [s] => { state.set_dirty(); - statements.pop().unwrap() + *stmt = mem::take(s); } - statements => Stmt::Block(statements, *pos), - }; + _ => *stmt = Stmt::Block(block, *pos), + } } // try { pure try_block } catch ( var ) { catch_block } -> try_block Stmt::TryCatch(x, _, _) if x.0.statements.iter().all(Stmt::is_pure) => { @@ -609,18 +611,16 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { match expr { // {} Expr::Stmt(x) if x.statements.is_empty() => { state.set_dirty(); *expr = Expr::Unit(x.pos) } - // { Stmt(Expr) } - Expr::Stmt(x) if x.statements.len() == 1 && x.statements[0].is_pure() && matches!(x.statements[0], Stmt::Expr(_)) => - { - state.set_dirty(); - if let Stmt::Expr(e) = mem::take(&mut x.statements[0]) { - *expr = e; - } else { - unreachable!(); + // { stmt; ... } - do not count promotion as dirty because it gets turned back into an array + Expr::Stmt(x) => { + x.statements = optimize_stmt_block(mem::take(&mut x.statements).into_vec(), state, true, true, false).into(); + + // { Stmt(Expr) } - promote + match x.statements.as_mut() { + [ Stmt::Expr(e) ] => { state.set_dirty(); *expr = mem::take(e); } + _ => () } } - // { stmt; ... } - do not count promotion as dirty because it gets turned back into an array - Expr::Stmt(x) => x.statements = optimize_stmt_block(mem::take(&mut x.statements).into_vec(), state, true, true, false).into(), // lhs.rhs #[cfg(not(feature = "no_object"))] Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { From a3ee0f4245281925e40e18603f7603cf088b510a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 18:32:20 +0800 Subject: [PATCH 09/15] Fix no_index build. --- src/engine.rs | 12 ++++++------ tests/maps.rs | 8 +++++++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index ab081435..21400e02 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1562,7 +1562,7 @@ impl Engine { state: &mut State, _lib: &[&Module], target: &'t mut Dynamic, - mut idx: Dynamic, + mut _idx: Dynamic, idx_pos: Position, _create: bool, _is_ref: bool, @@ -1575,7 +1575,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Dynamic(Union::Array(arr, _)) => { // val_array[idx] - let index = idx + let index = _idx .as_int() .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; @@ -1595,8 +1595,8 @@ impl Engine { #[cfg(not(feature = "no_object"))] Dynamic(Union::Map(map, _)) => { // val_map[idx] - let index = &*idx.read_lock::().ok_or_else(|| { - self.make_type_mismatch_err::(idx.type_name(), idx_pos) + let index = &*_idx.read_lock::().ok_or_else(|| { + self.make_type_mismatch_err::(_idx.type_name(), idx_pos) })?; if _create && !map.contains_key(index.as_str()) { @@ -1613,7 +1613,7 @@ impl Engine { Dynamic(Union::Str(s, _)) => { // val_string[idx] let chars_len = s.chars().count(); - let index = idx + let index = _idx .as_int() .map_err(|err| self.make_type_mismatch_err::(err, idx_pos))?; @@ -1631,7 +1631,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] _ if _indexers => { let type_name = target.type_name(); - let args = &mut [target, &mut idx]; + let args = &mut [target, &mut _idx]; let hash_get = FnCallHash::from_native(calc_fn_hash(empty(), FN_IDX_GET, 2)); self.exec_fn_call( _mods, state, _lib, FN_IDX_GET, hash_get, args, _is_ref, true, idx_pos, None, diff --git a/tests/maps.rs b/tests/maps.rs index 30dcf985..170aed90 100644 --- a/tests/maps.rs +++ b/tests/maps.rs @@ -38,8 +38,14 @@ fn test_map_indexing() -> Result<(), Box> { engine.eval::<()>("let y = #{a: 1, b: 2, c: 3}; y.z")?; + #[cfg(not(feature = "no_index"))] assert_eq!( - engine.eval::(r#"let y = #{`a\nb`: 1}; y["a\\nb"]"#)?, + engine.eval::( + r#" + let y = #{`a +b`: 1}; y["a\nb"] + "# + )?, 1 ); From 94fc5af285cf6049f76a23c81de71001edfbd8a9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 23:06:48 +0800 Subject: [PATCH 10/15] Store short index in variable access. --- src/ast.rs | 23 +++++++++++------- src/engine.rs | 35 +++++++++++++++++---------- src/fn_call.rs | 4 ++-- src/optimize.rs | 8 +++---- src/parser.rs | 64 +++++++++++++++++++++++++++---------------------- src/token.rs | 55 +++++++++++++++++++++--------------------- 6 files changed, 106 insertions(+), 83 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 6335ba31..b3eda699 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -9,7 +9,7 @@ use crate::stdlib::{ fmt, hash::Hash, iter::empty, - num::NonZeroUsize, + num::{NonZeroU8, NonZeroUsize}, ops::{Add, AddAssign}, string::String, vec, @@ -1574,8 +1574,15 @@ pub enum Expr { ), /// () Unit(Position), - /// Variable access - (optional index, optional (hash, modules), variable name) - Variable(Box<(Option, Option<(u64, NamespaceRef)>, Ident)>), + /// Variable access - optional short index, (optional index, optional (hash, modules), variable name) + /// + /// The short index is [`u8`] which is used when the index is <= 255, which should be the vast + /// majority of cases (unless there are more than 255 variables defined!). + /// This is to avoid reading a pointer redirection during each variable access. + Variable( + Option, + Box<(Option, Option<(u64, NamespaceRef)>, Ident)>, + ), /// Property access - ((getter, hash), (setter, hash), prop) Property(Box<((Identifier, u64), (Identifier, u64), Ident)>), /// { [statement][Stmt] ... } @@ -1644,7 +1651,7 @@ impl Expr { #[inline(always)] pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { match self { - Self::Variable(x) if !non_qualified || x.1.is_none() => Some((x.2).name.as_str()), + Self::Variable(_, x) if !non_qualified || x.1.is_none() => Some((x.2).name.as_str()), _ => None, } } @@ -1666,7 +1673,7 @@ impl Expr { Self::Map(_, pos) => *pos, Self::Property(x) => (x.2).pos, Self::Stmt(x) => x.pos, - Self::Variable(x) => (x.2).pos, + Self::Variable(_, x) => (x.2).pos, Self::FnCall(_, pos) => *pos, Self::And(x, _) | Self::Or(x, _) => x.lhs.position(), @@ -1696,7 +1703,7 @@ impl Expr { Self::FnPointer(_, pos) => *pos = new_pos, Self::Array(_, pos) => *pos = new_pos, Self::Map(_, pos) => *pos = new_pos, - Self::Variable(x) => (x.2).pos = new_pos, + Self::Variable(_, x) => (x.2).pos = new_pos, Self::Property(x) => (x.2).pos = new_pos, Self::Stmt(x) => x.pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, @@ -1724,7 +1731,7 @@ impl Expr { Self::Stmt(x) => x.statements.iter().all(Stmt::is_pure), - Self::Variable(_) => true, + Self::Variable(_, _) => true, _ => self.is_constant(), } @@ -1794,7 +1801,7 @@ impl Expr { _ => false, }, - Self::Variable(_) => match token { + Self::Variable(_, _) => match token { #[cfg(not(feature = "no_index"))] Token::LeftBracket => true, Token::LeftParen => true, diff --git a/src/engine.rs b/src/engine.rs index 21400e02..aeb0c16b 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -959,7 +959,12 @@ impl Engine { expr: &Expr, ) -> Result<(Target<'s>, Position), Box> { match expr { - Expr::Variable(v) => match v.as_ref() { + Expr::Variable(Some(_), _) => { + self.search_scope_only(scope, mods, state, lib, this_ptr, expr) + } + Expr::Variable(None, v) => match v.as_ref() { + // Normal variable access + (_, None, _) => self.search_scope_only(scope, mods, state, lib, this_ptr, expr), // Qualified variable (_, Some((hash_var, modules)), Ident { name, pos, .. }) => { let module = self.search_imports(mods, state, modules).ok_or_else(|| { @@ -983,8 +988,6 @@ impl Engine { target.set_access_mode(AccessMode::ReadOnly); Ok((target.into(), *pos)) } - // Normal variable access - _ => self.search_scope_only(scope, mods, state, lib, this_ptr, expr), }, _ => unreachable!("Expr::Variable expected, but gets {:?}", expr), } @@ -1000,8 +1003,8 @@ impl Engine { this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, ) -> Result<(Target<'s>, Position), Box> { - let (index, _, Ident { name, pos, .. }) = match expr { - Expr::Variable(v) => v.as_ref(), + let (short_index, (index, _, Ident { name, pos, .. })) = match expr { + Expr::Variable(i, v) => (i, v.as_ref()), _ => unreachable!("Expr::Variable expected, but gets {:?}", expr), }; @@ -1015,11 +1018,17 @@ impl Engine { } // Check if it is directly indexed - let index = if state.always_search { &None } else { index }; + let index = if state.always_search { + 0 + } else { + short_index.map_or_else( + || index.map(NonZeroUsize::get).unwrap_or(0), + |x| x.get() as usize, + ) + }; // Check the variable resolver, if any if let Some(ref resolve_var) = self.resolve_var { - let index = index.map(NonZeroUsize::get).unwrap_or(0); let context = EvalContext { engine: self, scope, @@ -1037,8 +1046,8 @@ impl Engine { } } - let index = if let Some(index) = index { - scope.len() - index.get() + let index = if index > 0 { + scope.len() - index } else { // Find the variable in the scope scope @@ -1401,7 +1410,7 @@ impl Engine { match lhs { // id.??? or id[???] - Expr::Variable(x) => { + Expr::Variable(_, x) => { let Ident { name: var_name, pos: var_pos, @@ -1679,11 +1688,11 @@ impl Engine { Expr::CharConstant(x, _) => Ok((*x).into()), Expr::FnPointer(x, _) => Ok(FnPtr::new_unchecked(x.clone(), Default::default()).into()), - Expr::Variable(x) if (x.2).name == KEYWORD_THIS => this_ptr + Expr::Variable(None, x) if x.0.is_none() && (x.2).name == KEYWORD_THIS => this_ptr .as_deref() .cloned() .ok_or_else(|| EvalAltResult::ErrorUnboundThis((x.2).pos).into()), - Expr::Variable(_) => self + Expr::Variable(_, _) => self .search_namespace(scope, mods, state, lib, this_ptr, expr) .map(|(val, _)| val.take_or_clone()), @@ -2061,7 +2070,7 @@ impl Engine { // Must be either `var[index] op= val` or `var.prop op= val` match lhs_expr { // name op= rhs (handled above) - Expr::Variable(_) => { + Expr::Variable(_, _) => { unreachable!("Expr::Variable case should already been handled") } // idx_lhs[idx_expr] op= rhs diff --git a/src/fn_call.rs b/src/fn_call.rs index f323632b..ccf67349 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -750,7 +750,7 @@ impl Engine { // Method call of script function - map first argument to `this` let (first, rest) = args.split_first_mut().unwrap(); - let orig_source = mem::take(&mut state.source); + let orig_source = state.source.take(); state.source = source; let level = _level + 1; @@ -780,7 +780,7 @@ impl Engine { backup.as_mut().unwrap().change_first_arg_to_copy(args); } - let orig_source = mem::take(&mut state.source); + let orig_source = state.source.take(); state.source = source; let level = _level + 1; diff --git a/src/optimize.rs b/src/optimize.rs index 6d8edc6a..796af57f 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -385,7 +385,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { match stmt { // expr op= expr Stmt::Assignment(x, _) => match x.0 { - Expr::Variable(_) => optimize_expr(&mut x.2, state), + Expr::Variable(_, _) => optimize_expr(&mut x.2, state), _ => { optimize_expr(&mut x.0, state); optimize_expr(&mut x.2, state); @@ -635,7 +635,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { .unwrap_or_else(|| Expr::Unit(*pos)); } // var.rhs - (Expr::Variable(_), rhs) => optimize_expr(rhs, state), + (Expr::Variable(_, _), rhs) => optimize_expr(rhs, state), // lhs.rhs (lhs, rhs) => { optimize_expr(lhs, state); optimize_expr(rhs, state); } } @@ -670,7 +670,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { *expr = Expr::CharConstant(s.chars().nth(*i as usize).unwrap(), *pos); } // var[rhs] - (Expr::Variable(_), rhs) => optimize_expr(rhs, state), + (Expr::Variable(_, _), rhs) => optimize_expr(rhs, state), // lhs[rhs] (lhs, rhs) => { optimize_expr(lhs, state); optimize_expr(rhs, state); } }, @@ -901,7 +901,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { } // constant-name - Expr::Variable(x) if x.1.is_none() && state.find_constant(&x.2.name).is_some() => { + Expr::Variable(_, x) if x.1.is_none() && state.find_constant(&x.2.name).is_some() => { state.set_dirty(); // Replace constant with value diff --git a/src/parser.rs b/src/parser.rs index 1ba1b130..f1db5fd8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -15,7 +15,7 @@ use crate::stdlib::{ format, hash::{Hash, Hasher}, iter::empty, - num::NonZeroUsize, + num::{NonZeroU8, NonZeroUsize}, string::{String, ToString}, vec, vec::Vec, @@ -225,7 +225,7 @@ impl Expr { #[inline(always)] fn into_property(self, state: &mut ParseState) -> Self { match self { - Self::Variable(x) if x.1.is_none() => { + Self::Variable(_, x) if x.1.is_none() => { let ident = x.2; let getter = state.get_identifier(crate::engine::make_getter(&ident.name)); let hash_get = calc_fn_hash(empty(), &getter, 1); @@ -1081,7 +1081,7 @@ fn parse_primary( name: state.get_identifier(s), pos: settings.pos, }; - Expr::Variable(Box::new((None, None, var_name_def))) + Expr::Variable(None, Box::new((None, None, var_name_def))) } // Namespace qualification #[cfg(not(feature = "no_module"))] @@ -1095,7 +1095,7 @@ fn parse_primary( name: state.get_identifier(s), pos: settings.pos, }; - Expr::Variable(Box::new((None, None, var_name_def))) + Expr::Variable(None, Box::new((None, None, var_name_def))) } // Normal variable access _ => { @@ -1104,7 +1104,14 @@ fn parse_primary( name: state.get_identifier(s), pos: settings.pos, }; - Expr::Variable(Box::new((index, None, var_name_def))) + let short_index = index.and_then(|x| { + if x.get() <= u8::MAX as usize { + NonZeroU8::new(x.get() as u8) + } else { + None + } + }); + Expr::Variable(short_index, Box::new((index, None, var_name_def))) } } } @@ -1123,7 +1130,7 @@ fn parse_primary( name: state.get_identifier(s), pos: settings.pos, }; - Expr::Variable(Box::new((None, None, var_name_def))) + Expr::Variable(None, Box::new((None, None, var_name_def))) } // Access to `this` as a variable is OK within a function scope _ if s == KEYWORD_THIS && settings.is_function_scope => { @@ -1131,7 +1138,7 @@ fn parse_primary( name: state.get_identifier(s), pos: settings.pos, }; - Expr::Variable(Box::new((None, None, var_name_def))) + Expr::Variable(None, Box::new((None, None, var_name_def))) } // Cannot access to `this` as a variable not in a function scope _ if s == KEYWORD_THIS => { @@ -1168,7 +1175,7 @@ fn parse_primary( root_expr = match (root_expr, tail_token) { // Qualified function call with ! - (Expr::Variable(x), Token::Bang) if x.1.is_some() => { + (Expr::Variable(_, x), Token::Bang) if x.1.is_some() => { return Err(if !match_token(input, Token::LeftParen).0 { LexError::UnexpectedInput(Token::Bang.syntax().to_string()).into_err(tail_pos) } else { @@ -1180,7 +1187,7 @@ fn parse_primary( }); } // Function call with ! - (Expr::Variable(x), Token::Bang) => { + (Expr::Variable(_, x), Token::Bang) => { let (matched, pos) = match_token(input, Token::LeftParen); if !matched { return Err(PERR::MissingToken( @@ -1196,31 +1203,30 @@ fn parse_primary( parse_fn_call(input, state, lib, name, true, ns, settings.level_up())? } // Function call - (Expr::Variable(x), Token::LeftParen) => { + (Expr::Variable(_, x), Token::LeftParen) => { let (_, namespace, Ident { name, pos, .. }) = *x; settings.pos = pos; let ns = namespace.map(|(_, ns)| ns); parse_fn_call(input, state, lib, name, false, ns, settings.level_up())? } // module access - (Expr::Variable(x), Token::DoubleColon) => match input.next().unwrap() { + (Expr::Variable(_, x), Token::DoubleColon) => match input.next().unwrap() { (Token::Identifier(id2), pos2) => { - let (index, mut namespace, var_name_def) = *x; + let (_, mut namespace, var_name_def) = *x; if let Some((_, ref mut namespace)) = namespace { namespace.push(var_name_def); } else { let mut ns: NamespaceRef = Default::default(); ns.push(var_name_def); - let index = 42; // Dummy - namespace = Some((index, ns)); + namespace = Some((42, ns)); } let var_name_def = Ident { name: state.get_identifier(id2), pos: pos2, }; - Expr::Variable(Box::new((index, namespace, var_name_def))) + Expr::Variable(None, Box::new((None, namespace, var_name_def))) } (Token::Reserved(id2), pos2) if is_valid_identifier(id2.chars()) => { return Err(PERR::Reserved(id2).into_err(pos2)); @@ -1263,9 +1269,9 @@ fn parse_primary( // Cache the hash key for namespace-qualified variables match &mut root_expr { - Expr::Variable(x) if x.1.is_some() => Some(x), + Expr::Variable(_, x) if x.1.is_some() => Some(x), Expr::Index(x, _) | Expr::Dot(x, _) => match &mut x.lhs { - Expr::Variable(x) if x.1.is_some() => Some(x), + Expr::Variable(_, x) if x.1.is_some() => Some(x), _ => None, }, _ => None, @@ -1423,13 +1429,14 @@ fn make_assignment_stmt<'a>( Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) } // var (non-indexed) = rhs - Expr::Variable(x) if x.0.is_none() => { + Expr::Variable(None, x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } // var (indexed) = rhs - Expr::Variable(x) => { + Expr::Variable(i, x) => { let (index, _, Ident { name, pos, .. }) = x.as_ref(); - match state.stack[(state.stack.len() - index.unwrap().get())].1 { + let index = i.map_or_else(|| index.unwrap().get(), |n| n.get() as usize); + match state.stack[state.stack.len() - index].1 { AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } @@ -1444,13 +1451,14 @@ fn make_assignment_stmt<'a>( match check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(_, _))) { Position::NONE => match &x.lhs { // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(x) if x.0.is_none() => { + Expr::Variable(None, x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } // var[???] (indexed) = rhs, var.??? (indexed) = rhs - Expr::Variable(x) => { + Expr::Variable(i, x) => { let (index, _, Ident { name, pos, .. }) = x.as_ref(); - match state.stack[(state.stack.len() - index.unwrap().get())].1 { + let index = i.map_or_else(|| index.unwrap().get(), |n| n.get() as usize); + match state.stack[state.stack.len() - index].1 { AccessMode::ReadWrite => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } @@ -1532,7 +1540,7 @@ fn make_dot_expr( Expr::Index(x, pos) } // lhs.id - (lhs, Expr::Variable(x)) if x.1.is_none() => { + (lhs, Expr::Variable(_, x)) if x.1.is_none() => { let ident = x.2; let getter = state.get_identifier(crate::engine::make_getter(&ident.name)); let hash_get = calc_fn_hash(empty(), &getter, 1); @@ -1544,7 +1552,7 @@ fn make_dot_expr( Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } // lhs.module::id - syntax error - (_, Expr::Variable(x)) if x.1.is_some() => { + (_, Expr::Variable(_, x)) if x.1.is_some() => { return Err(PERR::PropertyExpected.into_err(x.1.unwrap().1[0].pos)) } // lhs.prop @@ -1553,7 +1561,7 @@ fn make_dot_expr( } // lhs.dot_lhs.dot_rhs (lhs, Expr::Dot(x, pos)) => match x.lhs { - Expr::Variable(_) | Expr::Property(_) => { + Expr::Variable(_, _) | Expr::Property(_) => { let rhs = Expr::Dot( Box::new(BinaryExpr { lhs: x.lhs.into_property(state), @@ -1869,7 +1877,7 @@ fn parse_custom_syntax( segments.push(name.clone().into()); tokens.push(state.get_identifier(MARKER_IDENT)); let var_name_def = Ident { name, pos }; - keywords.push(Expr::Variable(Box::new((None, None, var_name_def)))); + keywords.push(Expr::Variable(None, Box::new((None, None, var_name_def)))); } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); @@ -2825,7 +2833,7 @@ fn make_curry_from_externals( name: x.clone(), pos: Position::NONE, }; - args.push(Expr::Variable(Box::new((None, None, var_def)))); + args.push(Expr::Variable(None, Box::new((None, None, var_def)))); }); let expr = Expr::FnCall( diff --git a/src/token.rs b/src/token.rs index 389ad8f7..9d0884c0 100644 --- a/src/token.rs +++ b/src/token.rs @@ -9,7 +9,6 @@ use crate::stdlib::{ cell::Cell, char, fmt, format, iter::{FusedIterator, Peekable}, - mem, num::NonZeroUsize, ops::{Add, AddAssign}, rc::Rc, @@ -828,7 +827,7 @@ impl From for String { /// This type is volatile and may change. #[derive(Debug, Clone, Eq, PartialEq, Default)] pub struct TokenizeState { - /// Maximum length of a string (0 = unlimited). + /// Maximum length of a string. pub max_string_size: Option, /// Can the next token be a unary operator? pub non_unary: bool, @@ -1187,7 +1186,7 @@ fn get_next_token_inner( } // Within text? - if let Some(ch) = mem::take(&mut state.is_within_text_terminated_by) { + if let Some(ch) = state.is_within_text_terminated_by.take() { let start_pos = *pos; return parse_string_literal(stream, state, pos, ch, false, true, true, true).map_or_else( @@ -1316,42 +1315,42 @@ fn get_next_token_inner( } // Parse number - if let Some(radix) = radix_base { - let out: String = result.iter().skip(2).filter(|&&c| c != NUM_SEP).collect(); + return Some(( + if let Some(radix) = radix_base { + let out: String = + result.iter().skip(2).filter(|&&c| c != NUM_SEP).collect(); - return Some(( INT::from_str_radix(&out, radix) .map(Token::IntegerConstant) .unwrap_or_else(|_| { Token::LexError(LERR::MalformedNumber(result.into_iter().collect())) - }), - start_pos, - )); - } else { - let out: String = result.iter().filter(|&&c| c != NUM_SEP).collect(); - let num = INT::from_str(&out).map(Token::IntegerConstant); + }) + } else { + let out: String = result.iter().filter(|&&c| c != NUM_SEP).collect(); + let num = INT::from_str(&out).map(Token::IntegerConstant); - // If integer parsing is unnecessary, try float instead - #[cfg(not(feature = "no_float"))] - let num = - num.or_else(|_| FloatWrapper::from_str(&out).map(Token::FloatConstant)); + // If integer parsing is unnecessary, try float instead + #[cfg(not(feature = "no_float"))] + let num = + num.or_else(|_| FloatWrapper::from_str(&out).map(Token::FloatConstant)); - // Then try decimal - #[cfg(feature = "decimal")] - let num = num.or_else(|_| Decimal::from_str(&out).map(Token::DecimalConstant)); + // Then try decimal + #[cfg(feature = "decimal")] + let num = + num.or_else(|_| Decimal::from_str(&out).map(Token::DecimalConstant)); - // Then try decimal in scientific notation - #[cfg(feature = "decimal")] - let num = - num.or_else(|_| Decimal::from_scientific(&out).map(Token::DecimalConstant)); + // Then try decimal in scientific notation + #[cfg(feature = "decimal")] + let num = num.or_else(|_| { + Decimal::from_scientific(&out).map(Token::DecimalConstant) + }); - return Some(( num.unwrap_or_else(|_| { Token::LexError(LERR::MalformedNumber(result.into_iter().collect())) - }), - start_pos, - )); - } + }) + }, + start_pos, + )); } // letter or underscore ... From d3cfb3c605e71d98b22cd3f81f55efe65d4a015c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 5 Apr 2021 23:59:15 +0800 Subject: [PATCH 11/15] Optimize position in variable access. --- src/ast.rs | 29 +++++++--- src/engine.rs | 84 ++++++++++++--------------- src/fn_call.rs | 7 +-- src/optimize.rs | 12 ++-- src/parser.rs | 151 +++++++++++++++++++++++++----------------------- src/syntax.rs | 2 +- 6 files changed, 146 insertions(+), 139 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index b3eda699..01baba7f 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1574,14 +1574,19 @@ pub enum Expr { ), /// () Unit(Position), - /// Variable access - optional short index, (optional index, optional (hash, modules), variable name) + /// Variable access - optional short index, position, (optional index, optional (hash, modules), variable name) /// /// The short index is [`u8`] which is used when the index is <= 255, which should be the vast /// majority of cases (unless there are more than 255 variables defined!). /// This is to avoid reading a pointer redirection during each variable access. Variable( Option, - Box<(Option, Option<(u64, NamespaceRef)>, Ident)>, + Position, + Box<( + Option, + Option<(u64, NamespaceRef)>, + Identifier, + )>, ), /// Property access - ((getter, hash), (setter, hash), prop) Property(Box<((Identifier, u64), (Identifier, u64), Ident)>), @@ -1647,11 +1652,19 @@ impl Expr { _ => return None, }) } + /// Is the expression a simple variable access? + #[inline(always)] + pub(crate) fn is_variable_access(&self, non_qualified: bool) -> bool { + match self { + Self::Variable(_, _, x) => !non_qualified || x.1.is_none(), + _ => false, + } + } /// Return the variable name if the expression a simple variable access. #[inline(always)] - pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> { + pub(crate) fn get_variable_name(&self, non_qualified: bool) -> Option<&str> { match self { - Self::Variable(_, x) if !non_qualified || x.1.is_none() => Some((x.2).name.as_str()), + Self::Variable(_, _, x) if !non_qualified || x.1.is_none() => Some(x.2.as_str()), _ => None, } } @@ -1673,7 +1686,7 @@ impl Expr { Self::Map(_, pos) => *pos, Self::Property(x) => (x.2).pos, Self::Stmt(x) => x.pos, - Self::Variable(_, x) => (x.2).pos, + Self::Variable(_, pos, _) => *pos, Self::FnCall(_, pos) => *pos, Self::And(x, _) | Self::Or(x, _) => x.lhs.position(), @@ -1703,7 +1716,7 @@ impl Expr { Self::FnPointer(_, pos) => *pos = new_pos, Self::Array(_, pos) => *pos = new_pos, Self::Map(_, pos) => *pos = new_pos, - Self::Variable(_, x) => (x.2).pos = new_pos, + Self::Variable(_, pos, _) => *pos = new_pos, Self::Property(x) => (x.2).pos = new_pos, Self::Stmt(x) => x.pos = new_pos, Self::FnCall(_, pos) => *pos = new_pos, @@ -1731,7 +1744,7 @@ impl Expr { Self::Stmt(x) => x.statements.iter().all(Stmt::is_pure), - Self::Variable(_, _) => true, + Self::Variable(_, _, _) => true, _ => self.is_constant(), } @@ -1801,7 +1814,7 @@ impl Expr { _ => false, }, - Self::Variable(_, _) => match token { + Self::Variable(_, _, _) => match token { #[cfg(not(feature = "no_index"))] Token::LeftBracket => true, Token::LeftParen => true, diff --git a/src/engine.rs b/src/engine.rs index aeb0c16b..17a84051 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -959,14 +959,14 @@ impl Engine { expr: &Expr, ) -> Result<(Target<'s>, Position), Box> { match expr { - Expr::Variable(Some(_), _) => { + Expr::Variable(Some(_), _, _) => { self.search_scope_only(scope, mods, state, lib, this_ptr, expr) } - Expr::Variable(None, v) => match v.as_ref() { + Expr::Variable(None, var_pos, v) => match v.as_ref() { // Normal variable access (_, None, _) => self.search_scope_only(scope, mods, state, lib, this_ptr, expr), // Qualified variable - (_, Some((hash_var, modules)), Ident { name, pos, .. }) => { + (_, Some((hash_var, modules)), var_name) => { let module = self.search_imports(mods, state, modules).ok_or_else(|| { EvalAltResult::ErrorModuleNotFound( modules[0].name.to_string(), @@ -976,17 +976,17 @@ impl Engine { let target = module.get_qualified_var(*hash_var).map_err(|mut err| { match *err { EvalAltResult::ErrorVariableNotFound(ref mut err_name, _) => { - *err_name = format!("{}{}", modules, name); + *err_name = format!("{}{}", modules, var_name); } _ => (), } - err.fill_position(*pos) + err.fill_position(*var_pos) })?; // Module variables are constant let mut target = target.clone(); target.set_access_mode(AccessMode::ReadOnly); - Ok((target.into(), *pos)) + Ok((target.into(), *var_pos)) } }, _ => unreachable!("Expr::Variable expected, but gets {:?}", expr), @@ -1003,30 +1003,23 @@ impl Engine { this_ptr: &'s mut Option<&mut Dynamic>, expr: &Expr, ) -> Result<(Target<'s>, Position), Box> { - let (short_index, (index, _, Ident { name, pos, .. })) = match expr { - Expr::Variable(i, v) => (i, v.as_ref()), + // Make sure that the pointer indirection is taken only when absolutely necessary. + + let (index, var_pos) = match expr { + // Check if the variable is `this` + Expr::Variable(None, pos, v) if v.0.is_none() && v.2 == KEYWORD_THIS => { + return if let Some(val) = this_ptr { + Ok(((*val).into(), *pos)) + } else { + EvalAltResult::ErrorUnboundThis(*pos).into() + } + } + _ if state.always_search => (0, expr.position()), + Expr::Variable(Some(i), pos, _) => (i.get() as usize, *pos), + Expr::Variable(None, pos, v) => (v.0.map(NonZeroUsize::get).unwrap_or(0), *pos), _ => unreachable!("Expr::Variable expected, but gets {:?}", expr), }; - // Check if the variable is `this` - if *name == KEYWORD_THIS { - return if let Some(val) = this_ptr { - Ok(((*val).into(), *pos)) - } else { - EvalAltResult::ErrorUnboundThis(*pos).into() - }; - } - - // Check if it is directly indexed - let index = if state.always_search { - 0 - } else { - short_index.map_or_else( - || index.map(NonZeroUsize::get).unwrap_or(0), - |x| x.get() as usize, - ) - }; - // Check the variable resolver, if any if let Some(ref resolve_var) = self.resolve_var { let context = EvalContext { @@ -1039,10 +1032,11 @@ impl Engine { level: 0, }; if let Some(mut result) = - resolve_var(name, index, &context).map_err(|err| err.fill_position(*pos))? + resolve_var(expr.get_variable_name(true).unwrap(), index, &context) + .map_err(|err| err.fill_position(var_pos))? { result.set_access_mode(AccessMode::ReadOnly); - return Ok((result.into(), *pos)); + return Ok((result.into(), var_pos)); } } @@ -1050,15 +1044,16 @@ impl Engine { scope.len() - index } else { // Find the variable in the scope + let var_name = expr.get_variable_name(true).unwrap(); scope - .get_index(name) - .ok_or_else(|| EvalAltResult::ErrorVariableNotFound(name.to_string(), *pos))? + .get_index(var_name) + .ok_or_else(|| EvalAltResult::ErrorVariableNotFound(var_name.to_string(), var_pos))? .0 }; let val = scope.get_mut_by_index(index); - Ok((val.into(), *pos)) + Ok((val.into(), var_pos)) } /// Chain-evaluate a dot/index chain. @@ -1410,13 +1405,7 @@ impl Engine { match lhs { // id.??? or id[???] - Expr::Variable(_, x) => { - let Ident { - name: var_name, - pos: var_pos, - .. - } = &x.2; - + Expr::Variable(_, var_pos, x) => { self.inc_operations(state, *var_pos)?; let (target, pos) = @@ -1424,8 +1413,7 @@ impl Engine { // Constants cannot be modified if target.as_ref().is_read_only() && new_val.is_some() { - return EvalAltResult::ErrorAssignmentToConstant(var_name.to_string(), pos) - .into(); + return EvalAltResult::ErrorAssignmentToConstant(x.2.to_string(), pos).into(); } let obj_ptr = &mut target.into(); @@ -1688,11 +1676,11 @@ impl Engine { Expr::CharConstant(x, _) => Ok((*x).into()), Expr::FnPointer(x, _) => Ok(FnPtr::new_unchecked(x.clone(), Default::default()).into()), - Expr::Variable(None, x) if x.0.is_none() && (x.2).name == KEYWORD_THIS => this_ptr + Expr::Variable(None, var_pos, x) if x.0.is_none() && x.2 == KEYWORD_THIS => this_ptr .as_deref() .cloned() - .ok_or_else(|| EvalAltResult::ErrorUnboundThis((x.2).pos).into()), - Expr::Variable(_, _) => self + .ok_or_else(|| EvalAltResult::ErrorUnboundThis(*var_pos).into()), + Expr::Variable(_, _, _) => self .search_namespace(scope, mods, state, lib, this_ptr, expr) .map(|(val, _)| val.take_or_clone()), @@ -2019,7 +2007,7 @@ impl Engine { .flatten()), // var op= rhs - Stmt::Assignment(x, op_pos) if x.0.get_variable_access(false).is_some() => { + Stmt::Assignment(x, op_pos) if x.0.is_variable_access(false) => { let (lhs_expr, op_info, rhs_expr) = x.as_ref(); let rhs_val = self .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)? @@ -2029,7 +2017,7 @@ impl Engine { if !lhs_ptr.is_ref() { return EvalAltResult::ErrorAssignmentToConstant( - lhs_expr.get_variable_access(false).unwrap().to_string(), + lhs_expr.get_variable_name(false).unwrap().to_string(), pos, ) .into(); @@ -2040,7 +2028,7 @@ impl Engine { if lhs_ptr.as_ref().is_read_only() { // Assignment to constant variable EvalAltResult::ErrorAssignmentToConstant( - lhs_expr.get_variable_access(false).unwrap().to_string(), + lhs_expr.get_variable_name(false).unwrap().to_string(), pos, ) .into() @@ -2070,7 +2058,7 @@ impl Engine { // Must be either `var[index] op= val` or `var.prop op= val` match lhs_expr { // name op= rhs (handled above) - Expr::Variable(_, _) => { + Expr::Variable(_, _, _) => { unreachable!("Expr::Variable case should already been handled") } // idx_lhs[idx_expr] op= rhs diff --git a/src/fn_call.rs b/src/fn_call.rs index ccf67349..c0d2845e 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -1293,10 +1293,7 @@ impl Engine { // If the first argument is a variable, and there is no curried arguments, // convert to method-call style in order to leverage potential &mut first argument and // avoid cloning the value - if curry.is_empty() - && !args_expr.is_empty() - && args_expr[0].get_variable_access(false).is_some() - { + if curry.is_empty() && !args_expr.is_empty() && args_expr[0].is_variable_access(false) { // func(x, ...) -> x.func(...) arg_values = args_expr .iter() @@ -1378,7 +1375,7 @@ impl Engine { // 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 !args_expr.is_empty() && args_expr[0].get_variable_access(true).is_some() { + if !args_expr.is_empty() && args_expr[0].is_variable_access(true) { // func(x, ...) -> x.func(...) arg_values = args_expr .iter() diff --git a/src/optimize.rs b/src/optimize.rs index 796af57f..de82a8f9 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -385,7 +385,7 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut State, preserve_result: bool) { match stmt { // expr op= expr Stmt::Assignment(x, _) => match x.0 { - Expr::Variable(_, _) => optimize_expr(&mut x.2, state), + Expr::Variable(_, _, _) => optimize_expr(&mut x.2, state), _ => { optimize_expr(&mut x.0, state); optimize_expr(&mut x.2, state); @@ -635,7 +635,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { .unwrap_or_else(|| Expr::Unit(*pos)); } // var.rhs - (Expr::Variable(_, _), rhs) => optimize_expr(rhs, state), + (Expr::Variable(_, _, _), rhs) => optimize_expr(rhs, state), // lhs.rhs (lhs, rhs) => { optimize_expr(lhs, state); optimize_expr(rhs, state); } } @@ -670,7 +670,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { *expr = Expr::CharConstant(s.chars().nth(*i as usize).unwrap(), *pos); } // var[rhs] - (Expr::Variable(_, _), rhs) => optimize_expr(rhs, state), + (Expr::Variable(_, _, _), rhs) => optimize_expr(rhs, state), // lhs[rhs] (lhs, rhs) => { optimize_expr(lhs, state); optimize_expr(rhs, state); } }, @@ -901,12 +901,12 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { } // constant-name - Expr::Variable(_, x) if x.1.is_none() && state.find_constant(&x.2.name).is_some() => { + Expr::Variable(_, pos, x) if x.1.is_none() && state.find_constant(&x.2).is_some() => { state.set_dirty(); // Replace constant with value - let mut result = state.find_constant(&x.2.name).unwrap().clone(); - result.set_position(x.2.pos); + let mut result = state.find_constant(&x.2).unwrap().clone(); + result.set_position(*pos); *expr = result; } diff --git a/src/parser.rs b/src/parser.rs index f1db5fd8..409b6723 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -225,17 +225,20 @@ impl Expr { #[inline(always)] fn into_property(self, state: &mut ParseState) -> Self { match self { - Self::Variable(_, x) if x.1.is_none() => { + Self::Variable(_, pos, x) if x.1.is_none() => { let ident = x.2; - let getter = state.get_identifier(crate::engine::make_getter(&ident.name)); + let getter = state.get_identifier(crate::engine::make_getter(&ident)); let hash_get = calc_fn_hash(empty(), &getter, 1); - let setter = state.get_identifier(crate::engine::make_setter(&ident.name)); + let setter = state.get_identifier(crate::engine::make_setter(&ident)); let hash_set = calc_fn_hash(empty(), &setter, 2); Self::Property(Box::new(( (getter, hash_get), (setter, hash_set), - ident.into(), + Ident { + name: state.get_identifier(ident), + pos, + }, ))) } _ => self, @@ -1077,11 +1080,11 @@ fn parse_primary( // Once the identifier consumed we must enable next variables capturing state.allow_capture = true; } - let var_name_def = Ident { - name: state.get_identifier(s), - pos: settings.pos, - }; - Expr::Variable(None, Box::new((None, None, var_name_def))) + Expr::Variable( + None, + settings.pos, + Box::new((None, None, state.get_identifier(s))), + ) } // Namespace qualification #[cfg(not(feature = "no_module"))] @@ -1091,19 +1094,15 @@ fn parse_primary( // Once the identifier consumed we must enable next variables capturing state.allow_capture = true; } - let var_name_def = Ident { - name: state.get_identifier(s), - pos: settings.pos, - }; - Expr::Variable(None, Box::new((None, None, var_name_def))) + Expr::Variable( + None, + settings.pos, + Box::new((None, None, state.get_identifier(s))), + ) } // Normal variable access _ => { let index = state.access_var(&s, settings.pos); - let var_name_def = Ident { - name: state.get_identifier(s), - pos: settings.pos, - }; let short_index = index.and_then(|x| { if x.get() <= u8::MAX as usize { NonZeroU8::new(x.get() as u8) @@ -1111,7 +1110,11 @@ fn parse_primary( None } }); - Expr::Variable(short_index, Box::new((index, None, var_name_def))) + Expr::Variable( + short_index, + settings.pos, + Box::new((index, None, state.get_identifier(s))), + ) } } } @@ -1125,21 +1128,17 @@ fn parse_primary( match input.peek().unwrap().0 { // Function call is allowed to have reserved keyword - Token::LeftParen | Token::Bang if is_keyword_function(&s) => { - let var_name_def = Ident { - name: state.get_identifier(s), - pos: settings.pos, - }; - Expr::Variable(None, Box::new((None, None, var_name_def))) - } + Token::LeftParen | Token::Bang if is_keyword_function(&s) => Expr::Variable( + None, + settings.pos, + Box::new((None, None, state.get_identifier(s))), + ), // Access to `this` as a variable is OK within a function scope - _ if s == KEYWORD_THIS && settings.is_function_scope => { - let var_name_def = Ident { - name: state.get_identifier(s), - pos: settings.pos, - }; - Expr::Variable(None, Box::new((None, None, var_name_def))) - } + _ if s == KEYWORD_THIS && settings.is_function_scope => Expr::Variable( + None, + settings.pos, + Box::new((None, None, state.get_identifier(s))), + ), // Cannot access to `this` as a variable not in a function scope _ if s == KEYWORD_THIS => { let msg = format!("'{}' can only be used in functions", s); @@ -1175,7 +1174,7 @@ fn parse_primary( root_expr = match (root_expr, tail_token) { // Qualified function call with ! - (Expr::Variable(_, x), Token::Bang) if x.1.is_some() => { + (Expr::Variable(_, _, x), Token::Bang) if x.1.is_some() => { return Err(if !match_token(input, Token::LeftParen).0 { LexError::UnexpectedInput(Token::Bang.syntax().to_string()).into_err(tail_pos) } else { @@ -1187,7 +1186,7 @@ fn parse_primary( }); } // Function call with ! - (Expr::Variable(_, x), Token::Bang) => { + (Expr::Variable(_, var_pos, x), Token::Bang) => { let (matched, pos) = match_token(input, Token::LeftParen); if !matched { return Err(PERR::MissingToken( @@ -1197,22 +1196,26 @@ fn parse_primary( .into_err(pos)); } - let (_, namespace, Ident { name, pos, .. }) = *x; - settings.pos = pos; + let (_, namespace, name) = *x; + settings.pos = var_pos; let ns = namespace.map(|(_, ns)| ns); parse_fn_call(input, state, lib, name, true, ns, settings.level_up())? } // Function call - (Expr::Variable(_, x), Token::LeftParen) => { - let (_, namespace, Ident { name, pos, .. }) = *x; - settings.pos = pos; + (Expr::Variable(_, var_pos, x), Token::LeftParen) => { + let (_, namespace, name) = *x; + settings.pos = var_pos; let ns = namespace.map(|(_, ns)| ns); parse_fn_call(input, state, lib, name, false, ns, settings.level_up())? } // module access - (Expr::Variable(_, x), Token::DoubleColon) => match input.next().unwrap() { + (Expr::Variable(_, var_pos, x), Token::DoubleColon) => match input.next().unwrap() { (Token::Identifier(id2), pos2) => { - let (_, mut namespace, var_name_def) = *x; + let (_, mut namespace, var_name) = *x; + let var_name_def = Ident { + name: var_name, + pos: var_pos, + }; if let Some((_, ref mut namespace)) = namespace { namespace.push(var_name_def); @@ -1222,11 +1225,11 @@ fn parse_primary( namespace = Some((42, ns)); } - let var_name_def = Ident { - name: state.get_identifier(id2), - pos: pos2, - }; - Expr::Variable(None, Box::new((None, namespace, var_name_def))) + Expr::Variable( + None, + pos2, + Box::new((None, namespace, state.get_identifier(id2))), + ) } (Token::Reserved(id2), pos2) if is_valid_identifier(id2.chars()) => { return Err(PERR::Reserved(id2).into_err(pos2)); @@ -1269,15 +1272,15 @@ fn parse_primary( // Cache the hash key for namespace-qualified variables match &mut root_expr { - Expr::Variable(_, x) if x.1.is_some() => Some(x), + Expr::Variable(_, _, x) if x.1.is_some() => Some(x), Expr::Index(x, _) | Expr::Dot(x, _) => match &mut x.lhs { - Expr::Variable(_, x) if x.1.is_some() => Some(x), + Expr::Variable(_, _, x) if x.1.is_some() => Some(x), _ => None, }, _ => None, } .map(|x| match x.as_mut() { - (_, Some((ref mut hash, ref mut namespace)), Ident { name, .. }) => { + (_, Some((ref mut hash, ref mut namespace)), name) => { *hash = calc_fn_hash(namespace.iter().map(|v| v.name.as_str()), name, 0); #[cfg(not(feature = "no_module"))] @@ -1429,12 +1432,12 @@ fn make_assignment_stmt<'a>( Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) } // var (non-indexed) = rhs - Expr::Variable(None, x) if x.0.is_none() => { + Expr::Variable(None, _, x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } // var (indexed) = rhs - Expr::Variable(i, x) => { - let (index, _, Ident { name, pos, .. }) = x.as_ref(); + Expr::Variable(i, var_pos, x) => { + let (index, _, name) = x.as_ref(); let index = i.map_or_else(|| index.unwrap().get(), |n| n.get() as usize); match state.stack[state.stack.len() - index].1 { AccessMode::ReadWrite => { @@ -1442,7 +1445,7 @@ fn make_assignment_stmt<'a>( } // Constant values cannot be assigned to AccessMode::ReadOnly => { - Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) + Err(PERR::AssignmentToConstant(name.to_string()).into_err(*var_pos)) } } } @@ -1451,12 +1454,12 @@ fn make_assignment_stmt<'a>( match check_lvalue(&x.rhs, matches!(lhs, Expr::Dot(_, _))) { Position::NONE => match &x.lhs { // var[???] (non-indexed) = rhs, var.??? (non-indexed) = rhs - Expr::Variable(None, x) if x.0.is_none() => { + Expr::Variable(None, _, x) if x.0.is_none() => { Ok(Stmt::Assignment(Box::new((lhs, op_info, rhs)), op_pos)) } // var[???] (indexed) = rhs, var.??? (indexed) = rhs - Expr::Variable(i, x) => { - let (index, _, Ident { name, pos, .. }) = x.as_ref(); + Expr::Variable(i, var_pos, x) => { + let (index, _, name) = x.as_ref(); let index = i.map_or_else(|| index.unwrap().get(), |n| n.get() as usize); match state.stack[state.stack.len() - index].1 { AccessMode::ReadWrite => { @@ -1464,7 +1467,7 @@ fn make_assignment_stmt<'a>( } // Constant values cannot be assigned to AccessMode::ReadOnly => { - Err(PERR::AssignmentToConstant(name.to_string()).into_err(*pos)) + Err(PERR::AssignmentToConstant(name.to_string()).into_err(*var_pos)) } } } @@ -1540,19 +1543,26 @@ fn make_dot_expr( Expr::Index(x, pos) } // lhs.id - (lhs, Expr::Variable(_, x)) if x.1.is_none() => { + (lhs, Expr::Variable(_, var_pos, x)) if x.1.is_none() => { let ident = x.2; - let getter = state.get_identifier(crate::engine::make_getter(&ident.name)); + let getter = state.get_identifier(crate::engine::make_getter(&ident)); let hash_get = calc_fn_hash(empty(), &getter, 1); - let setter = state.get_identifier(crate::engine::make_setter(&ident.name)); + let setter = state.get_identifier(crate::engine::make_setter(&ident)); let hash_set = calc_fn_hash(empty(), &setter, 2); - let rhs = Expr::Property(Box::new(((getter, hash_get), (setter, hash_set), ident))); + let rhs = Expr::Property(Box::new(( + (getter, hash_get), + (setter, hash_set), + Ident { + name: state.get_identifier(ident), + pos: var_pos, + }, + ))); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) } // lhs.module::id - syntax error - (_, Expr::Variable(_, x)) if x.1.is_some() => { + (_, Expr::Variable(_, _, x)) if x.1.is_some() => { return Err(PERR::PropertyExpected.into_err(x.1.unwrap().1[0].pos)) } // lhs.prop @@ -1561,7 +1571,7 @@ fn make_dot_expr( } // lhs.dot_lhs.dot_rhs (lhs, Expr::Dot(x, pos)) => match x.lhs { - Expr::Variable(_, _) | Expr::Property(_) => { + Expr::Variable(_, _, _) | Expr::Property(_) => { let rhs = Expr::Dot( Box::new(BinaryExpr { lhs: x.lhs.into_property(state), @@ -1876,8 +1886,7 @@ fn parse_custom_syntax( let name = state.get_identifier(s); segments.push(name.clone().into()); tokens.push(state.get_identifier(MARKER_IDENT)); - let var_name_def = Ident { name, pos }; - keywords.push(Expr::Variable(None, Box::new((None, None, var_name_def)))); + keywords.push(Expr::Variable(None, pos, Box::new((None, None, name)))); } (Token::Reserved(s), pos) if is_valid_identifier(s.chars()) => { return Err(PERR::Reserved(s).into_err(pos)); @@ -2829,11 +2838,11 @@ fn make_curry_from_externals( args.push(fn_expr); externals.iter().for_each(|x| { - let var_def = Ident { - name: x.clone(), - pos: Position::NONE, - }; - args.push(Expr::Variable(None, Box::new((None, None, var_def)))); + args.push(Expr::Variable( + None, + Position::NONE, + Box::new((None, None, x.clone())), + )); }); let expr = Expr::FnCall( diff --git a/src/syntax.rs b/src/syntax.rs index 3eea6244..dc661ea8 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -45,7 +45,7 @@ impl Expression<'_> { /// If this expression is a variable name, return it. Otherwise [`None`]. #[inline(always)] pub fn get_variable_name(&self) -> Option<&str> { - self.0.get_variable_access(true) + self.0.get_variable_name(true) } /// Get the expression. #[inline(always)] From 131147c65d4b6373dc46cd558607072caea01e57 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Apr 2021 12:26:38 +0800 Subject: [PATCH 12/15] Optimize Fn construct. --- src/fn_native.rs | 35 +++++++++++++++++++++-------------- src/optimize.rs | 18 +++++++++++++++++- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/fn_native.rs b/src/fn_native.rs index 3625cd08..8ff22a05 100644 --- a/src/fn_native.rs +++ b/src/fn_native.rs @@ -13,8 +13,8 @@ use crate::stdlib::{ }; use crate::token::is_valid_identifier; use crate::{ - calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ImmutableString, Module, Position, - RhaiResult, StaticVec, + calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, Identifier, ImmutableString, Module, + Position, RhaiResult, StaticVec, }; /// Trait that maps to `Send + Sync` only under the `sync` feature. @@ -252,20 +252,17 @@ pub type FnCallArgs<'a> = [&'a mut Dynamic]; /// A general function pointer, which may carry additional (i.e. curried) argument values /// to be passed onto a function during a call. #[derive(Debug, Clone)] -pub struct FnPtr(ImmutableString, StaticVec); +pub struct FnPtr(Identifier, StaticVec); impl FnPtr { /// Create a new function pointer. #[inline(always)] - pub fn new(name: impl Into) -> Result> { + pub fn new(name: impl Into) -> Result> { name.into().try_into() } /// Create a new function pointer without checking its parameters. #[inline(always)] - pub(crate) fn new_unchecked( - name: impl Into, - curry: StaticVec, - ) -> Self { + pub(crate) fn new_unchecked(name: impl Into, curry: StaticVec) -> Self { Self(name.into(), curry) } /// Get the name of the function. @@ -275,12 +272,12 @@ impl FnPtr { } /// Get the name of the function. #[inline(always)] - pub(crate) fn get_fn_name(&self) -> &ImmutableString { + pub(crate) fn get_fn_name(&self) -> &Identifier { &self.0 } /// Get the underlying data of the function pointer. #[inline(always)] - pub(crate) fn take_data(self) -> (ImmutableString, StaticVec) { + pub(crate) fn take_data(self) -> (Identifier, StaticVec) { (self.0, self.1) } /// Get the curried arguments. @@ -362,11 +359,11 @@ impl fmt::Display for FnPtr { } } -impl TryFrom for FnPtr { +impl TryFrom for FnPtr { type Error = Box; #[inline(always)] - fn try_from(value: ImmutableString) -> Result { + fn try_from(value: Identifier) -> Result { if is_valid_identifier(value.chars()) { Ok(Self(value, Default::default())) } else { @@ -375,12 +372,22 @@ impl TryFrom for FnPtr { } } +impl TryFrom for FnPtr { + type Error = Box; + + #[inline(always)] + fn try_from(value: ImmutableString) -> Result { + let s: Identifier = value.into(); + Self::try_from(s) + } +} + impl TryFrom for FnPtr { type Error = Box; #[inline(always)] fn try_from(value: String) -> Result { - let s: ImmutableString = value.into(); + let s: Identifier = value.into(); Self::try_from(s) } } @@ -390,7 +397,7 @@ impl TryFrom<&str> for FnPtr { #[inline(always)] fn try_from(value: &str) -> Result { - let s: ImmutableString = value.into(); + let s: Identifier = value.into(); Self::try_from(s) } } diff --git a/src/optimize.rs b/src/optimize.rs index de82a8f9..22eb64e6 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -2,7 +2,7 @@ use crate::ast::{Expr, Stmt, StmtBlock}; use crate::dynamic::AccessMode; -use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_PRINT, KEYWORD_TYPE_OF}; +use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT, KEYWORD_TYPE_OF}; use crate::fn_builtin::get_builtin_binary_op_fn; use crate::parser::map_dynamic_to_expr; use crate::stdlib::{ @@ -794,6 +794,22 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { Expr::FnCall(x, _) if x.name == KEYWORD_EVAL => { state.propagate_constants = false; } + // Fn + Expr::FnCall(x, pos) + if x.namespace.is_none() // Non-qualified + && state.optimization_level == OptimizationLevel::Simple // simple optimizations + && x.num_args() == 1 + && matches!(x.args[0], Expr::StringConstant(_, _)) + && x.name == KEYWORD_FN_PTR + => { + if let Expr::StringConstant(s, _) = mem::take(&mut x.args[0]) { + state.set_dirty(); + *expr = Expr::FnPointer(s, *pos); + } else { + unreachable!(); + } + } + // Do not call some special keywords Expr::FnCall(x, _) if DONT_EVAL_KEYWORDS.contains(&x.name.as_ref()) => { x.args.iter_mut().for_each(|a| optimize_expr(a, state)); From 7ec49a95108da61a50ea2072a5ba6ef86de9ebb5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Apr 2021 23:18:28 +0800 Subject: [PATCH 13/15] Fix f32_feature with serde. --- .github/workflows/build.yml | 4 ++-- src/serde/serialize.rs | 2 +- tests/serde.rs | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 26553d24..8687601c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,10 +23,10 @@ jobs: - "--features sync" - "--features no_optimize" - "--features no_float" - - "--features f32_float" + - "--features f32_float,serde,metadata,internals" - "--features decimal" - "--features no_float,decimal" - - "--tests --features only_i32" + - "--tests --features only_i32,serde,metadata,internals" - "--features only_i64" - "--features no_index" - "--features no_object" diff --git a/src/serde/serialize.rs b/src/serde/serialize.rs index e5f3f6a4..ba75842a 100644 --- a/src/serde/serialize.rs +++ b/src/serde/serialize.rs @@ -26,7 +26,7 @@ impl Serialize for Dynamic { Union::Float(x, _) => ser.serialize_f64(**x), #[cfg(not(feature = "no_float"))] #[cfg(feature = "f32_float")] - Union::Float(x, _) => ser.serialize_f32(*x), + Union::Float(x, _) => ser.serialize_f32(**x), #[cfg(feature = "decimal")] #[cfg(not(feature = "f32_float"))] diff --git a/tests/serde.rs b/tests/serde.rs index 8ee666c4..f8557062 100644 --- a/tests/serde.rs +++ b/tests/serde.rs @@ -10,6 +10,8 @@ use serde::{Deserialize, Serialize}; use rhai::Array; #[cfg(not(feature = "no_object"))] use rhai::Map; +#[cfg(not(feature = "no_float"))] +use rhai::FLOAT; #[cfg(feature = "decimal")] use rust_decimal::Decimal; @@ -358,7 +360,7 @@ fn test_serde_de_primary_types() -> Result<(), Box> { #[cfg(not(feature = "no_float"))] { - assert_eq!(123.456, from_dynamic::(&123.456_f64.into())?); + assert_eq!(123.456, from_dynamic::(&123.456.into())?); assert_eq!(123.456, from_dynamic::(&Dynamic::from(123.456_f32))?); } @@ -447,8 +449,8 @@ fn test_serde_de_struct() -> Result<(), Box> { fn test_serde_de_script() -> Result<(), Box> { #[derive(Debug, Deserialize)] struct Point { - x: f64, - y: f64, + x: FLOAT, + y: FLOAT, } #[derive(Debug, Deserialize)] From f17a826f998191a85cdb1ca0c897573cbe460ab9 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Apr 2021 23:18:41 +0800 Subject: [PATCH 14/15] Refine debug print-out. --- src/ast.rs | 152 ++++++++++++++++++++++++++++------- src/dynamic.rs | 24 +++--- src/engine.rs | 16 +++- src/module/mod.rs | 78 ++++++++---------- src/optimize.rs | 15 ++-- src/packages/fn_basic.rs | 2 +- src/packages/string_basic.rs | 20 ++--- src/parser.rs | 8 +- src/token.rs | 4 +- tests/optimizer.rs | 23 ++++-- 10 files changed, 219 insertions(+), 123 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 01baba7f..c964b214 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -25,6 +25,9 @@ use crate::{ #[cfg(not(feature = "no_float"))] use crate::{stdlib::str::FromStr, FLOAT}; +#[cfg(not(feature = "no_float"))] +use num_traits::Float; + #[cfg(not(feature = "no_index"))] use crate::Array; @@ -776,7 +779,7 @@ pub struct Ident { impl fmt::Debug for Ident { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Ident({:?} @ {:?})", self.name, self.pos) + write!(f, "{:?} @ {:?}", self.name, self.pos) } } @@ -1440,13 +1443,13 @@ impl FnCallExpr { } } -/// A type that wraps a [`FLOAT`] and implements [`Hash`]. +/// A type that wraps a floating-point number and implements [`Hash`]. #[cfg(not(feature = "no_float"))] #[derive(Clone, Copy, PartialEq, PartialOrd)] -pub struct FloatWrapper(FLOAT); +pub struct FloatWrapper(F); #[cfg(not(feature = "no_float"))] -impl Hash for FloatWrapper { +impl Hash for FloatWrapper { #[inline(always)] fn hash(&self, state: &mut H) { self.0.to_ne_bytes().hash(state); @@ -1454,24 +1457,24 @@ impl Hash for FloatWrapper { } #[cfg(not(feature = "no_float"))] -impl AsRef for FloatWrapper { +impl AsRef for FloatWrapper { #[inline(always)] - fn as_ref(&self) -> &FLOAT { + fn as_ref(&self) -> &F { &self.0 } } #[cfg(not(feature = "no_float"))] -impl AsMut for FloatWrapper { +impl AsMut for FloatWrapper { #[inline(always)] - fn as_mut(&mut self) -> &mut FLOAT { + fn as_mut(&mut self) -> &mut F { &mut self.0 } } #[cfg(not(feature = "no_float"))] -impl crate::stdlib::ops::Deref for FloatWrapper { - type Target = FLOAT; +impl crate::stdlib::ops::Deref for FloatWrapper { + type Target = F; #[inline(always)] fn deref(&self) -> &Self::Target { @@ -1480,7 +1483,7 @@ impl crate::stdlib::ops::Deref for FloatWrapper { } #[cfg(not(feature = "no_float"))] -impl crate::stdlib::ops::DerefMut for FloatWrapper { +impl crate::stdlib::ops::DerefMut for FloatWrapper { #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 @@ -1488,52 +1491,67 @@ impl crate::stdlib::ops::DerefMut for FloatWrapper { } #[cfg(not(feature = "no_float"))] -impl fmt::Debug for FloatWrapper { +impl fmt::Debug for FloatWrapper { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(self, f) + fmt::Display::fmt(&self.0, f) } } #[cfg(not(feature = "no_float"))] -impl fmt::Display for FloatWrapper { +impl> fmt::Display for FloatWrapper { #[inline(always)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - #[cfg(feature = "no_std")] - #[cfg(not(feature = "no_float"))] - use num_traits::Float; - let abs = self.0.abs(); - if abs > 10000000000000.0 || abs < 0.0000000000001 { + if abs > Self::MAX_NATURAL_FLOAT_FOR_DISPLAY.into() + || abs < Self::MIN_NATURAL_FLOAT_FOR_DISPLAY.into() + { write!(f, "{:e}", self.0) } else { - self.0.fmt(f) + fmt::Display::fmt(&self.0, f)?; + if abs.fract().is_zero() { + f.write_str(".0")?; + } + Ok(()) } } } #[cfg(not(feature = "no_float"))] -impl From for FloatWrapper { +impl From for FloatWrapper { #[inline(always)] - fn from(value: FLOAT) -> Self { + fn from(value: F) -> Self { Self::new(value) } } #[cfg(not(feature = "no_float"))] -impl FromStr for FloatWrapper { - type Err = ::Err; +impl FromStr for FloatWrapper { + type Err = ::Err; #[inline(always)] fn from_str(s: &str) -> Result { - FLOAT::from_str(s).map(Into::::into) + F::from_str(s).map(Into::::into) } } #[cfg(not(feature = "no_float"))] -impl FloatWrapper { +impl FloatWrapper { + /// Maximum floating-point number for natural display before switching to scientific notation. + pub const MAX_NATURAL_FLOAT_FOR_DISPLAY: f32 = 10000000000000.0; + + /// Minimum floating-point number for natural display before switching to scientific notation. + pub const MIN_NATURAL_FLOAT_FOR_DISPLAY: f32 = 0.0000000000001; + #[inline(always)] - pub const fn new(value: FLOAT) -> Self { + pub fn new(value: F) -> Self { + Self(value) + } +} + +impl FloatWrapper { + #[inline(always)] + pub(crate) const fn const_new(value: FLOAT) -> Self { Self(value) } } @@ -1544,7 +1562,7 @@ impl FloatWrapper { /// # Volatile Data Structure /// /// This type is volatile and may change. -#[derive(Debug, Clone, Hash)] +#[derive(Clone, Hash)] pub enum Expr { /// Dynamic constant. /// Used to hold either an [`Array`] or [`Map`][crate::Map] literal for quick cloning. @@ -1556,7 +1574,7 @@ pub enum Expr { IntegerConstant(INT, Position), /// Floating-point constant. #[cfg(not(feature = "no_float"))] - FloatConstant(FloatWrapper, Position), + FloatConstant(FloatWrapper, Position), /// Character constant. CharConstant(char, Position), /// [String][ImmutableString] constant. @@ -1613,6 +1631,80 @@ impl Default for Expr { } } +impl fmt::Debug for Expr { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DynamicConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), + Self::BoolConstant(value, pos) => write!(f, "{} @ {:?}", value, pos), + Self::IntegerConstant(value, pos) => write!(f, "{} @ {:?}", value, pos), + #[cfg(not(feature = "no_float"))] + Self::FloatConstant(value, pos) => write!(f, "{} @ {:?}", value, pos), + Self::CharConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), + Self::StringConstant(value, pos) => write!(f, "{:?} @ {:?}", value, pos), + Self::FnPointer(value, pos) => write!(f, "Fn({:?}) @ {:?}", value, pos), + Self::Unit(pos) => write!(f, "() @ {:?}", pos), + Self::InterpolatedString(x) => { + f.write_str("InterpolatedString")?; + f.debug_list().entries(x.iter()).finish() + } + Self::Array(x, pos) => { + f.write_str("Array")?; + f.debug_list().entries(x.iter()).finish()?; + write!(f, " @ {:?}", pos) + } + Self::Map(x, pos) => { + f.write_str("Map")?; + f.debug_map() + .entries(x.0.iter().map(|(k, v)| (k, v))) + .finish()?; + write!(f, " @ {:?}", pos) + } + Self::Variable(i, pos, x) => { + f.write_str("Variable(")?; + match x.1 { + Some((_, ref namespace)) => write!(f, "{}::", namespace)?, + _ => (), + } + write!(f, "{}", x.2)?; + match i.map_or_else(|| x.0, |n| NonZeroUsize::new(n.get() as usize)) { + Some(n) => write!(f, " [{}]", n)?, + _ => (), + } + write!(f, ") @ {:?}", pos) + } + Self::Property(x) => write!(f, "Property({:?} @ {:?})", x.2.name, x.2.pos), + Self::Stmt(x) => { + f.write_str("Stmt")?; + f.debug_list().entries(x.statements.iter()).finish()?; + write!(f, " @ {:?}", x.pos) + } + Self::FnCall(x, pos) => { + f.debug_tuple("FnCall").field(x).finish()?; + write!(f, " @ {:?}", pos) + } + Self::Dot(x, pos) | Self::Index(x, pos) | Self::And(x, pos) | Self::Or(x, pos) => { + let op = match self { + Self::Dot(_, _) => "Dot", + Self::Index(_, _) => "Index", + Self::And(_, _) => "And", + Self::Or(_, _) => "Or", + _ => unreachable!(), + }; + + f.debug_struct(op) + .field("lhs", &x.lhs) + .field("rhs", &x.rhs) + .finish()?; + write!(f, " @ {:?}", pos) + } + Self::Custom(x, pos) => { + f.debug_tuple("Custom").field(x).finish()?; + write!(f, " @ {:?}", pos) + } + } + } +} + impl Expr { /// Get the [`Dynamic`] value of a constant expression. /// @@ -1914,7 +2006,7 @@ mod tests { assert_eq!(size_of::>(), 16); assert_eq!(size_of::(), 32); assert_eq!(size_of::>(), 32); - assert_eq!(size_of::(), 80); + assert_eq!(size_of::(), 96); assert_eq!(size_of::(), 288); assert_eq!(size_of::(), 56); assert_eq!(size_of::(), 16); diff --git a/src/dynamic.rs b/src/dynamic.rs index 3c0c23d4..92025c68 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -159,7 +159,7 @@ pub enum Union { Int(INT, AccessMode), /// A floating-point value. #[cfg(not(feature = "no_float"))] - Float(FloatWrapper, AccessMode), + Float(FloatWrapper, AccessMode), /// A fixed-precision decimal value. #[cfg(feature = "decimal")] Decimal(Box, AccessMode), @@ -682,16 +682,22 @@ impl Dynamic { pub const NEGATIVE_ONE: Dynamic = Self(Union::Int(-1, AccessMode::ReadWrite)); /// A [`Dynamic`] containing the floating-point zero. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ZERO: Dynamic = - Self(Union::Float(FloatWrapper::new(0.0), AccessMode::ReadWrite)); + pub const FLOAT_ZERO: Dynamic = Self(Union::Float( + FloatWrapper::const_new(0.0), + AccessMode::ReadWrite, + )); /// A [`Dynamic`] containing the floating-point one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_ONE: Dynamic = - Self(Union::Float(FloatWrapper::new(1.0), AccessMode::ReadWrite)); + pub const FLOAT_ONE: Dynamic = Self(Union::Float( + FloatWrapper::const_new(1.0), + AccessMode::ReadWrite, + )); /// A [`Dynamic`] containing the floating-point negative one. #[cfg(not(feature = "no_float"))] - pub const FLOAT_NEGATIVE_ONE: Dynamic = - Self(Union::Float(FloatWrapper::new(-1.0), AccessMode::ReadWrite)); + pub const FLOAT_NEGATIVE_ONE: Dynamic = Self(Union::Float( + FloatWrapper::const_new(-1.0), + AccessMode::ReadWrite, + )); /// Get the [`AccessMode`] for this [`Dynamic`]. pub(crate) fn access_mode(&self) -> AccessMode { @@ -1670,9 +1676,9 @@ impl From for Dynamic { } } #[cfg(not(feature = "no_float"))] -impl From for Dynamic { +impl From> for Dynamic { #[inline(always)] - fn from(value: FloatWrapper) -> Self { + fn from(value: FloatWrapper) -> Self { Self(Union::Float(value, AccessMode::ReadWrite)) } } diff --git a/src/engine.rs b/src/engine.rs index 17a84051..dfc1225c 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -52,7 +52,7 @@ pub type Precedence = NonZeroU8; // We cannot use Cow here because `eval` may load a [module][Module] and // the module name will live beyond the AST of the eval script text. // The best we can do is a shared reference. -#[derive(Debug, Clone, Default)] +#[derive(Clone, Default)] pub struct Imports(StaticVec, StaticVec>); impl Imports { @@ -147,6 +147,20 @@ impl Imports { } } +impl fmt::Debug for Imports { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Imports")?; + + if self.is_empty() { + f.debug_map().finish() + } else { + f.debug_map() + .entries(self.0.iter().zip(self.1.iter())) + .finish() + } + } +} + #[cfg(not(feature = "unchecked"))] #[cfg(debug_assertions)] #[cfg(not(feature = "no_function"))] diff --git a/src/module/mod.rs b/src/module/mod.rs index 34b23693..aa20b2df 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -7,8 +7,8 @@ use crate::fn_register::RegisterNativeFunction; use crate::stdlib::{ any::TypeId, boxed::Box, - collections::BTreeMap, - fmt, format, + collections::{BTreeMap, BTreeSet}, + fmt, iter::empty, num::NonZeroUsize, ops::{Add, AddAssign, Deref, DerefMut}, @@ -173,50 +173,36 @@ impl Default for Module { impl fmt::Debug for Module { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "Module({}\n{}{}{})", - self.id - .as_ref() - .map(|id| format!("id: {:?},", id)) - .unwrap_or_default(), - if !self.modules.is_empty() { - format!( - " modules: {}\n", - self.modules - .keys() - .map(|m| m.as_str()) - .collect::>() - .join(", ") - ) - } else { - Default::default() - }, - if !self.variables.is_empty() { - format!( - " vars: {}\n", - self.variables - .iter() - .map(|(k, v)| format!("{}={:?}", k, v)) - .collect::>() - .join(", ") - ) - } else { - Default::default() - }, - if !self.functions.is_empty() { - format!( - " functions: {}\n", - self.functions - .values() - .map(|f| crate::stdlib::string::ToString::to_string(&f.func)) - .collect::>() - .join(", ") - ) - } else { - Default::default() - } - ) + let mut d = f.debug_struct("Module"); + + if let Some(ref id) = self.id { + d.field("id", id); + } + + if !self.modules.is_empty() { + d.field( + "modules", + &self + .modules + .keys() + .map(|m| m.as_str()) + .collect::>(), + ); + } + if !self.variables.is_empty() { + d.field("vars", &self.variables); + } + if !self.functions.is_empty() { + d.field( + "functions", + &self + .functions + .values() + .map(|f| crate::stdlib::string::ToString::to_string(&f.func)) + .collect::>(), + ); + } + d.finish() } } diff --git a/src/optimize.rs b/src/optimize.rs index 22eb64e6..03814584 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -17,8 +17,8 @@ use crate::stdlib::{ }; use crate::utils::get_hasher; use crate::{ - calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, Module, Position, Scope, - StaticVec, AST, + calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, ImmutableString, Module, + Position, Scope, StaticVec, AST, }; /// Level of optimization performed. @@ -799,15 +799,12 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { if x.namespace.is_none() // Non-qualified && state.optimization_level == OptimizationLevel::Simple // simple optimizations && x.num_args() == 1 - && matches!(x.args[0], Expr::StringConstant(_, _)) + && x.constant_args.len() == 1 + && x.constant_args[0].0.is::() && x.name == KEYWORD_FN_PTR => { - if let Expr::StringConstant(s, _) = mem::take(&mut x.args[0]) { - state.set_dirty(); - *expr = Expr::FnPointer(s, *pos); - } else { - unreachable!(); - } + state.set_dirty(); + *expr = Expr::FnPointer(mem::take(&mut x.constant_args[0].0).take_immutable_string().unwrap(), *pos); } // Do not call some special keywords diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index 3331f32c..ab589415 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -9,7 +9,7 @@ def_package!(crate:BasicFnPackage:"Basic Fn functions.", lib, { mod fn_ptr_functions { #[rhai_fn(name = "name", get = "name", pure)] pub fn name(f: &mut FnPtr) -> ImmutableString { - f.get_fn_name().clone() + f.get_fn_name().as_str().into() } #[cfg(not(feature = "no_function"))] diff --git a/src/packages/string_basic.rs b/src/packages/string_basic.rs index 2cceda30..5d05da0b 100644 --- a/src/packages/string_basic.rs +++ b/src/packages/string_basic.rs @@ -73,31 +73,23 @@ mod print_debug_functions { #[cfg(not(feature = "no_float"))] use num_traits::Float; + use crate::ast::FloatWrapper; + #[rhai_fn(name = "print", name = "to_string")] pub fn print_f64(number: f64) -> ImmutableString { - let abs = number.abs(); - if abs > 10000000000000.0 || abs < 0.0000000000001 { - format!("{:e}", number).into() - } else { - number.to_string().into() - } + FloatWrapper::new(number).to_string().into() } #[rhai_fn(name = "print", name = "to_string")] pub fn print_f32(number: f32) -> ImmutableString { - let abs = number.abs(); - if abs > 10000000000000.0 || abs < 0.0000000000001 { - format!("{:e}", number).into() - } else { - number.to_string().into() - } + FloatWrapper::new(number).to_string().into() } #[rhai_fn(name = "debug", name = "to_debug")] pub fn debug_f64(number: f64) -> ImmutableString { - number.to_string().into() + format!("{:?}", FloatWrapper::new(number)).into() } #[rhai_fn(name = "debug", name = "to_debug")] pub fn debug_f32(number: f32) -> ImmutableString { - number.to_string().into() + format!("{:?}", FloatWrapper::new(number)).into() } } diff --git a/src/parser.rs b/src/parser.rs index 409b6723..ab0aa0bc 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1271,16 +1271,16 @@ fn parse_primary( } // Cache the hash key for namespace-qualified variables - match &mut root_expr { - Expr::Variable(_, _, x) if x.1.is_some() => Some(x), - Expr::Index(x, _) | Expr::Dot(x, _) => match &mut x.lhs { + match root_expr { + Expr::Variable(_, _, ref mut x) if x.1.is_some() => Some(x), + Expr::Index(ref mut x, _) | Expr::Dot(ref mut x, _) => match &mut x.lhs { Expr::Variable(_, _, x) if x.1.is_some() => Some(x), _ => None, }, _ => None, } .map(|x| match x.as_mut() { - (_, Some((ref mut hash, ref mut namespace)), name) => { + (_, Some((hash, namespace)), name) => { *hash = calc_fn_hash(namespace.iter().map(|v| v.name.as_str()), name, 0); #[cfg(not(feature = "no_module"))] diff --git a/src/token.rs b/src/token.rs index 9d0884c0..cf5b06f2 100644 --- a/src/token.rs +++ b/src/token.rs @@ -18,7 +18,7 @@ use crate::stdlib::{ use crate::{Engine, LexError, StaticVec, INT}; #[cfg(not(feature = "no_float"))] -use crate::ast::FloatWrapper; +use crate::{ast::FloatWrapper, FLOAT}; #[cfg(feature = "decimal")] use rust_decimal::Decimal; @@ -210,7 +210,7 @@ pub enum Token { /// /// Reserved under the `no_float` feature. #[cfg(not(feature = "no_float"))] - FloatConstant(FloatWrapper), + FloatConstant(FloatWrapper), /// A [`Decimal`] constant. /// /// Requires the `decimal` feature. diff --git a/tests/optimizer.rs b/tests/optimizer.rs index caa98dcc..69c884fd 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -55,24 +55,33 @@ fn test_optimizer_parse() -> Result<(), Box> { let ast = engine.compile("{ const DECISION = false; if DECISION { 42 } else { 123 } }")?; - assert!(format!("{:?}", ast).starts_with( - r#"AST { source: None, body: [Expr(IntegerConstant(123, 1:53))], functions: Module("# - )); + assert_eq!( + format!("{:?}", ast), + "AST { source: None, body: [Expr(123 @ 1:53)], functions: Module, resolver: None }" + ); let ast = engine.compile("const DECISION = false; if DECISION { 42 } else { 123 }")?; - assert!(format!("{:?}", ast).starts_with(r#"AST { source: None, body: [Const(BoolConstant(false, 1:18), Ident("DECISION" @ 1:7), false, 1:1), Expr(IntegerConstant(123, 1:51))], functions: Module("#)); + assert_eq!( + format!("{:?}", ast), + r#"AST { source: None, body: [Const(false @ 1:18, "DECISION" @ 1:7, false, 1:1), Expr(123 @ 1:51)], functions: Module, resolver: None }"# + ); let ast = engine.compile("if 1 == 2 { 42 }")?; - assert!(format!("{:?}", ast).starts_with("AST { source: None, body: [], functions: Module(")); + assert_eq!( + format!("{:?}", ast), + "AST { source: None, body: [], functions: Module, resolver: None }" + ); engine.set_optimization_level(OptimizationLevel::Full); let ast = engine.compile("abs(-42)")?; - assert!(format!("{:?}", ast) - .starts_with(r"AST { source: None, body: [Expr(IntegerConstant(42, 1:1))]")); + assert_eq!( + format!("{:?}", ast), + "AST { source: None, body: [Expr(42 @ 1:1)], functions: Module, resolver: None }" + ); Ok(()) } From 0f2e7e38256cd055f6cc5bb4bac5fd725fc2b038 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 6 Apr 2021 23:28:22 +0800 Subject: [PATCH 15/15] Fix builds and tests. --- src/ast.rs | 1 + tests/optimizer.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/ast.rs b/src/ast.rs index c964b214..9d7902d8 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1549,6 +1549,7 @@ impl FloatWrapper { } } +#[cfg(not(feature = "no_float"))] impl FloatWrapper { #[inline(always)] pub(crate) const fn const_new(value: FLOAT) -> Self { diff --git a/tests/optimizer.rs b/tests/optimizer.rs index 69c884fd..cd3baf58 100644 --- a/tests/optimizer.rs +++ b/tests/optimizer.rs @@ -48,6 +48,7 @@ fn test_optimizer_run() -> Result<(), Box> { Ok(()) } +#[cfg(not(feature = "no_module"))] #[test] fn test_optimizer_parse() -> Result<(), Box> { let mut engine = Engine::new();