Fix unchecked.

Do not duplicate data size checking.

codegen/src/rhai_module.rs

@@ -130,7 +130,7 @@ pub fn generate_body(
             let mut namespace = FnNamespaceAccess::Internal;
 
             match function.params().special {
-                FnSpecialAccess::None => {}
+                FnSpecialAccess::None => (),
                 FnSpecialAccess::Index(_) | FnSpecialAccess::Property(_) => {
                     let reg_name = fn_literal.value();
                     if reg_name.starts_with(FN_GET)

src/ast.rs

@@ -952,7 +952,7 @@ pub enum Stmt {
     /// \[`export`\] `const` id `=` expr
     Const(Expr, Box<Ident>, bool, Position),
     /// expr op`=` expr
-    Assignment(Box<(Expr, Option<OpAssignment>, Expr)>, Position),
+    Assignment(Box<(Expr, Option<OpAssignment<'static>>, Expr)>, Position),
     /// func `(` expr `,` ... `)`
     ///
     /// Note - this is a duplicate of [`Expr::FnCall`] to cover the very common pattern of a single
@@ -1384,14 +1384,14 @@ pub struct BinaryExpr {
 /// # Volatile Data Structure
 ///
 /// This type is volatile and may change.
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
-pub struct OpAssignment {
+pub struct OpAssignment<'a> {
     pub hash_op_assign: u64,
     pub hash_op: u64,
-    pub op: &'static str,
+    pub op: &'a str,
 }
 
-impl OpAssignment {
+impl OpAssignment<'_> {
     /// Create a new [`OpAssignment`].
     ///
     /// # Panics

src/engine.rs

@@ -437,34 +437,11 @@ impl<'a> Target<'a> {
     #[inline(always)]
     pub fn propagate_changed_value(&mut self) -> Result<(), Box<EvalAltResult>> {
         match self {
-            Self::RefMut(_) | Self::TempValue(_) => Ok(()),
+            Self::RefMut(_) | Self::TempValue(_) => (),
             #[cfg(not(feature = "no_closure"))]
-            Self::LockGuard(_) => Ok(()),
+            Self::LockGuard(_) => (),
             #[cfg(not(feature = "no_index"))]
-            Self::BitField(_, _, value) => {
+            Self::BitField(value, index, new_val) => {
-                let new_val = value.clone();
-                self.set_value(new_val)
-            }
-            #[cfg(not(feature = "no_index"))]
-            Self::StringChar(_, _, ch) => {
-                let new_val = ch.clone();
-                self.set_value(new_val)
-            }
-        }
-    }
-    /// Update the value of the `Target`.
-    fn set_value(&mut self, new_val: Dynamic) -> Result<(), Box<EvalAltResult>> {
-        match self {
-            Self::RefMut(r) => **r = new_val,
-            #[cfg(not(feature = "no_closure"))]
-            Self::LockGuard((r, _)) => **r = new_val,
-            Self::TempValue(_) => panic!("cannot update a value"),
-            #[cfg(not(feature = "no_index"))]
-            Self::BitField(value, index, _) => {
-                let value = &mut *value
-                    .write_lock::<crate::INT>()
-                    .expect("never fails because `BitField` always holds an `INT`");
-
                 // Replace the bit at the specified index position
                 let new_bit = new_val.as_bool().map_err(|err| {
                     Box::new(EvalAltResult::ErrorMismatchDataType(
@@ -474,6 +451,10 @@ impl<'a> Target<'a> {
                     ))
                 })?;
 
+                let value = &mut *value
+                    .write_lock::<crate::INT>()
+                    .expect("never fails because `BitField` always holds an `INT`");
+
                 let index = *index;
 
                 if index < std::mem::size_of_val(value) * 8 {
@@ -483,14 +464,12 @@ impl<'a> Target<'a> {
                     } else {
                         *value &= !mask;
                     }
+                } else {
+                    unreachable!("bit-field index out of bounds: {}", index);
                 }
             }
             #[cfg(not(feature = "no_index"))]
-            Self::StringChar(s, index, _) => {
+            Self::StringChar(s, index, new_val) => {
-                let s = &mut *s
-                    .write_lock::<ImmutableString>()
-                    .expect("never fails because `StringChar` always holds an `ImmutableString`");
-
                 // Replace the character at the specified index position
                 let new_ch = new_val.as_char().map_err(|err| {
                     Box::new(EvalAltResult::ErrorMismatchDataType(
@@ -500,6 +479,10 @@ impl<'a> Target<'a> {
                     ))
                 })?;
 
+                let s = &mut *s
+                    .write_lock::<ImmutableString>()
+                    .expect("never fails because `StringChar` always holds an `ImmutableString`");
+
                 let index = *index;
 
                 *s = s
@@ -1236,8 +1219,6 @@ impl Engine {
                         let ((new_val, new_pos), (op_info, op_pos)) =
                             new_val.expect("never fails because `new_val` is `Some`");
                         let idx_val = idx_val.as_index_value();
-
-                        #[cfg(not(feature = "no_index"))]
                         let mut idx_val_for_setter = idx_val.clone();
 
                         let try_setter = match self.get_indexed_mut(
@@ -1310,13 +1291,13 @@ impl Engine {
                     // {xxx:map}.id op= ???
                     Expr::Property(x) if target.is::<Map>() && new_val.is_some() => {
                         let (name, pos) = &x.2;
+                        let ((new_val, new_pos), (op_info, op_pos)) =
+                            new_val.expect("never fails because `new_val` is `Some`");
                         let index = name.into();
                         {
                             let mut val = self.get_indexed_mut(
                                 mods, state, lib, target, index, *pos, true, false, level,
                             )?;
-                            let ((new_val, new_pos), (op_info, op_pos)) =
-                                new_val.expect("never fails because `new_val` is `Some`");
                             self.eval_op_assignment(
                                 mods, state, lib, op_info, op_pos, &mut val, root, new_val,
                             )
@@ -2355,7 +2336,7 @@ impl Engine {
                     mods,
                     state,
                     lib,
-                    op_info.clone(),
+                    *op_info,
                     *op_pos,
                     &mut lhs_ptr,
                     (var_name, pos),
@@ -2363,8 +2344,10 @@ impl Engine {
                 )
                 .map_err(|err| err.fill_position(rhs_expr.position()))?;
 
-                self.check_data_size(lhs_ptr.as_ref())
+                if op_info.is_some() {
-                    .map_err(|err| err.fill_position(lhs_expr.position()))?;
+                    self.check_data_size(lhs_ptr.as_ref())
+                        .map_err(|err| err.fill_position(lhs_expr.position()))?;
+                }
 
                 Ok(Dynamic::UNIT)
             }
@@ -2960,55 +2943,52 @@ impl Engine {
         result.and_then(|r| self.check_data_size(&r).map(|_| r))
     }
 
+    #[cfg(feature = "unchecked")]
+    #[inline(always)]
+    fn check_data_size(&self, _value: &Dynamic) -> Result<(), Box<EvalAltResult>> {
+        Ok(())
+    }
+
+    #[cfg(not(feature = "unchecked"))]
     fn check_data_size(&self, value: &Dynamic) -> Result<(), Box<EvalAltResult>> {
         // Recursively calculate the size of a value (especially `Array` and `Map`)
         fn calc_size(value: &Dynamic) -> (usize, usize, usize) {
-            match value {
+            match value.0 {
                 #[cfg(not(feature = "no_index"))]
-                Dynamic(Union::Array(arr, _, _)) => {
+                Union::Array(ref arr, _, _) => {
-                    let mut arrays = 0;
+                    arr.iter()
-                    let mut maps = 0;
+                        .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 {
-
+                            Union::Array(_, _, _) => {
-                    arr.iter().for_each(|value| match value {
+                                let (a, m, s) = calc_size(value);
-                        Dynamic(Union::Array(_, _, _)) => {
+                                (arrays + a + 1, maps + m, strings + s)
-                            let (a, m, _) = calc_size(value);
+                            }
-                            arrays += a;
+                            #[cfg(not(feature = "no_object"))]
-                            maps += m;
+                            Union::Map(_, _, _) => {
-                        }
+                                let (a, m, s) = calc_size(value);
-                        #[cfg(not(feature = "no_object"))]
+                                (arrays + a + 1, maps + m, strings + s)
-                        Dynamic(Union::Map(_, _, _)) => {
+                            }
-                            let (a, m, _) = calc_size(value);
+                            Union::Str(ref s, _, _) => (arrays + 1, maps, strings + s.len()),
-                            arrays += a;
+                            _ => (arrays + 1, maps, strings),
-                            maps += m;
+                        })
-                        }
-                        _ => arrays += 1,
-                    });
-
-                    (arrays, maps, 0)
                 }
                 #[cfg(not(feature = "no_object"))]
-                Dynamic(Union::Map(map, _, _)) => {
+                Union::Map(ref map, _, _) => {
-                    let mut arrays = 0;
+                    map.values()
-                    let mut maps = 0;
+                        .fold((0, 0, 0), |(arrays, maps, strings), value| match value.0 {
-
+                            #[cfg(not(feature = "no_index"))]
-                    map.values().for_each(|value| match value {
+                            Union::Array(_, _, _) => {
-                        #[cfg(not(feature = "no_index"))]
+                                let (a, m, s) = calc_size(value);
-                        Dynamic(Union::Array(_, _, _)) => {
+                                (arrays + a, maps + m + 1, strings + s)
-                            let (a, m, _) = calc_size(value);
+                            }
-                            arrays += a;
+                            Union::Map(_, _, _) => {
-                            maps += m;
+                                let (a, m, s) = calc_size(value);
-                        }
+                                (arrays + a, maps + m + 1, strings + s)
-                        Dynamic(Union::Map(_, _, _)) => {
+                            }
-                            let (a, m, _) = calc_size(value);
+                            Union::Str(ref s, _, _) => (arrays, maps + 1, strings + s.len()),
-                            arrays += a;
+                            _ => (arrays, maps + 1, strings),
-                            maps += m;
+                        })
-                        }
-                        _ => maps += 1,
-                    });
-
-                    (arrays, maps, 0)
                 }
-                Dynamic(Union::Str(s, _, _)) => (0, 0, s.len()),
+                Union::Str(ref s, _, _) => (0, 0, s.len()),
                 _ => (0, 0, 0),
             }
         }

src/optimize.rs

@@ -297,7 +297,7 @@ fn optimize_stmt_block(
                             Stmt::Noop(*pos)
                         };
                     }
-                    [.., second_last_stmt, Stmt::Noop(_)] if second_last_stmt.returns_value() => {}
+                    [.., second_last_stmt, Stmt::Noop(_)] if second_last_stmt.returns_value() => (),
                     [.., second_last_stmt, last_stmt]
                         if !last_stmt.returns_value() && is_pure(last_stmt) =>
                     {

src/token.rs

@@ -1095,7 +1095,7 @@ pub fn parse_string_literal(
 
         match next_char {
             // \r - ignore if followed by \n
-            '\r' if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) => {}
+            '\r' if stream.peek_next().map(|ch| ch == '\n').unwrap_or(false) => (),
             // \...
             '\\' if !verbatim && escape.is_empty() => {
                 escape.push('\\');

tests/data_size.rs

@@ -206,6 +206,18 @@ fn test_max_map_size() -> Result<(), Box<EvalAltResult>> {
         )
     );
 
+    assert!(matches!(
+        *engine
+            .consume(
+                "
+                    let x = #{};
+                    loop { x.a = x; }
+                "
+            )
+            .expect_err("should error"),
+        EvalAltResult::ErrorDataTooLarge(_, _)
+    ));
+
     assert!(matches!(
         *engine
             .eval::<Map>(