From bf5d6ab35afb8a4da2022022d507cf3f9822bc4a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 27 Aug 2022 16:26:41 +0800 Subject: [PATCH 1/8] Shut up clippy. --- src/api/custom_syntax.rs | 5 +- src/api/definitions/mod.rs | 10 +-- src/api/events.rs | 2 +- src/api/type_names.rs | 7 +- src/ast/ast.rs | 12 ++-- src/ast/expr.rs | 28 +++----- src/ast/flags.rs | 4 +- src/ast/stmt.rs | 42 ++++++------ src/engine.rs | 21 +++--- src/eval/chaining.rs | 45 ++++++++----- src/eval/data_check.rs | 2 +- src/eval/expr.rs | 33 +++++----- src/eval/stmt.rs | 10 +-- src/eval/target.rs | 6 +- src/func/builtin.rs | 17 +++-- src/func/call.rs | 6 +- src/func/register.rs | 4 +- src/lib.rs | 25 ++++++++ src/optimizer.rs | 27 ++++---- src/packages/array_basic.rs | 8 ++- src/packages/blob_basic.rs | 14 ++-- src/packages/iter_basic.rs | 19 +++--- src/packages/lang_core.rs | 4 +- src/packages/string_more.rs | 124 ++++++++++++++++++++++++++++-------- src/parser.rs | 24 +++---- src/reify.rs | 1 + src/tokenizer.rs | 16 ++--- src/types/dynamic.rs | 2 +- 28 files changed, 313 insertions(+), 205 deletions(-) diff --git a/src/api/custom_syntax.rs b/src/api/custom_syntax.rs index 64954d27..26abf716 100644 --- a/src/api/custom_syntax.rs +++ b/src/api/custom_syntax.rs @@ -134,10 +134,7 @@ impl Expression<'_> { Expr::CharConstant(x, ..) => reify!(*x => Option), Expr::StringConstant(x, ..) => reify!(x.clone() => Option), - Expr::Variable(x, ..) => { - let x: ImmutableString = x.3.clone().into(); - reify!(x => Option) - } + Expr::Variable(x, ..) => reify!(x.3.clone() => Option), Expr::BoolConstant(x, ..) => reify!(*x => Option), Expr::Unit(..) => reify!(() => Option), diff --git a/src/api/definitions/mod.rs b/src/api/definitions/mod.rs index e0caef31..8cb3b3ee 100644 --- a/src/api/definitions/mod.rs +++ b/src/api/definitions/mod.rs @@ -3,9 +3,8 @@ #![cfg(feature = "metadata")] use crate::module::FuncInfo; -use crate::plugin::*; use crate::tokenizer::{is_valid_function_name, Token}; -use crate::{Engine, Module, Scope, INT}; +use crate::{Engine, FnAccess, Module, Scope, INT}; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -117,19 +116,20 @@ impl Definitions<'_> { } /// Get the [`Engine`]. #[inline(always)] - pub fn engine(&self) -> &Engine { + #[must_use] + pub const fn engine(&self) -> &Engine { self.engine } /// Get the [`Scope`]. #[inline(always)] #[must_use] - pub fn scope(&self) -> Option<&Scope> { + pub const fn scope(&self) -> Option<&Scope> { self.scope } /// Get the configuration. #[inline(always)] #[must_use] - pub(crate) fn config(&self) -> &DefinitionsConfig { + pub(crate) const fn config(&self) -> &DefinitionsConfig { &self.config } } diff --git a/src/api/events.rs b/src/api/events.rs index 266c70c8..01463d2b 100644 --- a/src/api/events.rs +++ b/src/api/events.rs @@ -348,7 +348,7 @@ impl Engine { #[inline(always)] pub fn register_debugger( &mut self, - init: impl Fn(&Engine) -> Dynamic + SendSync + 'static, + init: impl Fn(&Self) -> Dynamic + SendSync + 'static, callback: impl Fn( EvalContext, crate::eval::DebuggerEvent, diff --git a/src/api/type_names.rs b/src/api/type_names.rs index 7d802376..de9120ac 100644 --- a/src/api/type_names.rs +++ b/src/api/type_names.rs @@ -102,11 +102,8 @@ fn map_std_type_name(name: &str, shorthands: bool) -> &str { }; } - if let Some(stripped) = name.strip_prefix("rhai::") { - map_std_type_name(stripped, shorthands) - } else { - name - } + name.strip_prefix("rhai::") + .map_or(name, |s| map_std_type_name(s, shorthands)) } impl Engine { diff --git a/src/ast/ast.rs b/src/ast/ast.rs index cabd7c10..da0454fe 100644 --- a/src/ast/ast.rs +++ b/src/ast/ast.rs @@ -167,7 +167,7 @@ impl AST { /// Get a reference to the source. #[inline(always)] #[must_use] - pub(crate) fn source_raw(&self) -> &Identifier { + pub(crate) const fn source_raw(&self) -> &Identifier { &self.source } /// Set the source. @@ -261,7 +261,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] #[must_use] - pub(crate) fn shared_lib(&self) -> &crate::Shared { + pub(crate) const fn shared_lib(&self) -> &crate::Shared { &self.lib } /// _(internals)_ Get the internal shared [`Module`][crate::Module] containing all script-defined functions. @@ -272,7 +272,7 @@ impl AST { #[cfg(not(feature = "no_function"))] #[inline(always)] #[must_use] - pub fn shared_lib(&self) -> &crate::Shared { + pub const fn shared_lib(&self) -> &crate::Shared { &self.lib } /// Get the embedded [module resolver][crate::ModuleResolver]. @@ -280,7 +280,7 @@ impl AST { #[cfg(not(feature = "no_module"))] #[inline(always)] #[must_use] - pub(crate) fn resolver( + pub(crate) const fn resolver( &self, ) -> Option<&crate::Shared> { self.resolver.as_ref() @@ -293,7 +293,7 @@ impl AST { #[cfg(not(feature = "no_module"))] #[inline(always)] #[must_use] - pub fn resolver( + pub const fn resolver( &self, ) -> Option<&crate::Shared> { self.resolver.as_ref() @@ -910,7 +910,7 @@ impl> Add for &AST { } } -impl> AddAssign for AST { +impl> AddAssign for AST { #[inline(always)] fn add_assign(&mut self, rhs: A) { self.combine(rhs.into()); diff --git a/src/ast/expr.rs b/src/ast/expr.rs index e86531d0..e2f66fd0 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -338,17 +338,7 @@ impl FloatWrapper { /// Create a new [`FloatWrapper`]. #[inline(always)] #[must_use] - pub fn new(value: F) -> Self { - Self(value) - } -} - -#[cfg(not(feature = "no_float"))] -impl FloatWrapper { - /// Create a new [`FloatWrapper`]. - #[inline(always)] - #[must_use] - pub const fn new_const(value: crate::FLOAT) -> Self { + pub const fn new(value: F) -> Self { Self(value) } } @@ -600,7 +590,7 @@ impl Expr { Self::FnCall(ref x, ..) if !x.is_qualified() && x.args.len() == 1 && x.name == KEYWORD_FN_PTR => { - if let Expr::StringConstant(ref s, ..) = x.args[0] { + if let Self::StringConstant(ref s, ..) = x.args[0] { FnPtr::new(s).ok()?.into() } else { return None; @@ -612,8 +602,8 @@ impl Expr { match x.name.as_str() { // x..y OP_EXCLUSIVE_RANGE => { - if let Expr::IntegerConstant(ref start, ..) = x.args[0] { - if let Expr::IntegerConstant(ref end, ..) = x.args[1] { + if let Self::IntegerConstant(ref start, ..) = x.args[0] { + if let Self::IntegerConstant(ref end, ..) = x.args[1] { (*start..*end).into() } else { return None; @@ -624,8 +614,8 @@ impl Expr { } // x..=y OP_INCLUSIVE_RANGE => { - if let Expr::IntegerConstant(ref start, ..) = x.args[0] { - if let Expr::IntegerConstant(ref end, ..) = x.args[1] { + if let Self::IntegerConstant(ref start, ..) = x.args[0] { + if let Self::IntegerConstant(ref end, ..) = x.args[1] { (*start..=*end).into() } else { return None; @@ -940,9 +930,9 @@ impl Expr { } Self::Index(x, ..) | Self::Dot(x, ..) - | Expr::And(x, ..) - | Expr::Or(x, ..) - | Expr::Coalesce(x, ..) => { + | Self::And(x, ..) + | Self::Or(x, ..) + | Self::Coalesce(x, ..) => { if !x.lhs.walk(path, on_node) { return false; } diff --git a/src/ast/flags.rs b/src/ast/flags.rs index 5d90f261..452a57e6 100644 --- a/src/ast/flags.rs +++ b/src/ast/flags.rs @@ -20,7 +20,7 @@ impl FnAccess { /// Is this function private? #[inline(always)] #[must_use] - pub fn is_private(self) -> bool { + pub const fn is_private(self) -> bool { match self { Self::Private => true, Self::Public => false, @@ -29,7 +29,7 @@ impl FnAccess { /// Is this function public? #[inline(always)] #[must_use] - pub fn is_public(self) -> bool { + pub const fn is_public(self) -> bool { match self { Self::Private => false, Self::Public => true, diff --git a/src/ast/stmt.rs b/src/ast/stmt.rs index 4a5ebcbf..a9d3b096 100644 --- a/src/ast/stmt.rs +++ b/src/ast/stmt.rs @@ -60,7 +60,7 @@ impl OpAssignment { #[must_use] #[inline(always)] pub fn new_op_assignment(name: &str, pos: Position) -> Self { - Self::new_op_assignment_from_token(Token::lookup_from_syntax(name).expect("operator"), pos) + Self::new_op_assignment_from_token(&Token::lookup_from_syntax(name).expect("operator"), pos) } /// Create a new [`OpAssignment`] from a [`Token`]. /// @@ -68,7 +68,7 @@ impl OpAssignment { /// /// Panics if the token is not an op-assignment operator. #[must_use] - pub fn new_op_assignment_from_token(op: Token, pos: Position) -> Self { + pub fn new_op_assignment_from_token(op: &Token, pos: Position) -> Self { let op_raw = op .get_base_op_from_assignment() .expect("op-assignment operator") @@ -90,7 +90,7 @@ impl OpAssignment { #[inline(always)] pub fn new_op_assignment_from_base(name: &str, pos: Position) -> Self { Self::new_op_assignment_from_base_token( - Token::lookup_from_syntax(name).expect("operator"), + &Token::lookup_from_syntax(name).expect("operator"), pos, ) } @@ -101,8 +101,8 @@ 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 { - Self::new_op_assignment_from_token(op.convert_to_op_assignment().expect("operator"), pos) + 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) } } @@ -157,13 +157,13 @@ impl ConditionalExpr { /// Is the condition always `true`? #[inline(always)] #[must_use] - pub fn is_always_true(&self) -> bool { + pub const fn is_always_true(&self) -> bool { matches!(self.condition, Expr::BoolConstant(true, ..)) } /// Is the condition always `false`? #[inline(always)] #[must_use] - pub fn is_always_false(&self) -> bool { + pub const fn is_always_false(&self) -> bool { matches!(self.condition, Expr::BoolConstant(false, ..)) } } @@ -228,12 +228,12 @@ impl RangeCase { /// Size of the range. #[inline(always)] #[must_use] - pub fn len(&self) -> usize { + pub fn len(&self) -> INT { match self { Self::ExclusiveInt(r, ..) if r.is_empty() => 0, - Self::ExclusiveInt(r, ..) => (r.end - r.start) as usize, + Self::ExclusiveInt(r, ..) => r.end - r.start, Self::InclusiveInt(r, ..) if r.is_empty() => 0, - Self::InclusiveInt(r, ..) => (*r.end() - *r.start()) as usize, + Self::InclusiveInt(r, ..) => *r.end() - *r.start() + 1, } } /// Is the specified number within this range? @@ -248,7 +248,7 @@ impl RangeCase { /// Is the specified range inclusive? #[inline(always)] #[must_use] - pub fn is_inclusive(&self) -> bool { + pub const fn is_inclusive(&self) -> bool { match self { Self::ExclusiveInt(..) => false, Self::InclusiveInt(..) => true, @@ -257,7 +257,7 @@ impl RangeCase { /// Get the index to the [`ConditionalExpr`]. #[inline(always)] #[must_use] - pub fn index(&self) -> usize { + pub const fn index(&self) -> usize { match self { Self::ExclusiveInt(.., n) | Self::InclusiveInt(.., n) => *n, } @@ -611,14 +611,14 @@ impl From for Stmt { } } -impl> From<(T, Position, Position)> for Stmt { +impl> From<(T, Position, Position)> for Stmt { #[inline(always)] fn from(value: (T, Position, Position)) -> Self { StmtBlock::new(value.0, value.1, value.2).into() } } -impl> From<(T, Span)> for Stmt { +impl> From<(T, Span)> for Stmt { #[inline(always)] fn from(value: (T, Span)) -> Self { StmtBlock::new_with_span(value.0, value.1).into() @@ -765,7 +765,7 @@ impl Stmt { Self::Noop(..) => true, Self::Expr(expr) => expr.is_pure(), Self::If(x, ..) => { - x.0.is_pure() && x.1.iter().all(Stmt::is_pure) && x.2.iter().all(Stmt::is_pure) + x.0.is_pure() && x.1.iter().all(Self::is_pure) && x.2.iter().all(Self::is_pure) } Self::Switch(x, ..) => { let (expr, sw) = &**x; @@ -786,7 +786,7 @@ impl Stmt { Self::While(x, ..) if matches!(x.0, Expr::BoolConstant(false, ..)) => true, Self::Do(x, options, ..) if matches!(x.0, Expr::BoolConstant(..)) => match x.0 { Expr::BoolConstant(cond, ..) if cond == options.contains(ASTFlags::NEGATED) => { - x.1.iter().all(Stmt::is_pure) + x.1.iter().all(Self::is_pure) } _ => false, }, @@ -796,13 +796,13 @@ impl Stmt { // For loops can be pure because if the iterable is pure, it is finite, // so infinite loops can never occur. - Self::For(x, ..) => x.2.is_pure() && x.3.iter().all(Stmt::is_pure), + Self::For(x, ..) => x.2.is_pure() && x.3.iter().all(Self::is_pure), Self::Var(..) | Self::Assignment(..) | Self::FnCall(..) => false, - Self::Block(block, ..) => block.iter().all(Stmt::is_pure), + Self::Block(block, ..) => block.iter().all(Self::is_pure), Self::BreakLoop(..) | Self::Return(..) => false, Self::TryCatch(x, ..) => { - x.try_block.iter().all(Stmt::is_pure) && x.catch_block.iter().all(Stmt::is_pure) + x.try_block.iter().all(Self::is_pure) && x.catch_block.iter().all(Self::is_pure) } #[cfg(not(feature = "no_module"))] @@ -828,7 +828,7 @@ impl Stmt { Self::Var(..) => true, Self::Expr(e) => match &**e { - Expr::Stmt(s) => s.iter().all(Stmt::is_block_dependent), + Expr::Stmt(s) => s.iter().all(Self::is_block_dependent), Expr::FnCall(x, ..) => !x.is_qualified() && x.name == KEYWORD_EVAL, _ => false, }, @@ -854,7 +854,7 @@ impl Stmt { Self::Var(x, ..) => x.1.is_pure(), Self::Expr(e) => match &**e { - Expr::Stmt(s) => s.iter().all(Stmt::is_internally_pure), + Expr::Stmt(s) => s.iter().all(Self::is_internally_pure), _ => self.is_pure(), }, diff --git a/src/engine.rs b/src/engine.rs index 1fea8e29..84c85701 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -237,13 +237,16 @@ impl Engine { { engine.print = Box::new(|s| println!("{}", s)); engine.debug = Box::new(|s, source, pos| { - if let Some(source) = source { - println!("{} @ {:?} | {}", source, pos, s); - } else if pos.is_none() { - println!("{}", s); - } else { - println!("{:?} | {}", pos, s); - } + source.map_or_else( + || { + if pos.is_none() { + println!("{}", s); + } else { + println!("{:?} | {}", pos, s); + } + }, + |source| println!("{} @ {:?} | {}", source, pos, s), + ) }); } @@ -316,7 +319,7 @@ impl Engine { &self, string: impl AsRef + Into, ) -> ImmutableString { - locked_write(&self.interned_strings).get(string).into() + locked_write(&self.interned_strings).get(string) } /// _(internals)_ Get an interned [string][ImmutableString]. @@ -331,7 +334,7 @@ impl Engine { &self, string: impl AsRef + Into, ) -> ImmutableString { - locked_write(&self.interned_strings).get(string).into() + locked_write(&self.interned_strings).get(string) } /// Get an empty [`ImmutableString`] which refers to a shared instance. diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 93720d79..351df42e 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -152,14 +152,14 @@ impl Engine { if let Ok(val) = self.call_indexer_get(global, caches, lib, target, idx, level) { - let mut res = val.into(); + let mut val = val.into(); // Run the op-assignment self.eval_op_assignment( - global, caches, lib, op_info, &mut res, root, new_val, + global, caches, lib, op_info, &mut val, root, new_val, level, )?; // Replace new value - new_val = res.take_or_clone(); + new_val = val.take_or_clone(); #[cfg(not(feature = "unchecked"))] self.check_data_size(&new_val, op_info.pos)?; } @@ -864,13 +864,16 @@ impl Engine { map.insert(index.clone().into(), Dynamic::UNIT); } - if let Some(value) = map.get_mut(index.as_str()) { - Ok(Target::from(value)) - } else if self.fail_on_invalid_map_property() { - Err(ERR::ErrorPropertyNotFound(index.to_string(), idx_pos).into()) - } else { - Ok(Target::from(Dynamic::UNIT)) - } + map.get_mut(index.as_str()).map_or_else( + || { + if self.fail_on_invalid_map_property() { + Err(ERR::ErrorPropertyNotFound(index.to_string(), idx_pos).into()) + } else { + Ok(Target::from(Dynamic::UNIT)) + } + }, + |value| Ok(Target::from(value)), + ) } #[cfg(not(feature = "no_index"))] @@ -970,21 +973,33 @@ impl Engine { .map_err(|typ| self.make_type_mismatch_err::(typ, idx_pos))?; let (ch, offset) = if index >= 0 { + if index >= crate::MAX_USIZE_INT { + return Err( + ERR::ErrorStringBounds(s.chars().count(), index, idx_pos).into() + ); + } + let offset = index as usize; ( s.chars().nth(offset).ok_or_else(|| { - let chars_len = s.chars().count(); - ERR::ErrorStringBounds(chars_len, index, idx_pos) + ERR::ErrorStringBounds(s.chars().count(), index, idx_pos) })?, offset, ) } else { - let offset = index.unsigned_abs() as usize; + let abs_index = index.unsigned_abs(); + + if abs_index as u64 >= usize::MAX as u64 { + return Err( + ERR::ErrorStringBounds(s.chars().count(), index, idx_pos).into() + ); + } + + let offset = abs_index as usize; ( // Count from end if negative s.chars().rev().nth(offset - 1).ok_or_else(|| { - let chars_len = s.chars().count(); - ERR::ErrorStringBounds(chars_len, index, idx_pos) + ERR::ErrorStringBounds(s.chars().count(), index, idx_pos) })?, offset, ) diff --git a/src/eval/data_check.rs b/src/eval/data_check.rs index a9cda2bc..293b1d99 100644 --- a/src/eval/data_check.rs +++ b/src/eval/data_check.rs @@ -72,7 +72,7 @@ impl Engine { /// Is there a data size limit set? #[cfg(not(feature = "unchecked"))] - pub(crate) fn has_data_size_limit(&self) -> bool { + pub(crate) const fn has_data_size_limit(&self) -> bool { let mut _limited = self.limits.max_string_size.is_some(); #[cfg(not(feature = "no_index"))] diff --git a/src/eval/expr.rs b/src/eval/expr.rs index 51265efc..ab499a6a 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -74,19 +74,22 @@ impl Engine { (_, namespace, hash_var, var_name) => { // foo:bar::baz::VARIABLE if let Some(module) = self.search_imports(global, namespace) { - return if let Some(mut target) = module.get_qualified_var(*hash_var) { - // Module variables are constant - target.set_access_mode(AccessMode::ReadOnly); - Ok((target.into(), *_var_pos)) - } else { - let sep = crate::tokenizer::Token::DoubleColon.literal_syntax(); + return module.get_qualified_var(*hash_var).map_or_else( + || { + let sep = crate::tokenizer::Token::DoubleColon.literal_syntax(); - Err(ERR::ErrorVariableNotFound( - format!("{namespace}{sep}{var_name}"), - namespace.position(), - ) - .into()) - }; + Err(ERR::ErrorVariableNotFound( + format!("{namespace}{sep}{var_name}"), + namespace.position(), + ) + .into()) + }, + |mut target| { + // Module variables are constant + target.set_access_mode(AccessMode::ReadOnly); + Ok((target.into(), *_var_pos)) + }, + ); } // global::VARIABLE @@ -363,7 +366,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Expr::Array(x, ..) => { - let mut arr = crate::Array::with_capacity(x.len()); + let mut array = crate::Array::with_capacity(x.len()); let mut result = Ok(Dynamic::UNIT); #[cfg(not(feature = "unchecked"))] @@ -383,7 +386,7 @@ impl Engine { #[cfg(not(feature = "unchecked"))] let val_sizes = Self::calc_data_sizes(&value, true); - arr.push(value); + array.push(value); #[cfg(not(feature = "unchecked"))] if self.has_data_size_limit() { @@ -396,7 +399,7 @@ impl Engine { } } - result.map(|_| arr.into()) + result.map(|_| array.into()) } #[cfg(not(feature = "no_object"))] diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 18d16eba..1003e896 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -631,8 +631,12 @@ impl Engine { for (x, iter_value) in func(iter_obj).enumerate() { // Increment counter if counter_index < usize::MAX { + // As the variable increments from 0, this should always work + // since any overflow will first be caught below. + let index_value = x as INT; + #[cfg(not(feature = "unchecked"))] - if x > INT::MAX as usize { + if index_value > crate::MAX_USIZE_INT { loop_result = Err(ERR::ErrorArithmetic( format!("for-loop counter overflow: {x}"), counter.pos, @@ -641,10 +645,8 @@ impl Engine { break; } - let index_value = Dynamic::from(x as INT); - *scope.get_mut_by_index(counter_index).write_lock().unwrap() = - index_value; + Dynamic::from_int(index_value); } let value = match iter_value { diff --git a/src/eval/target.rs b/src/eval/target.rs index e8d1678a..0114fa5b 100644 --- a/src/eval/target.rs +++ b/src/eval/target.rs @@ -16,7 +16,7 @@ use std::prelude::v1::*; pub fn calc_offset_len(length: usize, start: crate::INT, len: crate::INT) -> (usize, usize) { let start = if start < 0 { length - usize::min(start.unsigned_abs() as usize, length) - } else if start as usize >= length { + } else if start > crate::MAX_USIZE_INT || start as usize >= length { return (length, 0); } else { start as usize @@ -24,7 +24,7 @@ pub fn calc_offset_len(length: usize, start: crate::INT, len: crate::INT) -> (us let len = if len <= 0 { 0 - } else if len as usize > length - start { + } else if len > crate::MAX_USIZE_INT || len as usize > length - start { length - start } else { len as usize @@ -59,7 +59,7 @@ pub fn calc_index( } else { err() } - } else if start as usize >= length { + } else if start > crate::MAX_USIZE_INT || start as usize >= length { err() } else { Ok(start as usize) diff --git a/src/func/builtin.rs b/src/func/builtin.rs index 4856c61f..d97d1a27 100644 --- a/src/func/builtin.rs +++ b/src/func/builtin.rs @@ -700,16 +700,15 @@ pub fn get_builtin_op_assignment_fn(op: &str, x: &Dynamic, y: &Dynamic) -> Optio }), _ => None, }; - } else { - return match op { - "+=" => Some(|_, args| { - let x = std::mem::take(args[1]); - let array = &mut *args[0].write_lock::().expect(BUILTIN); - Ok(push(array, x).into()) - }), - _ => None, - }; } + return match op { + "+=" => Some(|_, args| { + let x = std::mem::take(args[1]); + let array = &mut *args[0].write_lock::().expect(BUILTIN); + Ok(push(array, x).into()) + }), + _ => None, + }; } #[cfg(not(feature = "no_index"))] diff --git a/src/func/call.rs b/src/func/call.rs index eb1bffda..071bb5c7 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -68,7 +68,7 @@ impl<'a> ArgBackup<'a> { // Replace the first reference with a reference to the clone, force-casting the lifetime. // Must remember to restore it later with `restore_first_arg`. // - // # Safety + // SAFETY: // // Blindly casting a reference to another lifetime saves allocation and string cloning, // but must be used with the utmost care. @@ -608,7 +608,7 @@ impl Engine { let num_params = args[1].as_int().expect("`INT`"); return Ok(( - if num_params < 0 { + if num_params < 0 || num_params > crate::MAX_USIZE_INT { false } else { let hash_script = calc_fn_hash(fn_name.as_str(), num_params as usize); @@ -1100,7 +1100,7 @@ impl Engine { .as_int() .map_err(|typ| self.make_type_mismatch_err::(typ, arg_pos))?; - return Ok(if num_params < 0 { + return Ok(if num_params < 0 || num_params > crate::MAX_USIZE_INT { false } else { let hash_script = calc_fn_hash(&fn_name, num_params as usize); diff --git a/src/func/register.rs b/src/func/register.rs index 40035b64..c282399f 100644 --- a/src/func/register.rs +++ b/src/func/register.rs @@ -47,9 +47,7 @@ pub fn by_value(data: &mut Dynamic) -> T { // If T is `&str`, data must be `ImmutableString`, so map directly to it data.flatten_in_place(); let ref_str = data.as_str_ref().expect("&str"); - // # Safety - // - // We already checked that `T` is `&str`, so it is safe to cast here. + // SAFETY: We already checked that `T` is `&str`, so it is safe to cast here. return unsafe { mem::transmute_copy::<_, T>(&ref_str) }; } if TypeId::of::() == TypeId::of::() { diff --git a/src/lib.rs b/src/lib.rs index 715dd979..60fc68da 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,7 +58,18 @@ #![cfg_attr(feature = "no_std", no_std)] #![deny(missing_docs)] +#![warn(clippy::all)] +#![warn(clippy::pedantic)] +#![warn(clippy::nursery)] +#![warn(clippy::cargo)] +#![warn(clippy::undocumented_unsafe_blocks)] #![allow(clippy::unit_arg)] +#![allow(clippy::missing_errors_doc)] +#![allow(clippy::used_underscore_binding)] +#![allow(clippy::inline_always)] +#![allow(clippy::module_name_repetitions)] +#![allow(clippy::negative_feature_names)] +#![allow(clippy::module_inception)] #[cfg(feature = "no_std")] extern crate alloc; @@ -123,6 +134,20 @@ type UNSIGNED_INT = u64; #[allow(non_camel_case_types)] type UNSIGNED_INT = u32; +/// The maximum integer that can fit into a [`usize`]. +#[cfg(not(target_pointer_width = "32"))] +const MAX_USIZE_INT: INT = INT::MAX; + +/// The maximum integer that can fit into a [`usize`]. +#[cfg(not(feature = "only_i32"))] +#[cfg(target_pointer_width = "32")] +const MAX_USIZE_INT: INT = usize::MAX as INT; + +/// The maximum integer that can fit into a [`usize`]. +#[cfg(feature = "only_i32")] +#[cfg(target_pointer_width = "32")] +const MAX_USIZE_INT: INT = INT::MAX; + /// Number of bits in [`INT`]. /// /// It is 64 unless the `only_i32` feature is enabled when it will be 32. diff --git a/src/optimizer.rs b/src/optimizer.rs index c45798a0..851628e8 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -301,21 +301,20 @@ fn optimize_stmt_block( while index < statements.len() { if preserve_result && index >= statements.len() - 1 { break; - } else { - match statements[index] { - ref stmt if is_pure(stmt) && index >= first_non_constant => { - state.set_dirty(); - statements.remove(index); - } - ref stmt if stmt.is_pure() => { - state.set_dirty(); - if index < first_non_constant { - first_non_constant -= 1; - } - statements.remove(index); - } - _ => index += 1, + } + match statements[index] { + ref stmt if is_pure(stmt) && index >= first_non_constant => { + state.set_dirty(); + statements.remove(index); } + ref stmt if stmt.is_pure() => { + state.set_dirty(); + if index < first_non_constant { + first_non_constant -= 1; + } + statements.remove(index); + } + _ => index += 1, } } diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 6018a982..3b3e9e5a 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -5,7 +5,7 @@ use crate::eval::{calc_index, calc_offset_len}; use crate::plugin::*; use crate::{ def_package, Array, Dynamic, ExclusiveRange, FnPtr, InclusiveRange, NativeCallContext, - Position, RhaiResultOf, StaticVec, ERR, INT, + Position, RhaiResultOf, StaticVec, ERR, INT, MAX_USIZE_INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -217,6 +217,8 @@ pub mod array_functions { len: INT, item: Dynamic, ) -> RhaiResultOf<()> { + let len = len.min(MAX_USIZE_INT); + if len <= 0 || (len as usize) <= array.len() { return Ok(()); } @@ -369,6 +371,8 @@ pub mod array_functions { /// ``` pub fn truncate(array: &mut Array, len: INT) { if !array.is_empty() { + let len = len.min(MAX_USIZE_INT); + if len > 0 { array.truncate(len as usize); } else { @@ -396,6 +400,8 @@ pub mod array_functions { /// ``` pub fn chop(array: &mut Array, len: INT) { if !array.is_empty() { + let len = len.min(MAX_USIZE_INT); + if len <= 0 { array.clear(); } else if (len as usize) < array.len() { diff --git a/src/packages/blob_basic.rs b/src/packages/blob_basic.rs index bdf96fd5..a3bbd532 100644 --- a/src/packages/blob_basic.rs +++ b/src/packages/blob_basic.rs @@ -4,7 +4,7 @@ use crate::eval::{calc_index, calc_offset_len}; use crate::plugin::*; use crate::{ def_package, Array, Blob, Dynamic, ExclusiveRange, InclusiveRange, NativeCallContext, Position, - RhaiResultOf, INT, INT_BYTES, + RhaiResultOf, INT, INT_BYTES, MAX_USIZE_INT, }; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -74,6 +74,7 @@ pub mod blob_functions { len: INT, value: INT, ) -> RhaiResultOf { + let len = len.min(MAX_USIZE_INT); let len = if len < 0 { 0 } else { len as usize }; let _ctx = ctx; @@ -342,20 +343,21 @@ pub mod blob_functions { if len <= 0 { return Ok(()); } + let len = len.min(MAX_USIZE_INT) as usize; let value = (value & 0x0000_00ff) as u8; let _ctx = ctx; // Check if blob will be over max size limit #[cfg(not(feature = "unchecked"))] - if _ctx.engine().max_array_size() > 0 && (len as usize) > _ctx.engine().max_array_size() { + if _ctx.engine().max_array_size() > 0 && len > _ctx.engine().max_array_size() { return Err( crate::ERR::ErrorDataTooLarge("Size of BLOB".to_string(), Position::NONE).into(), ); } - if len as usize > blob.len() { - blob.resize(len as usize, value); + if len > blob.len() { + blob.resize(len, value); } Ok(()) @@ -461,7 +463,9 @@ pub mod blob_functions { /// ``` pub fn truncate(blob: &mut Blob, len: INT) { if !blob.is_empty() { - if len >= 0 { + let len = len.min(MAX_USIZE_INT); + + if len > 0 { blob.truncate(len as usize); } else { blob.clear(); diff --git a/src/packages/iter_basic.rs b/src/packages/iter_basic.rs index 245f2cb3..6ec293cc 100644 --- a/src/packages/iter_basic.rs +++ b/src/packages/iter_basic.rs @@ -1,6 +1,8 @@ use crate::eval::calc_index; use crate::plugin::*; -use crate::{def_package, ExclusiveRange, InclusiveRange, RhaiResultOf, INT, INT_BITS}; +use crate::{ + def_package, ExclusiveRange, InclusiveRange, RhaiResultOf, INT, INT_BITS, MAX_USIZE_INT, +}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{ @@ -173,18 +175,13 @@ pub struct CharsStream(Vec, usize); impl CharsStream { pub fn new(string: &str, from: INT, len: INT) -> Self { - if len <= 0 { + if len <= 0 || from > MAX_USIZE_INT { return Self(Vec::new(), 0); } + let len = len.min(MAX_USIZE_INT) as usize; + if from >= 0 { - return Self( - string - .chars() - .skip(from as usize) - .take(len as usize) - .collect(), - 0, - ); + return Self(string.chars().skip(from as usize).take(len).collect(), 0); } let abs_from = from.unsigned_abs() as usize; @@ -194,7 +191,7 @@ impl CharsStream { } else { num_chars - abs_from }; - Self(string.chars().skip(offset).take(len as usize).collect(), 0) + Self(string.chars().skip(offset).take(len).collect(), 0) } } diff --git a/src/packages/lang_core.rs b/src/packages/lang_core.rs index 5c073146..25cfaee0 100644 --- a/src/packages/lang_core.rs +++ b/src/packages/lang_core.rs @@ -1,7 +1,7 @@ use crate::def_package; use crate::plugin::*; use crate::types::dynamic::Tag; -use crate::{Dynamic, RhaiResultOf, ERR, INT}; +use crate::{Dynamic, RhaiResultOf, ERR, INT, MAX_USIZE_INT}; #[cfg(feature = "no_std")] use std::prelude::v1::*; @@ -113,7 +113,7 @@ mod reflection_functions { } #[rhai_fn(name = "get_fn_metadata_list")] pub fn get_fn_metadata2(ctx: NativeCallContext, name: &str, params: INT) -> crate::Array { - if params < 0 { + if params < 0 || params > MAX_USIZE_INT { crate::Array::new() } else { collect_fn_metadata(ctx, |_, _, n, p, _| p == (params as usize) && n == name) diff --git a/src/packages/string_more.rs b/src/packages/string_more.rs index 8e018c22..da53a127 100644 --- a/src/packages/string_more.rs +++ b/src/packages/string_more.rs @@ -1,5 +1,8 @@ use crate::plugin::*; -use crate::{def_package, Dynamic, ExclusiveRange, InclusiveRange, RhaiResultOf, StaticVec, INT}; +use crate::{ + def_package, Dynamic, ExclusiveRange, InclusiveRange, RhaiResultOf, StaticVec, INT, + MAX_USIZE_INT, +}; #[cfg(feature = "no_std")] use std::prelude::v1::*; use std::{any::TypeId, mem}; @@ -263,10 +266,11 @@ mod string_functions { /// ``` pub fn truncate(string: &mut ImmutableString, len: INT) { if len > 0 { + let len = len.min(MAX_USIZE_INT) as usize; let chars: StaticVec<_> = string.chars().collect(); let copy = string.make_mut(); copy.clear(); - copy.extend(chars.into_iter().take(len as usize)); + copy.extend(chars.into_iter().take(len)); } else { clear(string); } @@ -344,8 +348,9 @@ mod string_functions { if string.is_empty() || len <= 0 { return ctx.engine().get_interned_string(""); } + let len = len.min(MAX_USIZE_INT) as usize; - let mut chars = StaticVec::::with_capacity(len as usize); + let mut chars = StaticVec::::with_capacity(len); for _ in 0..len { match string.make_mut().pop() { @@ -556,7 +561,13 @@ mod string_functions { } let start = if start < 0 { - let abs_start = start.unsigned_abs() as usize; + let abs_start = start.unsigned_abs(); + + if abs_start as u64 > MAX_USIZE_INT as u64 { + return -1 as INT; + } + + let abs_start = abs_start as usize; let chars: Vec<_> = string.chars().collect(); let num_chars = chars.len(); if abs_start > num_chars { @@ -570,7 +581,7 @@ mod string_functions { } } else if start == 0 { 0 - } else if start as usize >= string.chars().count() { + } else if start > MAX_USIZE_INT || start as usize >= string.chars().count() { return -1 as INT; } else { string @@ -632,7 +643,13 @@ mod string_functions { } let start = if start < 0 { - let abs_start = start.unsigned_abs() as usize; + let abs_start = start.unsigned_abs(); + + if abs_start as u64 > MAX_USIZE_INT as u64 { + return -1 as INT; + } + + let abs_start = abs_start as usize; let chars = string.chars().collect::>(); let num_chars = chars.len(); if abs_start > num_chars { @@ -646,7 +663,7 @@ mod string_functions { } } else if start == 0 { 0 - } else if start as usize >= string.chars().count() { + } else if start > MAX_USIZE_INT || start as usize >= string.chars().count() { return -1 as INT; } else { string @@ -704,16 +721,26 @@ mod string_functions { /// ``` pub fn get(string: &str, index: INT) -> Dynamic { if index >= 0 { + if index > MAX_USIZE_INT { + return Dynamic::UNIT; + } + string .chars() .nth(index as usize) .map_or_else(|| Dynamic::UNIT, Into::into) } else { // Count from end if negative + let abs_index = index.unsigned_abs(); + + if abs_index as u64 > MAX_USIZE_INT as u64 { + return Dynamic::UNIT; + } + string .chars() .rev() - .nth((index.unsigned_abs() as usize) - 1) + .nth((abs_index as usize) - 1) .map_or_else(|| Dynamic::UNIT, Into::into) } } @@ -742,14 +769,25 @@ mod string_functions { /// ``` pub fn set(string: &mut ImmutableString, index: INT, character: char) { if index >= 0 { + if index > MAX_USIZE_INT { + return; + } + let index = index as usize; + *string = string .chars() .enumerate() .map(|(i, ch)| if i == index { character } else { ch }) .collect(); } else { - let abs_index = index.unsigned_abs() as usize; + let abs_index = index.unsigned_abs(); + + if abs_index as u64 > MAX_USIZE_INT as u64 { + return; + } + + let abs_index = abs_index as usize; let string_len = string.chars().count(); if abs_index <= string_len { @@ -833,14 +871,20 @@ mod string_functions { let offset = if string.is_empty() || len <= 0 { return ctx.engine().get_interned_string(""); } else if start < 0 { - let abs_start = start.unsigned_abs() as usize; + let abs_start = start.unsigned_abs(); + + if abs_start as u64 > MAX_USIZE_INT as u64 { + return ctx.engine().get_interned_string(""); + } + + let abs_start = abs_start as usize; chars.extend(string.chars()); if abs_start > chars.len() { 0 } else { chars.len() - abs_start } - } else if start as usize >= string.chars().count() { + } else if start > MAX_USIZE_INT || start as usize >= string.chars().count() { return ctx.engine().get_interned_string(""); } else { start as usize @@ -962,14 +1006,20 @@ mod string_functions { string.make_mut().clear(); return; } else if start < 0 { - let abs_start = start.unsigned_abs() as usize; + let abs_start = start.unsigned_abs(); + + if abs_start as u64 > MAX_USIZE_INT as u64 { + return; + } + + let abs_start = abs_start as usize; chars.extend(string.chars()); if abs_start > chars.len() { 0 } else { chars.len() - abs_start } - } else if start as usize >= string.chars().count() { + } else if start > MAX_USIZE_INT || start as usize >= string.chars().count() { string.make_mut().clear(); return; } else { @@ -1131,11 +1181,12 @@ mod string_functions { if len <= 0 { return Ok(()); } + let len = len.min(MAX_USIZE_INT) as usize; let _ctx = ctx; // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] - if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { + if _ctx.engine().max_string_size() > 0 && len > _ctx.engine().max_string_size() { return Err(crate::ERR::ErrorDataTooLarge( "Length of string".to_string(), crate::Position::NONE, @@ -1145,10 +1196,10 @@ mod string_functions { let orig_len = string.chars().count(); - if len as usize > orig_len { + if len > orig_len { let p = string.make_mut(); - for _ in 0..(len as usize - orig_len) { + for _ in 0..(len - orig_len) { p.push(character); } @@ -1192,11 +1243,12 @@ mod string_functions { if len <= 0 { return Ok(()); } + let len = len.min(MAX_USIZE_INT) as usize; let _ctx = ctx; // Check if string will be over max size limit #[cfg(not(feature = "unchecked"))] - if _ctx.engine().max_string_size() > 0 && len as usize > _ctx.engine().max_string_size() { + if _ctx.engine().max_string_size() > 0 && len > _ctx.engine().max_string_size() { return Err(crate::ERR::ErrorDataTooLarge( "Length of string".to_string(), crate::Position::NONE, @@ -1207,16 +1259,16 @@ mod string_functions { let mut str_len = string.chars().count(); let padding_len = padding.chars().count(); - if len as usize > str_len { + if len > str_len { let p = string.make_mut(); - while str_len < len as usize { - if str_len + padding_len <= len as usize { + while str_len < len { + if str_len + padding_len <= len { p.push_str(padding); str_len += padding_len; } else { - p.extend(padding.chars().take(len as usize - str_len)); - str_len = len as usize; + p.extend(padding.chars().take(len - str_len)); + str_len = len; } } @@ -1263,7 +1315,16 @@ mod string_functions { #[rhai_fn(name = "split")] pub fn split_at(ctx: NativeCallContext, string: &mut ImmutableString, index: INT) -> Array { if index <= 0 { - let abs_index = index.unsigned_abs() as usize; + let abs_index = index.unsigned_abs(); + + if abs_index as u64 > MAX_USIZE_INT as u64 { + return vec![ + ctx.engine().get_interned_string("").into(), + string.as_str().into(), + ]; + } + + let abs_index = abs_index as usize; let num_chars = string.chars().count(); if abs_index > num_chars { vec![ @@ -1275,6 +1336,11 @@ mod string_functions { let prefix_len = prefix.len(); vec![prefix.into(), string[prefix_len..].into()] } + } else if index > MAX_USIZE_INT { + vec![ + string.as_str().into(), + ctx.engine().get_interned_string("").into(), + ] } else { let prefix: String = string.chars().take(index as usize).collect(); let prefix_len = prefix.len(); @@ -1341,7 +1407,8 @@ mod string_functions { /// ``` #[rhai_fn(name = "split")] pub fn splitn(string: &str, delimiter: &str, segments: INT) -> Array { - let pieces: usize = if segments < 1 { 1 } else { segments as usize }; + let segments = segments.min(MAX_USIZE_INT) as usize; + let pieces: usize = if segments < 1 { 1 } else { segments }; string.splitn(pieces, delimiter).map(Into::into).collect() } /// Split the string into segments based on a `delimiter` character, returning an array of the segments. @@ -1371,7 +1438,8 @@ mod string_functions { /// ``` #[rhai_fn(name = "split")] pub fn splitn_char(string: &str, delimiter: char, segments: INT) -> Array { - let pieces: usize = if segments < 1 { 1 } else { segments as usize }; + let segments = segments.min(MAX_USIZE_INT) as usize; + let pieces: usize = if segments < 1 { 1 } else { segments }; string.splitn(pieces, delimiter).map(Into::into).collect() } /// Split the string into segments based on a `delimiter` string, returning an array of the @@ -1402,7 +1470,8 @@ mod string_functions { /// ``` #[rhai_fn(name = "split_rev")] pub fn rsplitn(string: &str, delimiter: &str, segments: INT) -> Array { - let pieces: usize = if segments < 1 { 1 } else { segments as usize }; + let segments = segments.min(MAX_USIZE_INT) as usize; + let pieces: usize = if segments < 1 { 1 } else { segments }; string.rsplitn(pieces, delimiter).map(Into::into).collect() } /// Split the string into segments based on a `delimiter` character, returning an array of @@ -1433,7 +1502,8 @@ mod string_functions { /// ``` #[rhai_fn(name = "split_rev")] pub fn rsplitn_char(string: &str, delimiter: char, segments: INT) -> Array { - let pieces: usize = if segments < 1 { 1 } else { segments as usize }; + let segments = segments.min(MAX_USIZE_INT) as usize; + let pieces: usize = if segments < 1 { 1 } else { segments }; string.rsplitn(pieces, delimiter).map(Into::into).collect() } } diff --git a/src/parser.rs b/src/parser.rs index 8498b90d..216cd3ea 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -34,6 +34,10 @@ pub type ParseResult = Result; type FnLib = BTreeMap>; +const KEYWORD_SEMICOLON: &str = Token::SemiColon.literal_syntax(); + +const KEYWORD_CLOSE_BRACE: &str = Token::RightBrace.literal_syntax(); + /// Invalid variable name that acts as a search barrier in a [`Scope`]. const SCOPE_SEARCH_BARRIER_MARKER: &str = "$ BARRIER $"; @@ -41,8 +45,9 @@ const SCOPE_SEARCH_BARRIER_MARKER: &str = "$ BARRIER $"; const NEVER_ENDS: &str = "`Token`"; /// Unroll `switch` ranges no larger than this. -const SMALL_SWITCH_RANGE: usize = 16; +const SMALL_SWITCH_RANGE: INT = 16; +/// Number of string interners used: two additional for property getters/setters if not `no_object` const NUM_INTERNERS: usize = if cfg!(feature = "no_object") { 1 } else { 3 }; /// _(internals)_ A type that encapsulates the current state of the parser. @@ -899,13 +904,13 @@ impl Engine { let mut settings = settings; settings.pos = eat_token(input, Token::LeftBracket); - let mut arr = StaticVec::new_const(); + let mut array = StaticVec::new_const(); loop { const MISSING_RBRACKET: &str = "to end this array literal"; #[cfg(not(feature = "unchecked"))] - if self.max_array_size() > 0 && arr.len() >= self.max_array_size() { + if self.max_array_size() > 0 && array.len() >= self.max_array_size() { return Err(PERR::LiteralTooLarge( "Size of array literal".to_string(), self.max_array_size(), @@ -927,7 +932,7 @@ impl Engine { } _ => { let expr = self.parse_expr(input, state, lib, settings.level_up())?; - arr.push(expr); + array.push(expr); } } @@ -954,9 +959,9 @@ impl Engine { }; } - arr.shrink_to_fit(); + array.shrink_to_fit(); - Ok(Expr::Array(arr.into(), settings.pos)) + Ok(Expr::Array(array.into(), settings.pos)) } /// Parse a map literal. @@ -2010,7 +2015,7 @@ impl Engine { } } - let op_info = if let Some(op) = op { + let op_info = if let Some(ref op) = op { OpAssignment::new_op_assignment_from_token(op, op_pos) } else { OpAssignment::new_assignment(op_pos) @@ -2605,9 +2610,6 @@ impl Engine { inputs.shrink_to_fit(); tokens.shrink_to_fit(); - const KEYWORD_SEMICOLON: &str = Token::SemiColon.literal_syntax(); - const KEYWORD_CLOSE_BRACE: &str = Token::RightBrace.literal_syntax(); - let self_terminated = matches!( required_token.as_str(), // It is self-terminating if the last symbol is a block @@ -2912,7 +2914,7 @@ impl Engine { Ok(true) => (), Ok(false) => return Err(PERR::ForbiddenVariable(name.to_string()).into_err(pos)), Err(err) => match *err { - EvalAltResult::ErrorParsing(perr, pos) => return Err(perr.into_err(pos)), + EvalAltResult::ErrorParsing(e, pos) => return Err(e.into_err(pos)), _ => return Err(PERR::ForbiddenVariable(name.to_string()).into_err(pos)), }, } diff --git a/src/reify.rs b/src/reify.rs index bdbe2db8..c5e98bb3 100644 --- a/src/reify.rs +++ b/src/reify.rs @@ -11,6 +11,7 @@ #[macro_export] macro_rules! reify { ($old:ident, |$new:ident : $t:ty| $code:expr, || $fallback:expr) => {{ + #[allow(clippy::redundant_else)] if std::any::TypeId::of::<$t>() == std::any::Any::type_id(&$old) { // SAFETY: This is safe because we already checked to make sure the two types // are actually the same. diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 27333f42..1eb5bd93 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -2315,13 +2315,13 @@ impl InputStream for MultiInputsStream<'_> { if self.index >= self.streams.len() { // No more streams return None; - } else if let Some(ch) = self.streams[self.index].next() { + } + if let Some(ch) = self.streams[self.index].next() { // Next character in current stream return Some(ch); - } else { - // Jump to the next stream - self.index += 1; } + // Jump to the next stream + self.index += 1; } } fn peek_next(&mut self) -> Option { @@ -2333,13 +2333,13 @@ impl InputStream for MultiInputsStream<'_> { if self.index >= self.streams.len() { // No more streams return None; - } else if let Some(&ch) = self.streams[self.index].peek() { + } + if let Some(&ch) = self.streams[self.index].peek() { // Next character in current stream return Some(ch); - } else { - // Jump to the next stream - self.index += 1; } + // Jump to the next stream + self.index += 1; } } } diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index a318581b..29a88336 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -936,7 +936,7 @@ impl Dynamic { #[must_use] pub const fn from_float(value: crate::FLOAT) -> Self { Self(Union::Float( - crate::ast::FloatWrapper::new_const(value), + crate::ast::FloatWrapper::new(value), DEFAULT_TAG_VALUE, ReadWrite, )) From 6bc98bd2527b7b48bbb45408e54302f7d6ebe549 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 27 Aug 2022 16:29:39 +0800 Subject: [PATCH 2/8] Remove extra clippy lints. --- src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 60fc68da..f09e2b66 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,11 +58,11 @@ #![cfg_attr(feature = "no_std", no_std)] #![deny(missing_docs)] -#![warn(clippy::all)] -#![warn(clippy::pedantic)] -#![warn(clippy::nursery)] -#![warn(clippy::cargo)] -#![warn(clippy::undocumented_unsafe_blocks)] +// #![warn(clippy::all)] +// #![warn(clippy::pedantic)] +// #![warn(clippy::nursery)] +// #![warn(clippy::cargo)] +// #![warn(clippy::undocumented_unsafe_blocks)] #![allow(clippy::unit_arg)] #![allow(clippy::missing_errors_doc)] #![allow(clippy::used_underscore_binding)] From 1389541e7d5f8dc62ad85f4a49daf32a26928eb5 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 27 Aug 2022 17:28:59 +0800 Subject: [PATCH 3/8] Set minimum Rust version to 1.61.0. --- CHANGELOG.md | 2 ++ Cargo.toml | 2 +- README.md | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 115f189d..f0d85f74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Rhai Release Notes Version 1.10.0 ============== +The minimum Rust version is now `1.61.0` in order to use some `const` generics. + Bug fixes --------- diff --git a/Cargo.toml b/Cargo.toml index c4e9e2cd..2078eb79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = [".", "codegen"] [package] name = "rhai" version = "1.9.0" -rust-version = "1.60.0" +rust-version = "1.61.0" edition = "2018" resolver = "2" authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"] diff --git a/README.md b/README.md index 4dd9d785..1991590c 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Targets and builds * All CPU and O/S targets supported by Rust, including: * WebAssembly (WASM) * `no-std` -* Minimum Rust version 1.60.0 +* Minimum Rust version 1.61.0 Standard features From 80772df4f4ed541bbea944249a98b52d833de66c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 29 Aug 2022 14:27:05 +0800 Subject: [PATCH 4/8] Shut up clippy. --- src/eval/chaining.rs | 2 +- src/eval/debugger.rs | 2 +- src/eval/expr.rs | 9 ++++---- src/eval/stmt.rs | 8 +++---- src/eval/target.rs | 7 +++++- src/func/call.rs | 5 ++--- src/module/mod.rs | 7 +++--- src/optimizer.rs | 12 +++++----- src/packages/array_basic.rs | 17 +++++--------- src/serde/de.rs | 37 +++++++++++++++--------------- src/serde/metadata.rs | 10 ++++----- src/serde/ser.rs | 2 +- src/serde/str.rs | 2 +- src/tokenizer.rs | 18 +++++++-------- src/types/dynamic.rs | 42 +++++++++++++---------------------- src/types/error.rs | 10 ++------- src/types/immutable_string.rs | 16 ++++++------- src/types/interner.rs | 2 +- src/types/parse_error.rs | 8 +++---- tests/build_type.rs | 5 +++-- 20 files changed, 100 insertions(+), 121 deletions(-) diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index 351df42e..0e6869f9 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -989,7 +989,7 @@ impl Engine { } else { let abs_index = index.unsigned_abs(); - if abs_index as u64 >= usize::MAX as u64 { + if abs_index as u64 > usize::MAX as u64 { return Err( ERR::ErrorStringBounds(s.chars().count(), index, idx_pos).into() ); diff --git a/src/eval/debugger.rs b/src/eval/debugger.rs index bfdb2af5..e98771e7 100644 --- a/src/eval/debugger.rs +++ b/src/eval/debugger.rs @@ -397,7 +397,7 @@ impl Debugger { /// Get the custom state. #[inline(always)] #[must_use] - pub fn state(&self) -> &Dynamic { + pub const fn state(&self) -> &Dynamic { &self.state } /// Get a mutable reference to the custom state. diff --git a/src/eval/expr.rs b/src/eval/expr.rs index ab499a6a..9419e9e9 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -144,11 +144,10 @@ impl Engine { let (index, var_pos) = match expr { // Check if the variable is `this` Expr::Variable(v, None, pos) if v.0.is_none() && v.3 == KEYWORD_THIS => { - return if let Some(val) = this_ptr { - Ok(((*val).into(), *pos)) - } else { - Err(ERR::ErrorUnboundThis(*pos).into()) - } + return this_ptr.as_mut().map_or_else( + || Err(ERR::ErrorUnboundThis(*pos).into()), + |val| Ok(((*val).into(), *pos)), + ) } _ if global.always_search_scope => (0, expr.start_position()), Expr::Variable(.., Some(i), pos) => (i.get() as usize, *pos), diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index 1003e896..0076102d 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -484,12 +484,10 @@ impl Engine { self.eval_expr(scope, global, caches, lib, this_ptr, expr, level) } else if let Ok(None) = expr_result { // Default match clause - if let Some(index) = def_case { - let def_expr = &expressions[*index].expr; + def_case.as_ref().map_or(Ok(Dynamic::UNIT), |&index| { + let def_expr = &expressions[index].expr; self.eval_expr(scope, global, caches, lib, this_ptr, def_expr, level) - } else { - Ok(Dynamic::UNIT) - } + }) } else { expr_result.map(|_| Dynamic::UNIT) } diff --git a/src/eval/target.rs b/src/eval/target.rs index 0114fa5b..e20f5b07 100644 --- a/src/eval/target.rs +++ b/src/eval/target.rs @@ -15,7 +15,12 @@ use std::prelude::v1::*; #[allow(dead_code)] pub fn calc_offset_len(length: usize, start: crate::INT, len: crate::INT) -> (usize, usize) { let start = if start < 0 { - length - usize::min(start.unsigned_abs() as usize, length) + let abs_start = start.unsigned_abs(); + if abs_start as u64 > crate::MAX_USIZE_INT as u64 { + 0 + } else { + length - usize::min(abs_start as usize, length) + } } else if start > crate::MAX_USIZE_INT || start as usize >= length { return (length, 0); } else { diff --git a/src/func/call.rs b/src/func/call.rs index 071bb5c7..4b1696d8 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -1228,13 +1228,11 @@ impl Engine { if target_is_shared || target.is_temp_value() { arg_values.insert(0, target.take_or_clone().flatten()); - args.extend(arg_values.iter_mut()); } else { // Turn it into a method call only if the object is not shared and not a simple value is_ref_mut = true; let obj_ref = target.take_ref().expect("ref"); args.push(obj_ref); - args.extend(arg_values.iter_mut()); } } else { // func(..., ...) @@ -1246,8 +1244,9 @@ impl Engine { .map(|(value, ..)| arg_values.push(value.flatten())) })?; args.extend(curry.iter_mut()); - args.extend(arg_values.iter_mut()); } + + args.extend(arg_values.iter_mut()); } self.exec_fn_call( diff --git a/src/module/mod.rs b/src/module/mod.rs index 805c821a..672e2cf6 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -41,7 +41,7 @@ impl FnNamespace { /// Is this a module namespace? #[inline(always)] #[must_use] - pub fn is_module_namespace(self) -> bool { + pub const fn is_module_namespace(self) -> bool { match self { Self::Internal => true, Self::Global => false, @@ -50,7 +50,7 @@ impl FnNamespace { /// Is this a global namespace? #[inline(always)] #[must_use] - pub fn is_global_namespace(self) -> bool { + pub const fn is_global_namespace(self) -> bool { match self { Self::Internal => false, Self::Global => true, @@ -193,7 +193,6 @@ impl FuncInfo { sig.push_str(", "); } } - sig.push(')'); } else { let params: StaticVec<_> = self .metadata @@ -215,8 +214,8 @@ impl FuncInfo { }) .collect(); sig.push_str(¶ms.join(", ")); - sig.push(')'); } + sig.push(')'); if !self.func.is_script() && !return_type.is_empty() { sig.push_str(" -> "); diff --git a/src/optimizer.rs b/src/optimizer.rs index 851628e8..d76fc42c 100644 --- a/src/optimizer.rs +++ b/src/optimizer.rs @@ -988,7 +988,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { #[cfg(not(feature = "no_index"))] Expr::Index(x, ..) if !_chaining => match (&mut x.lhs, &mut x.rhs) { // array[int] - (Expr::Array(a, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < a.len() && a.iter().all(Expr::is_pure) => { + (Expr::Array(a, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && *i <= crate::MAX_USIZE_INT && (*i as usize) < a.len() && a.iter().all(Expr::is_pure) => { // Array literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); @@ -997,7 +997,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { *expr = result; } // array[-int] - (Expr::Array(a, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as usize <= a.len() && a.iter().all(Expr::is_pure) => { + (Expr::Array(a, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as u64 <= a.len() as u64 && a.iter().all(Expr::is_pure) => { // Array literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); @@ -1015,25 +1015,25 @@ fn optimize_expr(expr: &mut Expr, state: &mut OptimizerState, _chaining: bool) { .map_or_else(|| Expr::Unit(*pos), |(.., mut expr)| { expr.set_position(*pos); expr }); } // int[int] - (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < crate::INT_BITS => { + (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && *i <= crate::MAX_USIZE_INT && (*i as usize) < crate::INT_BITS => { // Bit-field literal indexing - get the bit state.set_dirty(); *expr = Expr::BoolConstant((*n & (1 << (*i as usize))) != 0, *pos); } // int[-int] - (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as usize <= crate::INT_BITS => { + (Expr::IntegerConstant(n, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as u64 <= crate::INT_BITS as u64 => { // Bit-field literal indexing - get the bit state.set_dirty(); *expr = Expr::BoolConstant((*n & (1 << (crate::INT_BITS - i.unsigned_abs() as usize))) != 0, *pos); } // string[int] - (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && (*i as usize) < s.chars().count() => { + (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, ..)) if *i >= 0 && *i <= crate::MAX_USIZE_INT && (*i as usize) < s.chars().count() => { // String literal indexing - get the character state.set_dirty(); *expr = Expr::CharConstant(s.chars().nth(*i as usize).unwrap(), *pos); } // string[-int] - (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as usize <= s.chars().count() => { + (Expr::StringConstant(s, pos), Expr::IntegerConstant(i, ..)) if *i < 0 && i.unsigned_abs() as u64 <= s.chars().count() as u64 => { // String literal indexing - get the character state.set_dirty(); *expr = Expr::CharConstant(s.chars().rev().nth(i.unsigned_abs() as usize - 1).unwrap(), *pos); diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 3b3e9e5a..0751a768 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -1312,8 +1312,7 @@ pub mod array_functions { /// /// print(x); // prints "[1, 2, 3, 4, 3, 2, 1]" /// ``` - #[rhai_fn(return_raw)] - pub fn dedup(ctx: NativeCallContext, array: &mut Array) -> RhaiResultOf<()> { + pub fn dedup(ctx: NativeCallContext, array: &mut Array) { let comparer = FnPtr::new_unchecked(OP_EQUALS, StaticVec::new_const()); dedup_by_comparer(ctx, array, comparer) } @@ -1340,14 +1339,10 @@ pub mod array_functions { /// /// print(x); // prints "[1, 2, 3, 4]" /// ``` - #[rhai_fn(name = "dedup", return_raw)] - pub fn dedup_by_comparer( - ctx: NativeCallContext, - array: &mut Array, - comparer: FnPtr, - ) -> RhaiResultOf<()> { + #[rhai_fn(name = "dedup")] + pub fn dedup_by_comparer(ctx: NativeCallContext, array: &mut Array, comparer: FnPtr) { if array.is_empty() { - return Ok(()); + return; } array.dedup_by(|x, y| { @@ -1357,8 +1352,6 @@ pub mod array_functions { .as_bool() .unwrap_or(false) }); - - Ok(()) } /// Remove duplicated _consecutive_ elements from the array that return `true` when applied a /// function named by `comparer`. @@ -1391,7 +1384,7 @@ pub mod array_functions { array: &mut Array, comparer: &str, ) -> RhaiResultOf<()> { - dedup_by_comparer(ctx, array, FnPtr::new(comparer)?) + Ok(dedup_by_comparer(ctx, array, FnPtr::new(comparer)?)) } /// Reduce an array by iterating through all elements while applying the `reducer` function. /// diff --git a/src/serde/de.rs b/src/serde/de.rs index 15c538cd..76c31c11 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -22,7 +22,7 @@ impl<'de> DynamicDeserializer<'de> { /// The reference is necessary because the deserialized type may hold references /// (especially `&str`) to the source [`Dynamic`][crate::Dynamic]. #[must_use] - pub fn from_dynamic(value: &'de Dynamic) -> Self { + pub const fn from_dynamic(value: &'de Dynamic) -> Self { Self { value } } /// Shortcut for a type conversion error. @@ -449,21 +449,22 @@ impl<'de> Deserializer<'de> for &mut DynamicDeserializer<'de> { visitor.visit_enum(s.as_str().into_deserializer()) } else { #[cfg(not(feature = "no_object"))] - if let Some(map) = self.value.downcast_ref::() { - let mut iter = map.iter(); - let first = iter.next(); - let second = iter.next(); - if let (Some((key, value)), None) = (first, second) { - visitor.visit_enum(EnumDeserializer { - tag: key, - content: DynamicDeserializer::from_dynamic(value), - }) - } else { - self.type_error() - } - } else { - self.type_error() - } + return self.value.downcast_ref::().map_or_else( + || self.type_error(), + |map| { + let mut iter = map.iter(); + let first = iter.next(); + let second = iter.next(); + if let (Some((key, value)), None) = (first, second) { + visitor.visit_enum(EnumDeserializer { + tag: key, + content: DynamicDeserializer::from_dynamic(value), + }) + } else { + self.type_error() + } + }, + ); #[cfg(feature = "no_object")] return self.type_error(); } @@ -488,7 +489,7 @@ struct IterateDynamicArray<'a, ITER: Iterator> { #[cfg(not(feature = "no_index"))] impl<'a, ITER: Iterator> IterateDynamicArray<'a, ITER> { #[must_use] - pub fn new(iter: ITER) -> Self { + pub const fn new(iter: ITER) -> Self { Self { iter } } } @@ -525,7 +526,7 @@ struct IterateMap<'a, K: Iterator, V: Iterator, V: Iterator> IterateMap<'a, K, V> { #[must_use] - pub fn new(keys: K, values: V) -> Self { + pub const fn new(keys: K, values: V) -> Self { Self { keys, values } } } diff --git a/src/serde/metadata.rs b/src/serde/metadata.rs index f4d9732f..7a562d5c 100644 --- a/src/serde/metadata.rs +++ b/src/serde/metadata.rs @@ -56,7 +56,7 @@ impl PartialOrd for FnMetadata<'_> { impl Ord for FnMetadata<'_> { fn cmp(&self, other: &Self) -> Ordering { - match self.name.cmp(&other.name) { + match self.name.cmp(other.name) { Ordering::Equal => self.num_params.cmp(&other.num_params), cmp => cmp, } @@ -79,8 +79,8 @@ impl<'a> From<&'a FuncInfo> for FnMetadata<'a> { base_hash, full_hash, #[cfg(not(feature = "no_module"))] - namespace: info.metadata.namespace.into(), - access: info.metadata.access.into(), + namespace: info.metadata.namespace, + access: info.metadata.access, name: &info.metadata.name, typ, num_params: info.metadata.params, @@ -150,7 +150,7 @@ impl<'a> From<&'a crate::Module> for ModuleMetadata<'a> { functions.sort(); Self { - doc: module.doc().into(), + doc: module.doc(), modules: module .iter_sub_modules() .map(|(name, m)| (name, m.as_ref().into())) @@ -206,7 +206,7 @@ pub fn gen_metadata_to_json( #[cfg(feature = "metadata")] if let Some(ast) = _ast { - global.doc = ast.doc().into(); + global.doc = ast.doc(); } serde_json::to_string_pretty(&global) diff --git a/src/serde/ser.rs b/src/serde/ser.rs index b7e2d915..1cdc8e28 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -20,7 +20,7 @@ struct DynamicSerializer { impl DynamicSerializer { /// Create a [`DynamicSerializer`] from a [`Dynamic`][crate::Dynamic] value. #[must_use] - pub fn new(_value: Dynamic) -> Self { + pub const fn new(_value: Dynamic) -> Self { Self { _key: Dynamic::UNIT, _value, diff --git a/src/serde/str.rs b/src/serde/str.rs index fd19de8c..df747a42 100644 --- a/src/serde/str.rs +++ b/src/serde/str.rs @@ -14,7 +14,7 @@ pub struct StringSliceDeserializer<'a> { impl<'a> StringSliceDeserializer<'a> { /// Create an `ImmutableStringDeserializer` from an `&str` reference. #[must_use] - pub fn from_str(value: &'a str) -> Self { + pub const fn from_str(value: &'a str) -> Self { Self { value } } /// Shortcut for a type conversion error. diff --git a/src/tokenizer.rs b/src/tokenizer.rs index 1eb5bd93..f69f2411 100644 --- a/src/tokenizer.rs +++ b/src/tokenizer.rs @@ -1701,11 +1701,11 @@ fn get_next_token_inner( // letter or underscore ... #[cfg(not(feature = "unicode-xid-ident"))] ('a'..='z' | '_' | 'A'..='Z', ..) => { - return get_identifier(stream, pos, start_pos, c); + return Some(get_identifier(stream, pos, start_pos, c)); } #[cfg(feature = "unicode-xid-ident")] (ch, ..) if unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' => { - return get_identifier(stream, pos, start_pos, c); + return Some(get_identifier(stream, pos, start_pos, c)); } // " - string literal @@ -2178,7 +2178,7 @@ fn get_identifier( pos: &mut Position, start_pos: Position, first_char: char, -) -> Option<(Token, Position)> { +) -> (Token, Position) { let mut result = smallvec::SmallVec::<[char; 8]>::new(); result.push(first_char); @@ -2197,17 +2197,17 @@ fn get_identifier( let identifier: String = result.into_iter().collect(); if let Some(token) = Token::lookup_from_syntax(&identifier) { - return Some((token, start_pos)); + return (token, start_pos); } if !is_valid_identifier { - return Some(( + return ( Token::LexError(LERR::MalformedIdentifier(identifier).into()), start_pos, - )); + ); } - Some((Token::Identifier(identifier.into()), start_pos)) + (Token::Identifier(identifier.into()), start_pos) } /// Is a keyword allowed as a function? @@ -2272,7 +2272,7 @@ pub fn is_id_continue(x: char) -> bool { #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] #[must_use] -pub fn is_id_first_alphabetic(x: char) -> bool { +pub const fn is_id_first_alphabetic(x: char) -> bool { x.is_ascii_alphabetic() } @@ -2280,7 +2280,7 @@ pub fn is_id_first_alphabetic(x: char) -> bool { #[cfg(not(feature = "unicode-xid-ident"))] #[inline(always)] #[must_use] -pub fn is_id_continue(x: char) -> bool { +pub const fn is_id_continue(x: char) -> bool { x.is_ascii_alphanumeric() || x == '_' } diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index 29a88336..5d4803d1 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -1166,7 +1166,7 @@ impl Dynamic { #[cfg(not(feature = "no_index"))] reify!(value, |v: crate::Blob| { // don't use blob.into() because it'll be converted into an Array - return Dynamic::from_blob(v); + return Self::from_blob(v); }); #[cfg(not(feature = "no_object"))] reify!(value, |v: crate::Map| return v.into()); @@ -1175,7 +1175,7 @@ impl Dynamic { #[cfg(not(feature = "no_std"))] reify!(value, |v: Instant| return v.into()); #[cfg(not(feature = "no_closure"))] - reify!(value, |v: crate::Shared>| return v + reify!(value, |v: crate::Shared>| return v .into()); Self(Union::Variant( @@ -1444,7 +1444,7 @@ impl Dynamic { let value = crate::func::locked_read(cell); return if (*value).type_id() != TypeId::of::() - && TypeId::of::() != TypeId::of::() + && TypeId::of::() != TypeId::of::() { None } else { @@ -1476,7 +1476,7 @@ impl Dynamic { let guard = crate::func::locked_write(cell); return if (*guard).type_id() != TypeId::of::() - && TypeId::of::() != TypeId::of::() + && TypeId::of::() != TypeId::of::() { None } else { @@ -1577,7 +1577,7 @@ impl Dynamic { _ => None, }; } - if TypeId::of::() == TypeId::of::() { + if TypeId::of::() == TypeId::of::() { return self.as_any().downcast_ref::(); } @@ -1675,7 +1675,7 @@ impl Dynamic { _ => None, }; } - if TypeId::of::() == TypeId::of::() { + if TypeId::of::() == TypeId::of::() { return self.as_any_mut().downcast_mut::(); } @@ -1962,7 +1962,7 @@ impl From> for Dynamic { #[inline] fn from(value: Vec) -> Self { Self(Union::Array( - Box::new(value.into_iter().map(Dynamic::from).collect()), + Box::new(value.into_iter().map(Self::from).collect()), DEFAULT_TAG_VALUE, ReadWrite, )) @@ -1973,7 +1973,7 @@ impl From<&[T]> for Dynamic { #[inline] fn from(value: &[T]) -> Self { Self(Union::Array( - Box::new(value.iter().cloned().map(Dynamic::from).collect()), + Box::new(value.iter().cloned().map(Self::from).collect()), DEFAULT_TAG_VALUE, ReadWrite, )) @@ -1984,7 +1984,7 @@ impl std::iter::FromIterator for Dynamic { #[inline] fn from_iter>(iter: X) -> Self { Self(Union::Array( - Box::new(iter.into_iter().map(Dynamic::from).collect()), + Box::new(iter.into_iter().map(Self::from).collect()), DEFAULT_TAG_VALUE, ReadWrite, )) @@ -2001,7 +2001,7 @@ impl, T: Variant + Clone> From> From> for Dynamic #[inline] fn from(value: std::collections::HashSet) -> Self { Self(Union::Map( - Box::new( - value - .into_iter() - .map(|k| (k.into(), Dynamic::UNIT)) - .collect(), - ), + Box::new(value.into_iter().map(|k| (k.into(), Self::UNIT)).collect()), DEFAULT_TAG_VALUE, ReadWrite, )) @@ -2036,7 +2031,7 @@ impl, T: Variant + Clone> From> From> for Dynamic #[inline] fn from(value: std::collections::BTreeSet) -> Self { Self(Union::Map( - Box::new( - value - .into_iter() - .map(|k| (k.into(), Dynamic::UNIT)) - .collect(), - ), + Box::new(value.into_iter().map(|k| (k.into(), Self::UNIT)).collect()), DEFAULT_TAG_VALUE, ReadWrite, )) @@ -2074,7 +2064,7 @@ impl From for Dynamic { } } #[cfg(not(feature = "no_closure"))] -impl From>> for Dynamic { +impl From>> for Dynamic { #[inline(always)] fn from(value: crate::Shared>) -> Self { Self(Union::Shared(value, DEFAULT_TAG_VALUE, ReadWrite)) @@ -2084,12 +2074,12 @@ impl From>> for Dynamic { impl From for Dynamic { #[inline(always)] fn from(value: ExclusiveRange) -> Self { - Dynamic::from(value) + Self::from(value) } } impl From for Dynamic { #[inline(always)] fn from(value: InclusiveRange) -> Self { - Dynamic::from(value) + Self::from(value) } } diff --git a/src/types/error.rs b/src/types/error.rs index 9c0133f4..6e9814cd 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -368,9 +368,6 @@ impl EvalAltResult { map.insert("function".into(), f.into()); map.insert("source".into(), s.into()); } - Self::ErrorInModule(m, ..) => { - map.insert("module".into(), m.into()); - } Self::ErrorMismatchDataType(r, a, ..) | Self::ErrorMismatchOutputType(r, a, ..) => { map.insert("requested".into(), r.into()); map.insert("actual".into(), a.into()); @@ -381,9 +378,6 @@ impl EvalAltResult { map.insert("length".into(), (*n as INT).into()); map.insert("index".into(), (*i as INT).into()); } - Self::ErrorIndexingType(t, ..) => { - map.insert("type".into(), t.into()); - } Self::ErrorVariableExists(v, ..) | Self::ErrorForbiddenVariable(v, ..) | Self::ErrorVariableNotFound(v, ..) @@ -395,14 +389,14 @@ impl EvalAltResult { Self::ErrorIndexNotFound(v, ..) => { map.insert("index".into(), v.clone()); } - Self::ErrorModuleNotFound(m, ..) => { + Self::ErrorInModule(m, ..) | Self::ErrorModuleNotFound(m, ..) => { map.insert("module".into(), m.into()); } Self::ErrorDotExpr(p, ..) => { map.insert("property".into(), p.into()); } - Self::ErrorDataTooLarge(t, ..) => { + Self::ErrorIndexingType(t, ..) | Self::ErrorDataTooLarge(t, ..) => { map.insert("type".into(), t.into()); } Self::ErrorTerminated(t, ..) => { diff --git a/src/types/immutable_string.rs b/src/types/immutable_string.rs index 5b4fefa5..6487efeb 100644 --- a/src/types/immutable_string.rs +++ b/src/types/immutable_string.rs @@ -264,9 +264,9 @@ impl Add for &ImmutableString { } } -impl AddAssign<&ImmutableString> for ImmutableString { +impl AddAssign<&Self> for ImmutableString { #[inline] - fn add_assign(&mut self, rhs: &ImmutableString) { + fn add_assign(&mut self, rhs: &Self) { if !rhs.is_empty() { if self.is_empty() { self.0 = rhs.0.clone(); @@ -277,9 +277,9 @@ impl AddAssign<&ImmutableString> for ImmutableString { } } -impl AddAssign for ImmutableString { +impl AddAssign for ImmutableString { #[inline] - fn add_assign(&mut self, rhs: ImmutableString) { + fn add_assign(&mut self, rhs: Self) { if !rhs.is_empty() { if self.is_empty() { self.0 = rhs.0; @@ -431,9 +431,9 @@ impl Sub for &ImmutableString { } } -impl SubAssign<&ImmutableString> for ImmutableString { +impl SubAssign<&Self> for ImmutableString { #[inline] - fn sub_assign(&mut self, rhs: &ImmutableString) { + fn sub_assign(&mut self, rhs: &Self) { if !rhs.is_empty() { if self.is_empty() { self.0 = rhs.0.clone(); @@ -445,9 +445,9 @@ impl SubAssign<&ImmutableString> for ImmutableString { } } -impl SubAssign for ImmutableString { +impl SubAssign for ImmutableString { #[inline] - fn sub_assign(&mut self, rhs: ImmutableString) { + fn sub_assign(&mut self, rhs: Self) { if !rhs.is_empty() { if self.is_empty() { self.0 = rhs.0; diff --git a/src/types/interner.rs b/src/types/interner.rs index 90c28461..40df1982 100644 --- a/src/types/interner.rs +++ b/src/types/interner.rs @@ -64,7 +64,7 @@ impl StringsInterner<'_> { #[inline(always)] #[must_use] pub fn get + Into>(&mut self, text: S) -> ImmutableString { - self.get_with_mapper(|s| s.into(), text) + self.get_with_mapper(Into::into, text) } /// Get an identifier from a text string, adding it to the interner if necessary. diff --git a/src/types/parse_error.rs b/src/types/parse_error.rs index a80255f0..47e3589a 100644 --- a/src/types/parse_error.rs +++ b/src/types/parse_error.rs @@ -302,13 +302,13 @@ impl ParseError { /// Get the [type][ParseErrorType] of this parse error. #[inline(always)] #[must_use] - pub fn err_type(&self) -> &ParseErrorType { + pub const fn err_type(&self) -> &ParseErrorType { &self.0 } /// Get the [position][Position] of this parse error. #[inline(always)] #[must_use] - pub fn position(&self) -> Position { + pub const fn position(&self) -> Position { self.1 } } @@ -323,7 +323,7 @@ impl From for RhaiError { impl From for ERR { #[inline(always)] fn from(err: ParseErrorType) -> Self { - ERR::ErrorParsing(err, Position::NONE) + Self::ErrorParsing(err, Position::NONE) } } @@ -337,6 +337,6 @@ impl From for RhaiError { impl From for ERR { #[inline(always)] fn from(err: ParseError) -> Self { - ERR::ErrorParsing(*err.0, err.1) + Self::ErrorParsing(*err.0, err.1) } } diff --git a/tests/build_type.rs b/tests/build_type.rs index 9fbcc1af..05b23bc2 100644 --- a/tests/build_type.rs +++ b/tests/build_type.rs @@ -3,7 +3,7 @@ use rhai::{CustomType, Engine, EvalAltResult, Position, TypeBuilder, INT}; #[test] fn build_type() -> Result<(), Box> { - #[derive(Debug, Clone, PartialEq)] + #[derive(Debug, Clone, PartialEq, Eq)] struct Vec3 { x: INT, y: INT, @@ -60,7 +60,8 @@ fn build_type() -> Result<(), Box> { .with_name("Vec3") .is_iterable() .with_fn("vec3", Self::new) - .is_iterable() + .with_fn("==", |x: &mut Vec3, y: Vec3| *x == y) + .with_fn("!=", |x: &mut Vec3, y: Vec3| *x != y) .with_get_set("x", Self::get_x, Self::set_x) .with_get_set("y", Self::get_y, Self::set_y) .with_get_set("z", Self::get_z, Self::set_z); From 3482625d81e386b428866b1d5bb167a5a6fe7c3a Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 29 Aug 2022 14:44:56 +0800 Subject: [PATCH 5/8] Bump MSRV. --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6235f1ed..64a543a1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,7 +8,7 @@ on: pull_request: {} env: - RUST_MSRV: 1.60.0 + RUST_MSRV: 1.61.0 jobs: msrv: From 0870318e4e787772cd9ebe5fb2fc78096dc3de45 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 29 Aug 2022 22:26:07 +0800 Subject: [PATCH 6/8] Fix bug in strict variables mode. --- CHANGELOG.md | 8 ++++++++ src/parser.rs | 32 ++++++++++++++------------------ tests/options.rs | 11 +++++++---- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01592c85..9e3954ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Rhai Release Notes ================== +Version 1.9.1 +============= + +This is a bug-fix version that fixes a bug. + +Accessing properties in _Strict Variables Mode_ no longer generates a _variable not found_ error. + + Version 1.9.0 ============= diff --git a/src/parser.rs b/src/parser.rs index 754513c8..ca3d7850 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1295,6 +1295,7 @@ impl Engine { input: &mut TokenStream, state: &mut ParseState, lib: &mut FnLib, + is_property: bool, settings: ParseSettings, ) -> ParseResult { #[cfg(not(feature = "unchecked"))] @@ -1435,11 +1436,11 @@ impl Engine { |crate::ast::Ident { name, pos }| { let (index, is_func) = state.access_var(name, lib, *pos); - if settings.options.contains(LangOptions::STRICT_VAR) - && !settings.in_closure + if !is_func && index.is_none() + && !settings.in_closure + && settings.options.contains(LangOptions::STRICT_VAR) && !state.scope.contains(name) - && !is_func { // If the parent scope is not inside another capturing closure // then we can conclude that the captured variable doesn't exist. @@ -1572,20 +1573,18 @@ impl Engine { // Once the identifier consumed we must enable next variables capturing state.allow_capture = true; } - Expr::Variable( - (None, ns, 0, state.get_interned_string(s)).into(), - None, - settings.pos, - ) + let name = state.get_interned_string(s); + Expr::Variable((None, ns, 0, name).into(), None, settings.pos) } // Normal variable access _ => { let (index, is_func) = state.access_var(&s, lib, settings.pos); - if settings.options.contains(LangOptions::STRICT_VAR) - && index.is_none() - && !state.scope.contains(&s) + if !is_property && !is_func + && index.is_none() + && settings.options.contains(LangOptions::STRICT_VAR) + && !state.scope.contains(&s) { return Err( PERR::VariableUndefined(s.to_string()).into_err(settings.pos) @@ -1599,11 +1598,8 @@ impl Engine { None } }); - Expr::Variable( - (index, ns, 0, state.get_interned_string(s)).into(), - short_index, - settings.pos, - ) + let name = state.get_interned_string(s); + Expr::Variable((index, ns, 0, name).into(), short_index, settings.pos) } } } @@ -1790,7 +1786,7 @@ impl Engine { (.., pos) => return Err(PERR::PropertyExpected.into_err(*pos)), } - let rhs = self.parse_primary(input, state, lib, settings.level_up())?; + let rhs = self.parse_primary(input, state, lib, true, settings.level_up())?; let op_flags = match op { Token::Period => ASTFlags::NONE, Token::Elvis => ASTFlags::NEGATED, @@ -1960,7 +1956,7 @@ impl Engine { // Token::EOF => Err(PERR::UnexpectedEOF.into_err(settings.pos)), // All other tokens - _ => self.parse_primary(input, state, lib, settings.level_up()), + _ => self.parse_primary(input, state, lib, false, settings.level_up()), } } diff --git a/tests/options.rs b/tests/options.rs index 27ff3ca2..8bc27a1d 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -56,10 +56,6 @@ fn test_options_allow() -> Result<(), Box> { fn test_options_strict_var() -> Result<(), Box> { let mut engine = Engine::new(); - let mut scope = Scope::new(); - scope.push("x", 42 as INT); - scope.push_constant("y", 0 as INT); - engine.compile("let x = if y { z } else { w };")?; #[cfg(not(feature = "no_function"))] @@ -78,9 +74,16 @@ fn test_options_strict_var() -> Result<(), Box> { #[cfg(not(feature = "no_function"))] engine.compile("let f = |y| x * y;")?; + let mut scope = Scope::new(); + scope.push("x", 42 as INT); + scope.push_constant("y", 0 as INT); + engine.set_strict_variables(true); assert!(engine.compile("let x = if y { z } else { w };").is_err()); + + engine.compile_with_scope(&mut scope, "if x.abs() { y } else { x + y.len };")?; + engine.compile("let y = 42; let x = y;")?; assert_eq!( From 498d6f54526a35d575c3714fa6a9683c71a91ca8 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Mon, 29 Aug 2022 22:27:07 +0800 Subject: [PATCH 7/8] Bump version to 1.9.1. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7ccfe77d..5311004c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = [".", "codegen"] [package] name = "rhai" -version = "1.9.0" +version = "1.9.1" rust-version = "1.60.0" edition = "2018" resolver = "2" From cfdca74bebb93920b1479f4a9ed2f8011b536dfc Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 30 Aug 2022 12:31:47 +0800 Subject: [PATCH 8/8] Fix test. --- tests/options.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/options.rs b/tests/options.rs index 8bc27a1d..4f04f1ec 100644 --- a/tests/options.rs +++ b/tests/options.rs @@ -82,6 +82,7 @@ fn test_options_strict_var() -> Result<(), Box> { assert!(engine.compile("let x = if y { z } else { w };").is_err()); + #[cfg(not(feature = "no_object"))] engine.compile_with_scope(&mut scope, "if x.abs() { y } else { x + y.len };")?; engine.compile("let y = 42; let x = y;")?;