From 89725a6dd4118c7102662dee859b2ac946c0b295 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Feb 2023 14:58:03 +0800 Subject: [PATCH 1/8] Fix bug when parsing !in. --- CHANGELOG.md | 1 + src/tokenizer.rs | 169 +++++++++++++++++++++++++---------------------- 2 files changed, 90 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d7c0965..3126de3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Bug fixes * `map` and `filter` for arrays are marked `pure`. Warnings are added to the documentation of pure array methods that take `this` closures. * Syntax such as `foo.bar::baz` no longer panics, but returns a proper parse error. * `x += y` where `x` and `y` are `char` now works correctly. +* Expressions such as `!inside` now parses correctly instead of as `!in` followed by `side`. Enhancements ------------ diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 16381ccf..16a7510c 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -6,6 +6,7 @@ use crate::engine::{ }; use crate::func::native::OnParseTokenCallback; use crate::{Engine, Identifier, LexError, Position, SmartString, StaticVec, INT, UNSIGNED_INT}; +use smallvec::SmallVec; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -980,7 +981,7 @@ pub fn parse_string_literal( if termination_char == next_char && escape.is_empty() { // Double wrapper if stream.peek_next().map_or(false, |c| c == termination_char) { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if let Some(ref mut last) = state.last_token { last.push(termination_char); } @@ -1122,7 +1123,7 @@ pub fn parse_string_literal( /// Consume the next character. #[inline(always)] -fn eat_next(stream: &mut impl InputStream, pos: &mut Position) -> Option { +fn eat_next_and_advance(stream: &mut impl InputStream, pos: &mut Position) -> Option { pos.advance(); stream.get_next() } @@ -1147,7 +1148,7 @@ fn scan_block_comment( match c { '/' => { if let Some(c2) = stream.peek_next().filter(|&c2| c2 == '*') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if let Some(comment) = comment.as_mut() { comment.push(c2); } @@ -1156,7 +1157,7 @@ fn scan_block_comment( } '*' => { if let Some(c2) = stream.peek_next().filter(|&c2| c2 == '/') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if let Some(comment) = comment.as_mut() { comment.push(c2); } @@ -1287,11 +1288,11 @@ fn get_next_token_inner( while let Some(next_char) = stream.peek_next() { match next_char { NUMBER_SEPARATOR => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); } ch if valid(ch) => { result.push(next_char); - eat_next(stream, pos); + eat_next_and_advance(stream, pos); } #[cfg(any(not(feature = "no_float"), feature = "decimal"))] '.' => { @@ -1357,7 +1358,7 @@ fn get_next_token_inner( if c == '0' && result.len() <= 1 => { result.push(next_char); - eat_next(stream, pos); + eat_next_and_advance(stream, pos); valid = match ch { 'x' | 'X' => is_hex_digit, @@ -1461,16 +1462,16 @@ fn get_next_token_inner( match stream.peek_next() { // `\r - start from next line Some('\r') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); // `\r\n if stream.peek_next() == Some('\n') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); } pos.new_line(); } // `\n - start from next line Some('\n') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); pos.new_line(); } _ => (), @@ -1522,13 +1523,13 @@ fn get_next_token_inner( // Unit ('(', ')') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Unit, start_pos)); } // Parentheses ('(', '*') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("(*".into())), start_pos)); } ('(', ..) => return Some((Token::LeftParen, start_pos)), @@ -1541,16 +1542,16 @@ fn get_next_token_inner( // Map literal #[cfg(not(feature = "no_object"))] ('#', '{') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::MapStart, start_pos)); } // Shebang ('#', '!') => return Some((Token::Reserved(Box::new("#!".into())), start_pos)), ('#', ' ') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); let token = if stream.peek_next() == Some('{') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); "# {" } else { "#" @@ -1562,11 +1563,11 @@ fn get_next_token_inner( // Operators ('+', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::PlusAssign, start_pos)); } ('+', '+') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("++".into())), start_pos)); } ('+', ..) if !state.next_token_cannot_be_unary => { @@ -1577,15 +1578,15 @@ fn get_next_token_inner( ('-', '0'..='9') if !state.next_token_cannot_be_unary => negated = Some(start_pos), ('-', '0'..='9') => return Some((Token::Minus, start_pos)), ('-', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::MinusAssign, start_pos)); } ('-', '>') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("->".into())), start_pos)); } ('-', '-') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("--".into())), start_pos)); } ('-', ..) if !state.next_token_cannot_be_unary => { @@ -1594,19 +1595,19 @@ fn get_next_token_inner( ('-', ..) => return Some((Token::Minus, start_pos)), ('*', ')') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("*)".into())), start_pos)); } ('*', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::MultiplyAssign, start_pos)); } ('*', '*') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( if stream.peek_next() == Some('=') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Token::PowerOfAssign } else { Token::PowerOf @@ -1618,13 +1619,13 @@ fn get_next_token_inner( // Comments ('/', '/') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); let mut comment: Option = match stream.peek_next() { #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] Some('/') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); // Long streams of `///...` are not doc-comments match stream.peek_next() { @@ -1634,7 +1635,7 @@ fn get_next_token_inner( } #[cfg(feature = "metadata")] Some('!') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Some("//!".into()) } _ if state.include_comments => Some("//".into()), @@ -1645,7 +1646,7 @@ fn get_next_token_inner( if c == '\r' { // \r\n if stream.peek_next() == Some('\n') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); } pos.new_line(); break; @@ -1676,13 +1677,13 @@ fn get_next_token_inner( } ('/', '*') => { state.comment_level = 1; - eat_next(stream, pos); + eat_next_and_advance(stream, pos); let mut comment: Option = match stream.peek_next() { #[cfg(not(feature = "no_function"))] #[cfg(feature = "metadata")] Some('*') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); // Long streams of `/****...` are not doc-comments match stream.peek_next() { @@ -1703,7 +1704,7 @@ fn get_next_token_inner( } ('/', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::DivideAssign, start_pos)); } ('/', ..) => return Some((Token::Divide, start_pos)), @@ -1712,15 +1713,15 @@ fn get_next_token_inner( (',', ..) => return Some((Token::Comma, start_pos)), ('.', '.') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( match stream.peek_next() { Some('.') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Token::Reserved(Box::new("...".into())) } Some('=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Token::InclusiveRange } _ => Token::ExclusiveRange, @@ -1731,56 +1732,56 @@ fn get_next_token_inner( ('.', ..) => return Some((Token::Period, start_pos)), ('=', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if stream.peek_next() == Some('=') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("===".into())), start_pos)); } return Some((Token::EqualsTo, start_pos)); } ('=', '>') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::DoubleArrow, start_pos)); } ('=', ..) => return Some((Token::Equals, start_pos)), #[cfg(not(feature = "no_module"))] (':', ':') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if stream.peek_next() == Some('<') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("::<".into())), start_pos)); } return Some((Token::DoubleColon, start_pos)); } (':', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new(":=".into())), start_pos)); } (':', ';') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new(":;".into())), start_pos)); } (':', ..) => return Some((Token::Colon, start_pos)), ('<', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::LessThanEqualsTo, start_pos)); } ('<', '-') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("<-".into())), start_pos)); } ('<', '<') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( if stream.peek_next() == Some('=') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Token::LeftShiftAssign } else { Token::LeftShift @@ -1789,21 +1790,21 @@ fn get_next_token_inner( )); } ('<', '|') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("<|".into())), start_pos)); } ('<', ..) => return Some((Token::LessThan, start_pos)), ('>', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::GreaterThanEqualsTo, start_pos)); } ('>', '>') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( if stream.peek_next() == Some('=') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); Token::RightShiftAssign } else { Token::RightShift @@ -1814,56 +1815,68 @@ fn get_next_token_inner( ('>', ..) => return Some((Token::GreaterThan, start_pos)), ('!', 'i') => { - eat_next(stream, pos); + stream.get_next().unwrap(); if stream.peek_next() == Some('n') { - eat_next(stream, pos); - return Some((Token::NotIn, start_pos)); + stream.get_next().unwrap(); + match stream.peek_next() { + Some(c) if is_id_continue(c) => { + stream.unget('n'); + stream.unget('i'); + return Some((Token::Bang, start_pos)); + } + _ => { + pos.advance(); + pos.advance(); + return Some((Token::NotIn, start_pos)); + } + } + } else { + stream.unget('i'); + return Some((Token::Bang, start_pos)); } - stream.unget('i'); - return Some((Token::Bang, start_pos)); } ('!', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); if stream.peek_next() == Some('=') { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("!==".into())), start_pos)); } return Some((Token::NotEqualsTo, start_pos)); } ('!', '.') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("!.".into())), start_pos)); } ('!', ..) => return Some((Token::Bang, start_pos)), ('|', '|') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Or, start_pos)); } ('|', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::OrAssign, start_pos)); } ('|', '>') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::Reserved(Box::new("|>".into())), start_pos)); } ('|', ..) => return Some((Token::Pipe, start_pos)), ('&', '&') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::And, start_pos)); } ('&', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::AndAssign, start_pos)); } ('&', ..) => return Some((Token::Ampersand, start_pos)), ('^', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::XOrAssign, start_pos)); } ('^', ..) => return Some((Token::XOr, start_pos)), @@ -1871,7 +1884,7 @@ fn get_next_token_inner( ('~', ..) => return Some((Token::Reserved(Box::new("~".into())), start_pos)), ('%', '=') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::ModuloAssign, start_pos)); } ('%', ..) => return Some((Token::Modulo, start_pos)), @@ -1881,7 +1894,7 @@ fn get_next_token_inner( ('$', ..) => return Some((Token::Reserved(Box::new("$".into())), start_pos)), ('?', '.') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( #[cfg(not(feature = "no_object"))] Token::Elvis, @@ -1891,11 +1904,11 @@ fn get_next_token_inner( )); } ('?', '?') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some((Token::DoubleQuestion, start_pos)); } ('?', '[') => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); return Some(( #[cfg(not(feature = "no_index"))] Token::QuestionBracket, @@ -1940,7 +1953,7 @@ fn parse_identifier_token( while let Some(next_char) = stream.peek_next() { match next_char { x if is_id_continue(x) => { - eat_next(stream, pos); + eat_next_and_advance(stream, pos); identifier.push(x); if let Some(ref mut last) = state.last_token { last.push(x); @@ -2091,8 +2104,8 @@ pub fn is_reserved_keyword_or_symbol(syntax: &str) -> bool { /// /// Multiple character streams are jointed together to form one single stream. pub struct MultiInputsStream<'a> { - /// Buffered character, if any. - pub buf: Option, + /// Buffered characters, if any. + pub buf: SmallVec<[char; 2]>, /// The current stream index. pub index: usize, /// The input character streams. @@ -2102,15 +2115,11 @@ pub struct MultiInputsStream<'a> { impl InputStream for MultiInputsStream<'_> { #[inline] fn unget(&mut self, ch: char) { - if self.buf.is_some() { - panic!("cannot unget two characters in a row"); - } - - self.buf = Some(ch); + self.buf.push(ch); } fn get_next(&mut self) -> Option { - if let Some(ch) = self.buf.take() { - return Some(ch); + if let ch @ Some(..) = self.buf.pop() { + return ch; } loop { @@ -2127,8 +2136,8 @@ impl InputStream for MultiInputsStream<'_> { } } fn peek_next(&mut self) -> Option { - if let Some(ch) = self.buf { - return Some(ch); + if let ch @ Some(..) = self.buf.last() { + return ch.cloned(); } loop { @@ -2368,7 +2377,7 @@ impl Engine { }, pos: Position::new(1, 0), stream: MultiInputsStream { - buf: None, + buf: SmallVec::new_const(), streams: input .into_iter() .map(|s| s.as_ref().chars().peekable()) From 8d1fa19331aedf9d481eafc5e4f2c072f135c096 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Feb 2023 18:33:22 +0800 Subject: [PATCH 2/8] Remove usage of all() --- src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9ac564bb..a6852433 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,7 +166,8 @@ const MAX_USIZE_INT: INT = INT::MAX; const MAX_USIZE_INT: INT = usize::MAX as INT; /// The maximum integer that can fit into a [`usize`]. -#[cfg(all(feature = "only_i32", target_pointer_width = "32"))] +#[cfg(feature = "only_i32")] +#[cfg(target_pointer_width = "32")] const MAX_USIZE_INT: INT = INT::MAX; /// Number of bits in [`INT`]. @@ -315,7 +316,8 @@ pub type OptimizationLevel = (); #[cfg(feature = "internals")] pub use types::dynamic::{AccessMode, DynamicReadLock, DynamicWriteLock, Variant}; -#[cfg(all(feature = "internals", not(feature = "no_float")))] +#[cfg(feature = "internals")] +#[cfg(not(feature = "no_float"))] pub use types::FloatWrapper; #[cfg(feature = "internals")] From 4fe80a202622bafafc480a38acc99c8f9d1a5726 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Feb 2023 23:42:13 +0800 Subject: [PATCH 3/8] Use &Token for op. --- src/api/deprecated.rs | 2 +- src/eval/stmt.rs | 6 +++--- src/func/builtin.rs | 4 ++-- src/func/call.rs | 14 +++++++------- src/func/native.rs | 4 ++-- src/optimizer.rs | 6 +++--- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/api/deprecated.rs b/src/api/deprecated.rs index dca53e94..47dd8d93 100644 --- a/src/api/deprecated.rs +++ b/src/api/deprecated.rs @@ -1133,7 +1133,7 @@ pub mod deprecated_array_functions { array: &mut Array, comparer: &str, ) -> RhaiResultOf<()> { - sort(ctx, array, FnPtr::new(comparer)?) + Ok(sort(ctx, array, FnPtr::new(comparer)?)) } /// Remove all elements in the array that returns `true` when applied a function named by `filter` /// and return them as a new array. diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index f9111ce6..ebd78364 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -141,7 +141,7 @@ impl Engine { if self.fast_operators() { if let Some((func, need_context)) = - get_builtin_op_assignment_fn(op_assign.clone(), args[0], args[1]) + get_builtin_op_assignment_fn(op_assign, args[0], args[1]) { // Built-in found auto_restore! { let orig_level = global.level; global.level += 1 } @@ -157,7 +157,7 @@ impl Engine { } } - let token = Some(op_assign.clone()); + let token = Some(op_assign); let op_assign = op_assign.literal_syntax(); match self.exec_native_fn_call(global, caches, op_assign, token, hash, args, true, *pos) @@ -166,7 +166,7 @@ impl Engine { Err(err) if matches!(*err, ERR::ErrorFunctionNotFound(ref f, ..) if f.starts_with(op_assign)) => { // Expand to `var = var op rhs` - let token = Some(op.clone()); + let token = Some(op); let op = op.literal_syntax(); *args[0] = self diff --git a/src/func/builtin.rs b/src/func/builtin.rs index 592297af..1b69ba43 100644 --- a/src/func/builtin.rs +++ b/src/func/builtin.rs @@ -86,7 +86,7 @@ fn const_false_fn(_: Option, _: &mut [&mut Dynamic]) -> RhaiR /// /// The return function will be registered as a _method_, so the first parameter cannot be consumed. #[must_use] -pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option { +pub fn get_builtin_binary_op_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Option { let type1 = x.type_id(); let type2 = y.type_id(); @@ -621,7 +621,7 @@ pub fn get_builtin_binary_op_fn(op: Token, x: &Dynamic, y: &Dynamic) -> Option Option { +pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Option { let type1 = x.type_id(); let type2 = y.type_id(); diff --git a/src/func/call.rs b/src/func/call.rs index 837d6fb7..40675ae1 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -164,7 +164,7 @@ impl Engine { _global: &GlobalRuntimeState, caches: &'s mut Caches, local_entry: &'s mut Option, - op_token: Option, + op_token: Option<&Token>, hash_base: u64, args: Option<&mut FnCallArgs>, allow_dynamic: bool, @@ -345,7 +345,7 @@ impl Engine { global: &mut GlobalRuntimeState, caches: &mut Caches, name: &str, - op_token: Option, + op_token: Option<&Token>, hash: u64, args: &mut FnCallArgs, is_ref_mut: bool, @@ -567,7 +567,7 @@ impl Engine { caches: &mut Caches, _scope: Option<&mut Scope>, fn_name: &str, - op_token: Option, + op_token: Option<&Token>, hashes: FnCallHashes, mut _args: &mut FnCallArgs, is_ref_mut: bool, @@ -1000,7 +1000,7 @@ impl Engine { scope: &mut Scope, mut this_ptr: Option<&mut Dynamic>, fn_name: &str, - op_token: Option, + op_token: Option<&Token>, first_arg: Option<&Expr>, args_expr: &[Expr], hashes: FnCallHashes, @@ -1564,10 +1564,10 @@ impl Engine { .. } = expr; - let op_token = op_token.clone(); + let op_token = op_token.as_ref(); // Short-circuit native unary operator call if under Fast Operators mode - if op_token == Some(Token::Bang) && self.fast_operators() && args.len() == 1 { + if op_token == Some(&Token::Bang) && self.fast_operators() && args.len() == 1 { let mut value = self .get_arg_value(global, caches, scope, this_ptr.as_deref_mut(), &args[0])? .0 @@ -1597,7 +1597,7 @@ impl Engine { let operands = &mut [&mut lhs, &mut rhs]; if let Some((func, need_context)) = - get_builtin_binary_op_fn(op_token.clone().unwrap(), operands[0], operands[1]) + get_builtin_binary_op_fn(op_token.as_ref().unwrap(), operands[0], operands[1]) { // Built-in found auto_restore! { let orig_level = global.level; global.level += 1 } diff --git a/src/func/native.rs b/src/func/native.rs index eaedd0ed..e776c4b0 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -446,7 +446,7 @@ impl<'a> NativeCallContext<'a> { global, caches, fn_name, - op_token, + op_token.as_ref(), calc_fn_hash(None, fn_name, args_len), args, is_ref_mut, @@ -473,7 +473,7 @@ impl<'a> NativeCallContext<'a> { caches, None, fn_name, - op_token, + op_token.as_ref(), hash, args, is_ref_mut, diff --git a/src/optimizer.rs b/src/optimizer.rs index 67211427..6de5862f 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -147,7 +147,7 @@ impl<'a> OptimizerState<'a> { pub fn call_fn_with_constant_arguments( &mut self, fn_name: &str, - op_token: Option, + op_token: Option<&Token>, arg_values: &mut [Dynamic], ) -> Option { self.engine @@ -1138,7 +1138,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { } // Overloaded operators can override built-in. _ if x.args.len() == 2 && x.op_token.is_some() && (state.engine.fast_operators() || !state.engine.has_native_fn_override(x.hashes.native(), &arg_types)) => { - if let Some(result) = get_builtin_binary_op_fn(x.op_token.clone().unwrap(), &arg_values[0], &arg_values[1]) + if let Some(result) = get_builtin_binary_op_fn(x.op_token.as_ref().unwrap(), &arg_values[0], &arg_values[1]) .and_then(|(f, ctx)| { let context = if ctx { Some((state.engine, x.name.as_str(), None, &state.global, *pos).into()) @@ -1193,7 +1193,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { KEYWORD_TYPE_OF if arg_values.len() == 1 => Some(state.engine.map_type_name(arg_values[0].type_name()).into()), #[cfg(not(feature = "no_closure"))] crate::engine::KEYWORD_IS_SHARED if arg_values.len() == 1 => Some(Dynamic::FALSE), - _ => state.call_fn_with_constant_arguments(&x.name, x.op_token.clone(), arg_values) + _ => state.call_fn_with_constant_arguments(&x.name, x.op_token.as_ref(), arg_values) }; if let Some(r) = result { From c58fe8710775c08e18188aaca1235bc6a962d853 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Fri, 10 Feb 2023 23:51:22 +0800 Subject: [PATCH 4/8] Fix error. --- src/packages/array_basic.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 7dabd569..0e968c4c 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -1468,10 +1468,9 @@ pub mod array_functions { /// /// print(x); // prints "[10, 9, 8, 7, 6, 5, 4, 3, 2, 1]" /// ``` - #[rhai_fn(return_raw)] - pub fn sort(ctx: NativeCallContext, array: &mut Array, comparer: FnPtr) -> RhaiResultOf<()> { + pub fn sort(ctx: NativeCallContext, array: &mut Array, comparer: FnPtr) { if array.len() <= 1 { - return Ok(()); + return; } array.sort_by(|x, y| { @@ -1489,8 +1488,6 @@ pub mod array_functions { }, ) }); - - Ok(()) } /// Sort the array. /// From 75718a5a8b3eeadd42297d2b7cf3caa2c8fc5160 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 Feb 2023 00:17:26 +0800 Subject: [PATCH 5/8] Satisfy clippy. --- src/ast/stmt.rs | 4 ++-- src/bin/rhai-dbg.rs | 9 +++------ src/eval/debugger.rs | 4 ++-- src/lib.rs | 9 ++++++--- src/module/mod.rs | 4 ++-- src/packages/blob_basic.rs | 4 +++- src/packages/lang_core.rs | 18 +++++++++--------- src/parser.rs | 8 ++++---- src/tokenizer.rs | 8 ++++---- 9 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index af4ff064..00ed80e2 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -93,7 +93,7 @@ impl OpAssignment { #[inline(always)] pub fn new_op_assignment_from_base(name: &str, pos: Position) -> Self { let op = Token::lookup_symbol_from_syntax(name).expect("operator"); - Self::new_op_assignment_from_base_token(op, pos) + Self::new_op_assignment_from_base_token(&op, pos) } /// Convert a [`Token`] into a new [`OpAssignment`]. /// @@ -102,7 +102,7 @@ impl OpAssignment { /// Panics if the token is cannot be converted into an op-assignment operator. #[inline(always)] #[must_use] - pub fn new_op_assignment_from_base_token(op: Token, pos: Position) -> Self { + pub fn new_op_assignment_from_base_token(op: &Token, pos: Position) -> Self { Self::new_op_assignment_from_token(op.convert_to_op_assignment().expect("operator"), pos) } } diff --git a/src/bin/rhai-dbg.rs b/src/bin/rhai-dbg.rs index db929a4b..6fedabb5 100644 --- a/src/bin/rhai-dbg.rs +++ b/src/bin/rhai-dbg.rs @@ -166,10 +166,7 @@ fn load_script(engine: &Engine) -> (rhai::AST, String) { let filename = match Path::new(&filename).canonicalize() { Err(err) => { - eprintln!( - "\x1b[31mError script file path: {}\n{}\x1b[39m", - filename, err - ); + eprintln!("\x1b[31mError script file path: {filename}\n{err}\x1b[39m"); exit(1); } Ok(f) => { @@ -315,7 +312,7 @@ fn debug_callback( } ["node"] => { if pos.is_none() { - println!("{:?}", node); + println!("{node:?}"); } else { match source { Some(source) => println!("{node:?} {source} @ {pos:?}"), @@ -400,7 +397,7 @@ fn debug_callback( #[cfg(not(feature = "no_position"))] rhai::debugger::BreakPoint::AtPosition { pos, .. } => { let line_num = format!("[{}] line ", i + 1); - print!("{}", line_num); + print!("{line_num}"); print_source(lines, *pos, line_num.len(), (0, 0)); } _ => println!("[{}] {bp}", i + 1), diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index 652134a3..0302bdc2 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -495,13 +495,13 @@ impl Engine { /// /// It is up to the [`Engine`] to reactivate the debugger. #[inline] - pub(crate) fn run_debugger_raw<'a>( + pub(crate) fn run_debugger_raw( &self, global: &mut GlobalRuntimeState, caches: &mut Caches, scope: &mut Scope, this_ptr: Option<&mut Dynamic>, - node: ASTNode<'a>, + node: ASTNode, event: DebuggerEvent, ) -> Result, Box> { if let Some(ref x) = self.debugger_interface { diff --git a/src/lib.rs b/src/lib.rs index a6852433..d27a2a4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,11 +80,14 @@ #![allow(clippy::upper_case_acronyms)] #![allow(clippy::match_same_arms)] // The lints below can be turned off to reduce signal/noise ratio -// #![allow(clippy::too_many_lines)] -// #![allow(clippy::let_underscore_drop)] +#![allow(clippy::too_many_lines)] +#![allow(clippy::let_underscore_drop)] #![allow(clippy::absurd_extreme_comparisons)] #![allow(clippy::unnecessary_cast)] -// #![allow(clippy::wildcard_imports)] +#![allow(clippy::wildcard_imports)] +#![allow(clippy::no_effect_underscore_binding)] +#![allow(clippy::semicolon_if_nothing_returned)] +#![allow(clippy::type_complexity)] #[cfg(feature = "no_std")] extern crate alloc; diff --git a/src/module/mod.rs b/src/module/mod.rs index e299b2a5..24cbc06e 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -233,7 +233,7 @@ impl fmt::Debug for Module { .modules .as_deref() .into_iter() - .flat_map(|m| m.keys()) + .flat_map(BTreeMap::keys) .map(SmartString::as_str) .collect::>(), ) @@ -2202,7 +2202,7 @@ impl Module { environ: ref mut e, .. } = f.func { - *e = Some(environ.clone()) + *e = Some(environ.clone()); } }); diff --git a/src/packages/blob_basic.rs b/src/packages/blob_basic.rs index 430b89c0..80449e92 100644 --- a/src/packages/blob_basic.rs +++ b/src/packages/blob_basic.rs @@ -513,11 +513,13 @@ pub mod blob_functions { /// /// print(b); // prints "[030405]" /// ``` - #[allow(clippy::cast_sign_loss, clippy::needless_pass_by_value)] + #[allow(clippy::cast_sign_loss, clippy::needless_pass_by_value, clippy::cast_possible_truncation)] pub fn chop(blob: &mut Blob, len: INT) { if !blob.is_empty() { if len <= 0 { blob.clear(); + } else if len > MAX_USIZE_INT { + // len > BLOB length } else if (len as usize) < blob.len() { blob.drain(0..blob.len() - len as usize); } diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs index 6b55f2e1..ce75f7ef 100644 --- a/src/packages/lang_core.rs +++ b/src/packages/lang_core.rs @@ -109,10 +109,10 @@ mod core_functions { /// ``` #[cfg(not(feature = "no_std"))] pub fn sleep(seconds: INT) { - if seconds <= 0 { - return; + if seconds > 0 { + #[allow(clippy::cast_sign_loss)] + std::thread::sleep(std::time::Duration::from_secs(seconds as u64)); } - std::thread::sleep(std::time::Duration::from_secs(seconds as u64)); } /// Parse a JSON string into a value. @@ -142,23 +142,23 @@ mod reflection_functions { /// Return an array of object maps containing metadata of all script-defined functions. pub fn get_fn_metadata_list(ctx: NativeCallContext) -> Array { - collect_fn_metadata(ctx, |_, _, _, _, _| true) + collect_fn_metadata(&ctx, |_, _, _, _, _| true) } /// Return an array of object maps containing metadata of all script-defined functions /// matching the specified name. #[rhai_fn(name = "get_fn_metadata_list")] pub fn get_fn_metadata(ctx: NativeCallContext, name: &str) -> Array { - collect_fn_metadata(ctx, |_, _, n, _, _| n == name) + collect_fn_metadata(&ctx, |_, _, n, _, _| n == name) } /// Return an array of object maps containing metadata of all script-defined functions /// matching the specified name and arity (number of parameters). #[rhai_fn(name = "get_fn_metadata_list")] #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] pub fn get_fn_metadata2(ctx: NativeCallContext, name: &str, params: INT) -> Array { - if !(0..=crate::MAX_USIZE_INT).contains(¶ms) { - Array::new() + if (0..=crate::MAX_USIZE_INT).contains(¶ms) { + collect_fn_metadata(&ctx, |_, _, n, p, _| p == (params as usize) && n == name) } else { - collect_fn_metadata(ctx, |_, _, n, p, _| p == (params as usize) && n == name) + Array::new() } } } @@ -167,7 +167,7 @@ mod reflection_functions { #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] fn collect_fn_metadata( - ctx: NativeCallContext, + ctx: &NativeCallContext, filter: impl Fn(FnNamespace, FnAccess, &str, usize, &crate::Shared) -> bool + Copy, ) -> crate::Array { diff --git a/src/parser.rs b/src/parser.rs index 9051b65d..465166ed 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3560,7 +3560,9 @@ impl Engine { // try { try_block } catch ( var ) { catch_block } let branch = self.parse_block(input, state, lib, settings)?.into(); - let expr = if !catch_var.is_empty() { + let expr = if catch_var.is_empty() { + Expr::Unit(catch_var.pos) + } else { // Remove the error variable from the stack state.stack.as_deref_mut().unwrap().pop(); @@ -3569,12 +3571,10 @@ impl Engine { None, catch_var.pos, ) - } else { - Expr::Unit(catch_var.pos) }; Ok(Stmt::TryCatch( - FlowControl { body, expr, branch }.into(), + FlowControl { expr, body, branch }.into(), settings.pos, )) } diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 5102ed9f..717b7282 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -1830,10 +1830,10 @@ fn get_next_token_inner( return Some((Token::NotIn, start_pos)); } } - } else { - stream.unget('i'); - return Some((Token::Bang, start_pos)); } + + stream.unget('i'); + return Some((Token::Bang, start_pos)); } ('!', '=') => { eat_next_and_advance(stream, pos); @@ -2137,7 +2137,7 @@ impl InputStream for MultiInputsStream<'_> { } fn peek_next(&mut self) -> Option { if let ch @ Some(..) = self.buf.last() { - return ch.cloned(); + return ch.copied(); } loop { From e177a5648a5a1d9d6f773d058c848751a97b4e56 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 Feb 2023 11:56:20 +0800 Subject: [PATCH 6/8] Remove hashing hack. --- src/ast/expr.rs | 19 +++++++--------- src/func/call.rs | 4 ---- src/func/hashing.rs | 53 ++++++--------------------------------------- src/module/mod.rs | 22 +++++++++---------- 4 files changed, 26 insertions(+), 72 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index c8cdedb1..53f186d8 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -2,7 +2,6 @@ use super::{ASTFlags, ASTNode, Ident, Namespace, Stmt, StmtBlock}; use crate::engine::{KEYWORD_FN_PTR, OP_EXCLUSIVE_RANGE, OP_INCLUSIVE_RANGE}; -use crate::func::hashing::ALT_ZERO_HASH; use crate::tokenizer::Token; use crate::types::dynamic::Union; use crate::{ @@ -17,7 +16,7 @@ use std::{ fmt::Write, hash::Hash, iter::once, - num::{NonZeroU64, NonZeroU8, NonZeroUsize}, + num::{NonZeroU8, NonZeroUsize}, }; /// _(internals)_ A binary expression. @@ -103,9 +102,9 @@ impl CustomExpr { pub struct FnCallHashes { /// Pre-calculated hash for a script-defined function ([`None`] if native functions only). #[cfg(not(feature = "no_function"))] - script: Option, + script: Option, /// Pre-calculated hash for a native Rust function with no parameter types. - native: NonZeroU64, + native: u64, } impl fmt::Debug for FnCallHashes { @@ -128,8 +127,6 @@ impl fmt::Debug for FnCallHashes { impl From for FnCallHashes { #[inline] fn from(hash: u64) -> Self { - let hash = NonZeroU64::new(if hash == 0 { ALT_ZERO_HASH } else { hash }).unwrap(); - Self { #[cfg(not(feature = "no_function"))] script: Some(hash), @@ -146,7 +143,7 @@ impl FnCallHashes { Self { #[cfg(not(feature = "no_function"))] script: None, - native: NonZeroU64::new(if hash == 0 { ALT_ZERO_HASH } else { hash }).unwrap(), + native: hash, } } /// Create a [`FnCallHashes`] with both native Rust and script function hashes. @@ -155,8 +152,8 @@ impl FnCallHashes { pub fn from_all(#[cfg(not(feature = "no_function"))] script: u64, native: u64) -> Self { Self { #[cfg(not(feature = "no_function"))] - script: NonZeroU64::new(if script == 0 { ALT_ZERO_HASH } else { script }), - native: NonZeroU64::new(if native == 0 { ALT_ZERO_HASH } else { native }).unwrap(), + script: Some(script), + native, } } /// Is this [`FnCallHashes`] native-only? @@ -174,7 +171,7 @@ impl FnCallHashes { #[inline(always)] #[must_use] pub const fn native(&self) -> u64 { - self.native.get() + self.native } /// Get the script hash. /// @@ -188,7 +185,7 @@ impl FnCallHashes { #[must_use] pub fn script(&self) -> u64 { assert!(self.script.is_some()); - self.script.as_ref().unwrap().get() + self.script.unwrap() } } diff --git a/src/func/call.rs b/src/func/call.rs index 40675ae1..0741a409 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -169,10 +169,6 @@ impl Engine { args: Option<&mut FnCallArgs>, allow_dynamic: bool, ) -> Option<&'s FnResolutionCacheEntry> { - if hash_base == 0 { - return None; - } - let mut hash = args.as_deref().map_or(hash_base, |args| { calc_fn_hash_full(hash_base, args.iter().map(|a| a.type_id())) }); diff --git a/src/func/hashing.rs b/src/func/hashing.rs index cffe5d9b..b3e04613 100644 --- a/src/func/hashing.rs +++ b/src/func/hashing.rs @@ -13,25 +13,7 @@ pub type StraightHashMap = hashbrown::HashMap; #[cfg(not(feature = "no_std"))] pub type StraightHashMap = std::collections::HashMap; - -/// Dummy hash value to map zeros to. This value can be anything. -/// -/// # Notes -/// -/// Hashes are `u64`, and they can be zero (although extremely unlikely). -/// It is possible to hijack the zero value to indicate non-existence, -/// like [`None`] in [`Option`]. -/// -/// When a hash is calculated to be zero, it gets mapped to this alternate hash value. -/// This has the effect of releasing the zero value at the expense of causing the probability of -/// this value to double, which has minor impacts. -pub const ALT_ZERO_HASH: u64 = 42; - -/// A hasher that only takes one single [`u64`] and returns it as a non-zero hash key. -/// -/// # Zeros -/// -/// If the value is zero, it is mapped to `ALT_ZERO_HASH`. +/// A hasher that only takes one single [`u64`] and returns it as a hash key. /// /// # Panics /// @@ -81,15 +63,11 @@ pub fn get_hasher() -> ahash::AHasher { } } -/// Calculate a non-zero [`u64`] hash key from a namespace-qualified variable name. +/// Calculate a [`u64`] hash key from a namespace-qualified variable name. /// /// Module names are passed in via `&str` references from an iterator. /// Parameter types are passed in via [`TypeId`] values from an iterator. /// -/// # Zeros -/// -/// If the hash happens to be zero, it is mapped to `ALT_ZERO_HASH`. -/// /// # Note /// /// The first module name is skipped. Hashing starts from the _second_ module in the chain. @@ -110,13 +88,10 @@ pub fn calc_var_hash<'a>(namespace: impl IntoIterator, var_name: count.hash(s); var_name.hash(s); - match s.finish() { - 0 => ALT_ZERO_HASH, - r => r, - } + s.finish() } -/// Calculate a non-zero [`u64`] hash key from a namespace-qualified function name +/// Calculate a [`u64`] hash key from a namespace-qualified function name /// and the number of parameters, but no parameter types. /// /// Module names making up the namespace are passed in via `&str` references from an iterator. @@ -124,10 +99,6 @@ pub fn calc_var_hash<'a>(namespace: impl IntoIterator, var_name: /// /// If the function is not namespace-qualified, pass [`None`] as the namespace. /// -/// # Zeros -/// -/// If the hash happens to be zero, it is mapped to `ALT_ZERO_HASH`. -/// /// # Note /// /// The first module name is skipped. Hashing starts from the _second_ module in the chain. @@ -152,19 +123,12 @@ pub fn calc_fn_hash<'a>( fn_name.hash(s); num.hash(s); - match s.finish() { - 0 => ALT_ZERO_HASH, - r => r, - } + s.finish() } -/// Calculate a non-zero [`u64`] hash key from a base [`u64`] hash key and a list of parameter types. +/// Calculate a [`u64`] hash key from a base [`u64`] hash key and a list of parameter types. /// /// Parameter types are passed in via [`TypeId`] values from an iterator. -/// -/// # Zeros -/// -/// If the hash happens to be zero, it is mapped to `ALT_ZERO_HASH`. #[inline] #[must_use] pub fn calc_fn_hash_full(base: u64, params: impl IntoIterator) -> u64 { @@ -177,8 +141,5 @@ pub fn calc_fn_hash_full(base: u64, params: impl IntoIterator) -> }); count.hash(s); - match s.finish() { - 0 => ALT_ZERO_HASH, - r => r, - } + s.finish() } diff --git a/src/module/mod.rs b/src/module/mod.rs index 24cbc06e..962938ec 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -145,7 +145,7 @@ impl FuncInfo { } } -/// _(internals)_ Calculate a non-zero [`u64`] hash key from a namespace-qualified function name and parameter types. +/// _(internals)_ Calculate a [`u64`] hash key from a namespace-qualified function name and parameter types. /// Exported under the `internals` feature only. /// /// Module names are passed in via `&str` references from an iterator. @@ -991,7 +991,7 @@ impl Module { type_id } - /// Set a Rust function into the [`Module`], returning a non-zero hash key. + /// Set a Rust function into the [`Module`], returning a [`u64`] hash key. /// /// If there is an existing Rust function of the same hash, it is replaced. /// @@ -1089,7 +1089,7 @@ impl Module { hash_fn } - /// _(metadata)_ Set a Rust function into the [`Module`], returning a non-zero hash key. + /// _(metadata)_ Set a Rust function into the [`Module`], returning a [`u64`] hash key. /// Exported under the `metadata` feature only. /// /// If there is an existing Rust function of the same hash, it is replaced. @@ -1144,7 +1144,7 @@ impl Module { /// Set a Rust function taking a reference to the scripting [`Engine`][crate::Engine], /// the current set of functions, plus a list of mutable [`Dynamic`] references - /// into the [`Module`], returning a non-zero hash key. + /// into the [`Module`], returning a [`u64`] hash key. /// /// Use this to register a built-in function which must reference settings on the scripting /// [`Engine`][crate::Engine] (e.g. to prevent growing an array beyond the allowed maximum size), @@ -1234,7 +1234,7 @@ impl Module { ) } - /// Set a Rust function into the [`Module`], returning a non-zero hash key. + /// Set a Rust function into the [`Module`], returning a [`u64`] hash key. /// /// If there is a similar existing Rust function, it is replaced. /// @@ -1287,7 +1287,7 @@ impl Module { ) } - /// Set a Rust getter function taking one mutable parameter, returning a non-zero hash key. + /// Set a Rust getter function taking one mutable parameter, returning a [`u64`] hash key. /// This function is automatically exposed to the global namespace. /// /// If there is a similar existing Rust getter function, it is replaced. @@ -1327,7 +1327,7 @@ impl Module { } /// Set a Rust setter function taking two parameters (the first one mutable) into the [`Module`], - /// returning a non-zero hash key. + /// returning a [`u64`] hash key. /// This function is automatically exposed to the global namespace. /// /// If there is a similar existing setter Rust function, it is replaced. @@ -1370,7 +1370,7 @@ impl Module { ) } - /// Set a pair of Rust getter and setter functions into the [`Module`], returning both non-zero hash keys. + /// Set a pair of Rust getter and setter functions into the [`Module`], returning both [`u64`] hash keys. /// This is a short-hand for [`set_getter_fn`][Module::set_getter_fn] and [`set_setter_fn`][Module::set_setter_fn]. /// /// These function are automatically exposed to the global namespace. @@ -1418,7 +1418,7 @@ impl Module { } /// Set a Rust index getter taking two parameters (the first one mutable) into the [`Module`], - /// returning a non-zero hash key. + /// returning a [`u64`] hash key. /// This function is automatically exposed to the global namespace. /// /// If there is a similar existing setter Rust function, it is replaced. @@ -1479,7 +1479,7 @@ impl Module { } /// Set a Rust index setter taking three parameters (the first one mutable) into the [`Module`], - /// returning a non-zero hash key. + /// returning a [`u64`] hash key. /// This function is automatically exposed to the global namespace. /// /// If there is a similar existing Rust function, it is replaced. @@ -1539,7 +1539,7 @@ impl Module { ) } - /// Set a pair of Rust index getter and setter functions into the [`Module`], returning both non-zero hash keys. + /// Set a pair of Rust index getter and setter functions into the [`Module`], returning both [`u64`] hash keys. /// This is a short-hand for [`set_indexer_get_fn`][Module::set_indexer_get_fn] and /// [`set_indexer_set_fn`][Module::set_indexer_set_fn]. /// From 9cb5154979f2c32169904a6f50b569c2b40ffdf6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 Feb 2023 12:52:07 +0800 Subject: [PATCH 7/8] Refactor FnCallHashes. --- src/ast/expr.rs | 21 ++++++++------- src/func/call.rs | 51 +++++++++++++++++++---------------- src/func/native.rs | 13 ++++----- src/parser.rs | 66 ++++++++++++++++++++++++++++------------------ 4 files changed, 88 insertions(+), 63 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 53f186d8..11a39eb2 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -124,34 +124,35 @@ impl fmt::Debug for FnCallHashes { } } -impl From for FnCallHashes { +impl FnCallHashes { + /// Create a [`FnCallHashes`] from a single hash. #[inline] - fn from(hash: u64) -> Self { + #[must_use] + pub const fn from_hash(hash: u64) -> Self { Self { #[cfg(not(feature = "no_function"))] script: Some(hash), native: hash, } } -} - -impl FnCallHashes { /// Create a [`FnCallHashes`] with only the native Rust hash. #[inline] #[must_use] - pub fn from_native(hash: u64) -> Self { + pub const fn from_native_only(hash: u64) -> Self { Self { #[cfg(not(feature = "no_function"))] script: None, native: hash, } } - /// Create a [`FnCallHashes`] with both native Rust and script function hashes. + /// Create a [`FnCallHashes`] with both script function and native Rust hashes. + /// + /// Not available under `no_function`. + #[cfg(not(feature = "no_function"))] #[inline] #[must_use] - pub fn from_all(#[cfg(not(feature = "no_function"))] script: u64, native: u64) -> Self { + pub const fn from_script_and_native(script: u64, native: u64) -> Self { Self { - #[cfg(not(feature = "no_function"))] script: Some(script), native, } @@ -577,7 +578,7 @@ impl Expr { FnCallExpr { namespace: Namespace::NONE, name: KEYWORD_FN_PTR.into(), - hashes: calc_fn_hash(None, f.fn_name(), 1).into(), + hashes: FnCallHashes::from_hash(calc_fn_hash(None, f.fn_name(), 1)), args: once(Self::StringConstant(f.fn_name().into(), pos)).collect(), capture_parent_scope: false, op_token: None, diff --git a/src/func/call.rs b/src/func/call.rs index 0741a409..bdcdf899 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -785,9 +785,9 @@ impl Engine { let fn_name = fn_ptr.fn_name(); // Recalculate hashes let new_hash = if !is_anon && !is_valid_function_name(fn_name) { - FnCallHashes::from_native(calc_fn_hash(None, fn_name, args.len())) + FnCallHashes::from_native_only(calc_fn_hash(None, fn_name, args.len())) } else { - calc_fn_hash(None, fn_name, args.len()).into() + FnCallHashes::from_hash(calc_fn_hash(None, fn_name, args.len())) }; // Map it to name(args) in function-call style @@ -867,14 +867,17 @@ impl Engine { args.insert(0, target.as_mut()); // Recalculate hash - let new_hash = if !is_anon && !is_valid_function_name(&fn_name) { - FnCallHashes::from_native(calc_fn_hash(None, &fn_name, args.len())) - } else { - FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] + let new_hash = match is_anon { + false if !is_valid_function_name(&fn_name) => { + FnCallHashes::from_native_only(calc_fn_hash(None, &fn_name, args.len())) + } + #[cfg(not(feature = "no_function"))] + _ => FnCallHashes::from_script_and_native( calc_fn_hash(None, &fn_name, args.len() - 1), calc_fn_hash(None, &fn_name, args.len()), - ) + ), + #[cfg(feature = "no_function")] + _ => FnCallHashes::from_native_only(calc_fn_hash(None, &fn_name, args.len())), }; // Map it to name(args) in function-call style @@ -943,18 +946,22 @@ impl Engine { call_args = &mut _arg_values; } // Recalculate the hash based on the new function name and new arguments - hash = if !is_anon && !is_valid_function_name(fn_name) { - FnCallHashes::from_native(calc_fn_hash( - None, - fn_name, - call_args.len() + 1, - )) - } else { - FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] - calc_fn_hash(None, fn_name, call_args.len()), - calc_fn_hash(None, fn_name, call_args.len() + 1), - ) + let args_len = call_args.len() + 1; + hash = match is_anon { + false if !is_valid_function_name(fn_name) => { + FnCallHashes::from_native_only(calc_fn_hash( + None, fn_name, args_len, + )) + } + #[cfg(not(feature = "no_function"))] + _ => FnCallHashes::from_script_and_native( + calc_fn_hash(None, fn_name, args_len - 1), + calc_fn_hash(None, fn_name, args_len), + ), + #[cfg(feature = "no_function")] + _ => FnCallHashes::from_native_only(calc_fn_hash( + None, fn_name, args_len, + )), }; } } @@ -1080,9 +1087,9 @@ impl Engine { let args_len = total_args + curry.len(); hashes = if !is_anon && !is_valid_function_name(name) { - FnCallHashes::from_native(calc_fn_hash(None, name, args_len)) + FnCallHashes::from_native_only(calc_fn_hash(None, name, args_len)) } else { - calc_fn_hash(None, name, args_len).into() + FnCallHashes::from_hash(calc_fn_hash(None, name, args_len)) }; } // Handle Fn() diff --git a/src/func/native.rs b/src/func/native.rs index e776c4b0..409e3948 100644 --- a/src/func/native.rs +++ b/src/func/native.rs @@ -457,14 +457,15 @@ impl<'a> NativeCallContext<'a> { // Native or script - let hash = if is_method_call { - FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] + let hash = match is_method_call { + #[cfg(not(feature = "no_function"))] + true => FnCallHashes::from_script_and_native( calc_fn_hash(None, fn_name, args_len - 1), calc_fn_hash(None, fn_name, args_len), - ) - } else { - calc_fn_hash(None, fn_name, args_len).into() + ), + #[cfg(feature = "no_function")] + true => FnCallHashes::from_native_only(calc_fn_hash(None, fn_name, args_len)), + _ => FnCallHashes::from_hash(calc_fn_hash(None, fn_name, args_len)), }; self.engine() diff --git a/src/parser.rs b/src/parser.rs index 465166ed..46c37a27 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -625,9 +625,9 @@ impl Engine { let hash = calc_fn_hash(None, &id, 0); let hashes = if is_valid_function_name(&id) { - hash.into() + FnCallHashes::from_hash(hash) } else { - FnCallHashes::from_native(hash) + FnCallHashes::from_native_only(hash) }; args.shrink_to_fit(); @@ -700,9 +700,9 @@ impl Engine { let hash = calc_fn_hash(None, &id, args.len()); let hashes = if is_valid_function_name(&id) { - hash.into() + FnCallHashes::from_hash(hash) } else { - FnCallHashes::from_native(hash) + FnCallHashes::from_native_only(hash) }; args.shrink_to_fit(); @@ -1952,7 +1952,7 @@ impl Engine { Ok(FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string("-"), - hashes: FnCallHashes::from_native(calc_fn_hash(None, "-", 1)), + hashes: FnCallHashes::from_native_only(calc_fn_hash(None, "-", 1)), args, op_token: Some(token), capture_parent_scope: false, @@ -1980,7 +1980,7 @@ impl Engine { Ok(FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string("+"), - hashes: FnCallHashes::from_native(calc_fn_hash(None, "+", 1)), + hashes: FnCallHashes::from_native_only(calc_fn_hash(None, "+", 1)), args, op_token: Some(token), capture_parent_scope: false, @@ -2001,7 +2001,7 @@ impl Engine { Ok(FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string("!"), - hashes: FnCallHashes::from_native(calc_fn_hash(None, "!", 1)), + hashes: FnCallHashes::from_native_only(calc_fn_hash(None, "!", 1)), args, op_token: Some(token), capture_parent_scope: false, @@ -2180,14 +2180,21 @@ impl Engine { // lhs.func(...) (lhs, Expr::FnCall(mut f, func_pos)) => { // Recalculate hash + let args_len = f.args.len() + 1; f.hashes = if is_valid_function_name(&f.name) { - FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] - calc_fn_hash(None, &f.name, f.args.len()), - calc_fn_hash(None, &f.name, f.args.len() + 1), - ) + #[cfg(not(feature = "no_function"))] + { + FnCallHashes::from_script_and_native( + calc_fn_hash(None, &f.name, args_len - 1), + calc_fn_hash(None, &f.name, args_len), + ) + } + #[cfg(feature = "no_function")] + { + FnCallHashes::from_native_only(calc_fn_hash(None, &f.name, args_len)) + } } else { - FnCallHashes::from_native(calc_fn_hash(None, &f.name, f.args.len() + 1)) + FnCallHashes::from_native_only(calc_fn_hash(None, &f.name, args_len)) }; let rhs = Expr::MethodCall(f, func_pos); @@ -2228,14 +2235,23 @@ impl Engine { // lhs.func().dot_rhs or lhs.func()[idx_rhs] Expr::FnCall(mut f, func_pos) => { // Recalculate hash + let args_len = f.args.len() + 1; f.hashes = if is_valid_function_name(&f.name) { - FnCallHashes::from_all( - #[cfg(not(feature = "no_function"))] - calc_fn_hash(None, &f.name, f.args.len()), - calc_fn_hash(None, &f.name, f.args.len() + 1), - ) + #[cfg(not(feature = "no_function"))] + { + FnCallHashes::from_script_and_native( + calc_fn_hash(None, &f.name, args_len - 1), + calc_fn_hash(None, &f.name, args_len), + ) + } + #[cfg(feature = "no_function")] + { + FnCallHashes::from_native_only(calc_fn_hash( + None, &f.name, args_len, + )) + } } else { - FnCallHashes::from_native(calc_fn_hash(None, &f.name, f.args.len() + 1)) + FnCallHashes::from_native_only(calc_fn_hash(None, &f.name, args_len)) }; let new_lhs = BinaryExpr { @@ -2351,7 +2367,7 @@ impl Engine { let mut op_base = FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string(&op), - hashes: FnCallHashes::from_native(hash), + hashes: FnCallHashes::from_native_only(hash), args, op_token: operator_token, capture_parent_scope: false, @@ -2394,7 +2410,7 @@ impl Engine { op_base.args.shrink_to_fit(); // Convert into a call to `contains` - op_base.hashes = calc_fn_hash(None, OP_CONTAINS, 2).into(); + op_base.hashes = FnCallHashes::from_hash(calc_fn_hash(None, OP_CONTAINS, 2)); op_base.name = state.get_interned_string(OP_CONTAINS); let fn_call = op_base.into_fn_call_expr(pos); @@ -2409,7 +2425,7 @@ impl Engine { let not_base = FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string(op), - hashes: FnCallHashes::from_native(calc_fn_hash(None, op, 1)), + hashes: FnCallHashes::from_native_only(calc_fn_hash(None, op, 1)), args, op_token: Some(Token::Bang), capture_parent_scope: false, @@ -2427,9 +2443,9 @@ impl Engine { .map_or(false, Option::is_some) => { op_base.hashes = if is_valid_script_function { - calc_fn_hash(None, &s, 2).into() + FnCallHashes::from_hash(calc_fn_hash(None, &s, 2)) } else { - FnCallHashes::from_native(calc_fn_hash(None, &s, 2)) + FnCallHashes::from_native_only(calc_fn_hash(None, &s, 2)) }; op_base.into_fn_call_expr(pos) } @@ -3709,7 +3725,7 @@ impl Engine { let expr = FnCallExpr { namespace: Namespace::NONE, name: state.get_interned_string(crate::engine::KEYWORD_FN_PTR_CURRY), - hashes: FnCallHashes::from_native(calc_fn_hash( + hashes: FnCallHashes::from_native_only(calc_fn_hash( None, crate::engine::KEYWORD_FN_PTR_CURRY, num_externals + 1, From 557b368fdbd74630318e9c2df59f5a1d7107e3b7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 11 Feb 2023 15:06:17 +0800 Subject: [PATCH 8/8] Fix op-assignment hashes. --- src/ast/expr.rs | 6 +++--- src/ast/stmt.rs | 37 ++++++++++++++++++++++++++++++------- src/eval/chaining.rs | 6 +++--- src/eval/stmt.rs | 20 ++++++-------------- src/packages/blob_basic.rs | 6 +++++- src/parser.rs | 20 ++++++++++---------- 6 files changed, 57 insertions(+), 38 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 11a39eb2..957a195e 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -5,8 +5,8 @@ use crate::engine::{KEYWORD_FN_PTR, OP_EXCLUSIVE_RANGE, OP_INCLUSIVE_RANGE}; use crate::tokenizer::Token; use crate::types::dynamic::Union; use crate::{ - calc_fn_hash, Dynamic, FnPtr, Identifier, ImmutableString, Position, SmartString, StaticVec, - INT, + calc_fn_hash, Dynamic, FnArgsVec, FnPtr, Identifier, ImmutableString, Position, SmartString, + StaticVec, INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -201,7 +201,7 @@ pub struct FnCallExpr { /// Pre-calculated hashes. pub hashes: FnCallHashes, /// List of function call argument expressions. - pub args: StaticVec, + pub args: FnArgsVec, /// Does this function call capture the parent scope? pub capture_parent_scope: bool, /// Is this function call a native operator? diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 00ed80e2..139475cf 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -24,15 +24,15 @@ use std::{ #[derive(Clone, PartialEq, Hash)] pub struct OpAssignment { /// Hash of the op-assignment call. - pub hash_op_assign: u64, + hash_op_assign: u64, /// Hash of the underlying operator call (for fallback). - pub hash_op: u64, + hash_op: u64, /// Op-assignment operator. - pub op_assign: Token, + op_assign: Token, /// Underlying operator. - pub op: Token, + op: Token, /// [Position] of the op-assignment operator. - pub pos: Position, + pos: Position, } impl OpAssignment { @@ -51,8 +51,31 @@ impl OpAssignment { /// Is this an op-assignment? #[must_use] #[inline(always)] - pub const fn is_op_assignment(&self) -> bool { - self.hash_op_assign != 0 || self.hash_op != 0 + pub fn is_op_assignment(&self) -> bool { + !matches!(self.op, Token::Equals) + } + /// Get information if this [`OpAssignment`] is an op-assignment. + /// + /// Returns `( hash_op_assign, hash_op, op_assign, op )`: + /// + /// * `hash_op_assign`: Hash of the op-assignment call. + /// * `hash_op`: Hash of the underlying operator call (for fallback). + /// * `op_assign`: Op-assignment operator. + /// * `op`: Underlying operator. + #[must_use] + #[inline] + pub fn get_op_assignment_info(&self) -> Option<(u64, u64, &Token, &Token)> { + if self.is_op_assignment() { + Some((self.hash_op_assign, self.hash_op, &self.op_assign, &self.op)) + } else { + None + } + } + /// Get the [position][Position] of this [`OpAssignment`]. + #[must_use] + #[inline(always)] + pub const fn position(&self) -> Position { + self.pos } /// Create a new [`OpAssignment`]. /// diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 6833e923..09066c8c 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -621,7 +621,7 @@ impl Engine { self.eval_op_assignment( global, caches, op_info, root, obj_ptr, new_val, )?; - self.check_data_size(obj_ptr.as_ref(), op_info.pos)?; + self.check_data_size(obj_ptr.as_ref(), op_info.position())?; None } // Indexed value cannot be referenced - use indexer @@ -647,7 +647,7 @@ impl Engine { )?; // Replace new value new_val = val.take_or_clone(); - self.check_data_size(&new_val, op_info.pos)?; + self.check_data_size(&new_val, op_info.position())?; } } @@ -729,7 +729,7 @@ impl Engine { global, caches, op_info, root, val_target, new_val, )?; } - self.check_data_size(target.source(), op_info.pos)?; + self.check_data_size(target.source(), op_info.position())?; Ok((Dynamic::UNIT, true)) } // {xxx:map}.id diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index ebd78364..417049b8 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -125,18 +125,10 @@ impl Engine { return Err(ERR::ErrorAssignmentToConstant(name.to_string(), pos).into()); } - if op_info.is_op_assignment() { - let OpAssignment { - hash_op_assign, - hash_op, - op_assign, - op, - pos, - } = op_info; + let pos = op_info.position(); + if let Some((hash1, hash2, op_assign, op)) = op_info.get_op_assignment_info() { let mut lock_guard = target.write_lock::().unwrap(); - - let hash = *hash_op_assign; let args = &mut [&mut *lock_guard, &mut new_val]; if self.fast_operators() { @@ -149,7 +141,7 @@ impl Engine { let context = if need_context { let op = op_assign.literal_syntax(); let source = global.source(); - Some((self, op, source, &*global, *pos).into()) + Some((self, op, source, &*global, pos).into()) } else { None }; @@ -160,7 +152,7 @@ impl Engine { let token = Some(op_assign); let op_assign = op_assign.literal_syntax(); - match self.exec_native_fn_call(global, caches, op_assign, token, hash, args, true, *pos) + match self.exec_native_fn_call(global, caches, op_assign, token, hash1, args, true, pos) { Ok(_) => (), Err(err) if matches!(*err, ERR::ErrorFunctionNotFound(ref f, ..) if f.starts_with(op_assign)) => @@ -170,7 +162,7 @@ impl Engine { let op = op.literal_syntax(); *args[0] = self - .exec_native_fn_call(global, caches, op, token, *hash_op, args, true, *pos)? + .exec_native_fn_call(global, caches, op, token, hash2, args, true, pos)? .0; } Err(err) => return Err(err), @@ -189,7 +181,7 @@ impl Engine { } } - target.propagate_changed_value(op_info.pos) + target.propagate_changed_value(pos) } /// Evaluate a statement. diff --git a/src/packages/blob_basic.rs b/src/packages/blob_basic.rs index 80449e92..a5af5296 100644 --- a/src/packages/blob_basic.rs +++ b/src/packages/blob_basic.rs @@ -513,7 +513,11 @@ pub mod blob_functions { /// /// print(b); // prints "[030405]" /// ``` - #[allow(clippy::cast_sign_loss, clippy::needless_pass_by_value, clippy::cast_possible_truncation)] + #[allow( + clippy::cast_sign_loss, + clippy::needless_pass_by_value, + clippy::cast_possible_truncation + )] pub fn chop(blob: &mut Blob, len: INT) { if !blob.is_empty() { if len <= 0 { diff --git a/src/parser.rs b/src/parser.rs index 46c37a27..9f8c6e1f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -17,9 +17,9 @@ use crate::tokenizer::{ use crate::types::dynamic::AccessMode; use crate::types::StringsInterner; use crate::{ - calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ExclusiveRange, Identifier, - ImmutableString, InclusiveRange, LexError, OptimizationLevel, ParseError, Position, Scope, - Shared, SmartString, StaticVec, AST, INT, PERR, + calc_fn_hash, Dynamic, Engine, EvalAltResult, EvalContext, ExclusiveRange, FnArgsVec, + Identifier, ImmutableString, InclusiveRange, LexError, OptimizationLevel, ParseError, Position, + Scope, Shared, SmartString, StaticVec, AST, INT, PERR, }; use bitflags::bitflags; #[cfg(feature = "no_std")] @@ -567,7 +567,7 @@ impl Engine { }; let mut _namespace = namespace; - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); match token { // id( @@ -1945,7 +1945,7 @@ impl Engine { // Call negative function expr => { - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); args.push(expr); args.shrink_to_fit(); @@ -1973,7 +1973,7 @@ impl Engine { // Call plus function expr => { - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); args.push(expr); args.shrink_to_fit(); @@ -1994,7 +1994,7 @@ impl Engine { let token = token.clone(); let pos = eat_token(input, Token::Bang); - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); args.push(self.parse_unary(input, state, lib, settings.level_up()?)?); args.shrink_to_fit(); @@ -2359,7 +2359,7 @@ impl Engine { Some(op_token.clone()) }; - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); args.push(root); args.push(rhs); args.shrink_to_fit(); @@ -2419,7 +2419,7 @@ impl Engine { } else { // Put a `!` call in front let op = Token::Bang.literal_syntax(); - let mut args = StaticVec::new_const(); + let mut args = FnArgsVec::new_const(); args.push(fn_call); let not_base = FnCallExpr { @@ -3708,7 +3708,7 @@ impl Engine { } let num_externals = externals.len(); - let mut args = StaticVec::with_capacity(externals.len() + 1); + let mut args = FnArgsVec::with_capacity(externals.len() + 1); args.push(fn_expr);