Extract index calculataion into functions.

CHANGELOG.md

@@ -1,6 +1,15 @@
 Rhai Release Notes
 ==================
 
+Version 1.5.0
+=============
+
+Bug fixes
+---------
+
+* `set_bit` for bit-flags with negative index now works correctly.
+
+
 Version 1.4.0
 =============
 

src/eval/chaining.rs

@@ -1,7 +1,7 @@
 //! Types to support chaining operations (i.e. indexing and dotting).
 #![cfg(any(not(feature = "no_index"), not(feature = "no_object")))]
 
-use super::{EvalState, GlobalRuntimeState, Target};
+use super::{calc_index, EvalState, GlobalRuntimeState, Target};
 use crate::ast::{Expr, OpAssignment};
 use crate::types::dynamic::Union;
 use crate::{Dynamic, Engine, Module, Position, RhaiResult, RhaiResultOf, Scope, StaticVec, ERR};
@@ -771,7 +771,7 @@ impl Engine {
         lib: &[&Module],
         target: &'t mut Dynamic,
         idx: Dynamic,
-        idx_pos: Position,
+        pos: Position,
         add_if_not_found: bool,
         use_indexers: bool,
         level: usize,
@@ -788,38 +788,12 @@ impl Engine {
                 // val_array[idx]
                 let index = idx
                     .as_int()
-                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, idx_pos))?;
+                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, pos))?;
+                let len = arr.len();
+                let arr_idx =
+                    calc_index(len, index, true, || ERR::ErrorArrayBounds(len, index, pos))?;
 
-                let arr_len = arr.len();
+                Ok(arr.get_mut(arr_idx).map(Target::from).unwrap())
-
-                #[cfg(not(feature = "unchecked"))]
-                let arr_idx = if index < 0 {
-                    // Count from end if negative
-                    arr_len
-                        - index
-                            .checked_abs()
-                            .ok_or_else(|| ERR::ErrorArrayBounds(arr_len, index, idx_pos))
-                            .and_then(|n| {
-                                if n as usize > arr_len {
-                                    Err(ERR::ErrorArrayBounds(arr_len, index, idx_pos).into())
-                                } else {
-                                    Ok(n as usize)
-                                }
-                            })?
-                } else {
-                    index as usize
-                };
-                #[cfg(feature = "unchecked")]
-                let arr_idx = if index < 0 {
-                    // Count from end if negative
-                    arr_len - index.abs() as usize
-                } else {
-                    index as usize
-                };
-
-                arr.get_mut(arr_idx)
-                    .map(Target::from)
-                    .ok_or_else(|| ERR::ErrorArrayBounds(arr_len, index, idx_pos).into())
             }
 
             #[cfg(not(feature = "no_index"))]
@@ -827,39 +801,13 @@ impl Engine {
                 // val_blob[idx]
                 let index = idx
                     .as_int()
-                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, idx_pos))?;
+                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, pos))?;
+                let len = arr.len();
+                let arr_idx =
+                    calc_index(len, index, true, || ERR::ErrorArrayBounds(len, index, pos))?;
 
-                let arr_len = arr.len();
+                let value = arr.get(arr_idx).map(|&v| (v as crate::INT).into()).unwrap();
 
-                #[cfg(not(feature = "unchecked"))]
-                let arr_idx = if index < 0 {
-                    // Count from end if negative
-                    arr_len
-                        - index
-                            .checked_abs()
-                            .ok_or_else(|| ERR::ErrorArrayBounds(arr_len, index, idx_pos))
-                            .and_then(|n| {
-                                if n as usize > arr_len {
-                                    Err(ERR::ErrorArrayBounds(arr_len, index, idx_pos).into())
-                                } else {
-                                    Ok(n as usize)
-                                }
-                            })?
-                } else {
-                    index as usize
-                };
-                #[cfg(feature = "unchecked")]
-                let arr_idx = if index < 0 {
-                    // Count from end if negative
-                    arr_len - index.abs() as usize
-                } else {
-                    index as usize
-                };
-
-                let value = arr
-                    .get(arr_idx)
-                    .map(|&v| (v as crate::INT).into())
-                    .ok_or_else(|| Box::new(ERR::ErrorArrayBounds(arr_len, index, idx_pos)))?;
                 Ok(Target::BlobByte {
                     source: target,
                     value,
@@ -871,7 +819,7 @@ impl Engine {
             Dynamic(Union::Map(map, _, _)) => {
                 // val_map[idx]
                 let index = idx.read_lock::<crate::ImmutableString>().ok_or_else(|| {
-                    self.make_type_mismatch_err::<crate::ImmutableString>(idx.type_name(), idx_pos)
+                    self.make_type_mismatch_err::<crate::ImmutableString>(idx.type_name(), pos)
                 })?;
 
                 if _add_if_not_found && !map.contains_key(index.as_str()) {
@@ -888,11 +836,6 @@ impl Engine {
             Dynamic(Union::Int(value, _, _))
                 if idx.is::<crate::ExclusiveRange>() || idx.is::<crate::InclusiveRange>() =>
             {
-                #[cfg(not(feature = "only_i32"))]
-                type BASE = u64;
-                #[cfg(feature = "only_i32")]
-                type BASE = u32;
-
                 // val_int[range]
                 const BITS: usize = std::mem::size_of::<crate::INT>() * 8;
 
@@ -900,40 +843,47 @@ impl Engine {
                     let start = range.start;
                     let end = range.end;
 
-                    if start < 0 || start as usize >= BITS {
+                    let start = calc_index(BITS, start, false, || {
-                        return Err(ERR::ErrorBitFieldBounds(BITS, start, idx_pos).into());
+                        ERR::ErrorBitFieldBounds(BITS, start, pos)
-                    } else if end < 0 || end as usize >= BITS {
+                    })?;
-                        return Err(ERR::ErrorBitFieldBounds(BITS, end, idx_pos).into());
+                    let end = calc_index(BITS, end, false, || {
-                    } else if end <= start {
+                        ERR::ErrorBitFieldBounds(BITS, end, pos)
+                    })?;
+
+                    if end <= start {
                         (0, 0)
-                    } else if end as usize == BITS && start == 0 {
+                    } else if end == BITS && start == 0 {
                         // -1 = all bits set
                         (0, -1)
                     } else {
                         (
                             start as u8,
                             // 2^bits - 1
-                            (((2 as BASE).pow((end - start) as u32) - 1) as crate::INT) << start,
+                            (((2 as crate::UINT).pow((end - start) as u32) - 1) as crate::INT)
+                                << start,
                         )
                     }
                 } else if let Some(range) = idx.read_lock::<crate::InclusiveRange>() {
                     let start = *range.start();
                     let end = *range.end();
 
-                    if start < 0 || start as usize >= BITS {
+                    let start = calc_index(BITS, start, false, || {
-                        return Err(ERR::ErrorBitFieldBounds(BITS, start, idx_pos).into());
+                        ERR::ErrorBitFieldBounds(BITS, start, pos)
-                    } else if end < 0 || end as usize >= BITS {
+                    })?;
-                        return Err(ERR::ErrorBitFieldBounds(BITS, end, idx_pos).into());
+                    let end = calc_index(BITS, end, false, || {
-                    } else if end < start {
+                        ERR::ErrorBitFieldBounds(BITS, end, pos)
+                    })?;
+
+                    if end < start {
                         (0, 0)
-                    } else if end as usize == BITS - 1 && start == 0 {
+                    } else if end == BITS - 1 && start == 0 {
                         // -1 = all bits set
                         (0, -1)
                     } else {
                         (
                             start as u8,
                             // 2^bits - 1
-                            (((2 as BASE).pow((end - start + 1) as u32) - 1) as crate::INT)
+                            (((2 as crate::UINT).pow((end - start + 1) as u32) - 1) as crate::INT)
                                 << start,
                         )
                     }
@@ -956,39 +906,20 @@ impl Engine {
                 // val_int[idx]
                 let index = idx
                     .as_int()
-                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, idx_pos))?;
+                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, pos))?;
 
                 const BITS: usize = std::mem::size_of::<crate::INT>() * 8;
 
-                let (bit_value, offset) = if index >= 0 {
+                let bit = calc_index(BITS, index, true, || {
-                    let offset = index as usize;
+                    ERR::ErrorBitFieldBounds(BITS, index, pos)
-                    (
+                })?;
-                        if offset >= BITS {
+
-                            return Err(ERR::ErrorBitFieldBounds(BITS, index, idx_pos).into());
+                let bit_value = (*value & (1 << bit)) != 0;
-                        } else {
-                            (*value & (1 << offset)) != 0
-                        },
-                        offset as u8,
-                    )
-                } else if let Some(abs_index) = index.checked_abs() {
-                    let offset = abs_index as usize;
-                    (
-                        // Count from end if negative
-                        if offset > BITS {
-                            return Err(ERR::ErrorBitFieldBounds(BITS, index, idx_pos).into());
-                        } else {
-                            (*value & (1 << (BITS - offset))) != 0
-                        },
-                        offset as u8,
-                    )
-                } else {
-                    return Err(ERR::ErrorBitFieldBounds(BITS, index, idx_pos).into());
-                };
 
                 Ok(Target::Bit {
                     source: target,
                     value: bit_value.into(),
-                    bit: offset,
+                    bit: bit as u8,
                 })
             }
 
@@ -997,14 +928,14 @@ impl Engine {
                 // val_string[idx]
                 let index = idx
                     .as_int()
-                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, idx_pos))?;
+                    .map_err(|typ| self.make_type_mismatch_err::<crate::INT>(typ, pos))?;
 
                 let (ch, offset) = if index >= 0 {
                     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(chars_len, index, pos)
                         })?,
                         offset,
                     )
@@ -1014,13 +945,13 @@ impl Engine {
                         // 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(chars_len, index, pos)
                         })?,
                         offset,
                     )
                 } else {
                     let chars_len = s.chars().count();
-                    return Err(ERR::ErrorStringBounds(chars_len, index, idx_pos).into());
+                    return Err(ERR::ErrorStringBounds(chars_len, index, pos).into());
                 };
 
                 Ok(Target::StringChar {
@@ -1033,8 +964,6 @@ impl Engine {
             _ if use_indexers => {
                 let args = &mut [target, &mut idx];
                 let hash_get = crate::ast::FnCallHashes::from_native(global.hash_idx_get());
-                let idx_pos = Position::NONE;
-
                 self.exec_fn_call(
                     global,
                     state,
@@ -1044,7 +973,7 @@ impl Engine {
                     args,
                     true,
                     true,
-                    idx_pos,
+                    Position::NONE,
                     None,
                     level,
                 )

src/eval/mod.rs

@@ -12,4 +12,4 @@ pub use chaining::{ChainArgument, ChainType};
 pub use eval_context::EvalContext;
 pub use eval_state::EvalState;
 pub use global_state::GlobalRuntimeState;
-pub use target::Target;
+pub use target::{calc_index, calc_offset_len, Target};

src/eval/target.rs

@@ -1,11 +1,77 @@
 //! Type to hold a mutable reference to the target of an evaluation.
 
 use crate::types::dynamic::Variant;
-use crate::{Dynamic, RhaiResultOf};
+use crate::{Dynamic, EvalAltResult, RhaiResultOf, INT};
 use std::ops::{Deref, DerefMut};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 
+// Calculate an offset+len pair given an actual length of the underlying array.
+//
+// Negative starting positions count from the end.
+//
+// Values going over bounds are limited to the actual length.
+#[inline(always)]
+pub fn calc_offset_len(length: usize, start: INT, len: INT) -> (usize, usize) {
+    let start = if start < 0 {
+        start.checked_abs().map_or(0, |positive_start| {
+            length - usize::min(positive_start as usize, length)
+        })
+    } else if start as usize >= length {
+        return (length, 0);
+    } else {
+        start as usize
+    };
+
+    let len = if len <= 0 {
+        0
+    } else if len as usize > length - start {
+        length - start
+    } else {
+        len as usize
+    };
+
+    (start, len)
+}
+
+// Calculate an offset+len pair given an actual length of the underlying array.
+//
+// Negative starting positions count from the end.
+//
+// Values going over bounds call the provided closure to create an error.
+#[inline(always)]
+pub fn calc_index(
+    length: usize,
+    start: INT,
+    negative_count_from_end: bool,
+    err: impl Fn() -> EvalAltResult,
+) -> RhaiResultOf<usize> {
+    if start < 0 {
+        if negative_count_from_end {
+            // Count from end if negative
+            #[cfg(not(feature = "unchecked"))]
+            return start
+                .checked_abs()
+                .ok_or_else(|| err().into())
+                .and_then(|positive_start| {
+                    if (positive_start as usize) > length {
+                        Err(err().into())
+                    } else {
+                        Ok(length - (positive_start as usize))
+                    }
+                });
+            #[cfg(feature = "unchecked")]
+            return Ok(actual - (start.abs() as usize));
+        } else {
+            Err(err().into())
+        }
+    } else if start as usize >= length {
+        Err(err().into())
+    } else {
+        Ok(start as usize)
+    }
+}
+
 /// A type that encapsulates a mutation target for an expression with side effects.
 #[derive(Debug)]
 pub enum Target<'a> {

src/lib.rs

@@ -112,7 +112,7 @@ pub type INT = i32;
 /// If the `only_i32` feature is enabled, this will be [`u32`] instead.
 #[cfg(not(feature = "only_i32"))]
 #[allow(non_camel_case_types)]
-type UNSIGNED_INT = u64;
+type UINT = u64;
 /// The unsigned system integer base type.
 /// It is defined as [`u32`] since the `only_i32` feature is used.
 ///

src/packages/array_basic.rs

@@ -2,6 +2,7 @@
 #![allow(non_snake_case)]
 
 use crate::engine::OP_EQUALS;
+use crate::eval::calc_offset_len;
 use crate::plugin::*;
 use crate::{
     def_package, Array, Dynamic, ExclusiveRange, FnPtr, InclusiveRange, NativeCallContext,
@@ -55,23 +56,18 @@ pub mod array_functions {
             array1
         }
     }
-    pub fn insert(array: &mut Array, position: INT, item: Dynamic) {
+    pub fn insert(array: &mut Array, index: INT, item: Dynamic) {
         if array.is_empty() {
             array.push(item);
-        } else if position < 0 {
+            return;
-            if let Some(n) = position.checked_abs() {
+        }
-                if n as usize > array.len() {
+
-                    array.insert(0, item);
+        let (index, _) = calc_offset_len(array.len(), index, 0);
-                } else {
+
-                    array.insert(array.len() - n as usize, item);
+        if index >= array.len() {
-                }
-            } else {
-                array.insert(0, item);
-            }
-        } else if (position as usize) >= array.len() {
             array.push(item);
         } else {
-            array.insert(position as usize, item);
+            array.insert(index, item);
         }
     }
     #[rhai_fn(return_raw)]
@@ -137,11 +133,11 @@ pub mod array_functions {
             array.remove(0)
         }
     }
-    pub fn remove(array: &mut Array, len: INT) -> Dynamic {
+    pub fn remove(array: &mut Array, index: INT) -> Dynamic {
-        if len < 0 || (len as usize) >= array.len() {
+        if index < 0 || (index as usize) >= array.len() {
             Dynamic::UNIT
         } else {
-            array.remove(len as usize)
+            array.remove(index as usize)
         }
     }
     pub fn clear(array: &mut Array) {
@@ -190,27 +186,13 @@ pub mod array_functions {
             return;
         }
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(array.len(), start, len);
-            let arr_len = array.len();
+
-            start
+        if start >= array.len() {
-                .checked_abs()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
             array.extend(replace);
-            return;
         } else {
-            start as usize
+            array.splice(start..start + len, replace);
-        };
+        }
-
-        let len = if len < 0 {
-            0
-        } else if len as usize > array.len() - start {
-            array.len() - start
-        } else {
-            len as usize
-        };
-
-        array.splice(start..start + len, replace.into_iter());
     }
     #[rhai_fn(name = "extract")]
     pub fn extract_range(array: &mut Array, range: ExclusiveRange) -> Array {
@@ -229,24 +211,7 @@ pub mod array_functions {
             return Array::new();
         }
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(array.len(), start, len);
-            let arr_len = array.len();
-            start
-                .checked_abs()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
-            return Array::new();
-        } else {
-            start as usize
-        };
-
-        let len = if len <= 0 {
-            0
-        } else if len as usize > array.len() - start {
-            array.len() - start
-        } else {
-            len as usize
-        };
 
         if len == 0 {
             Array::new()
@@ -261,20 +226,20 @@ pub mod array_functions {
     #[rhai_fn(name = "split")]
     pub fn split_at(array: &mut Array, start: INT) -> Array {
         if array.is_empty() {
-            Array::new()
+            return Array::new();
-        } else if start < 0 {
+        }
-            if let Some(n) = start.checked_abs() {
+
-                if n as usize > array.len() {
+        let (start, len) = calc_offset_len(array.len(), start, INT::MAX);
-                    mem::take(array)
+
-                } else {
+        if start == 0 {
-                    let mut result = Array::new();
+            if len >= array.len() {
-                    result.extend(array.drain(array.len() - n as usize..));
-                    result
-                }
-            } else {
                 mem::take(array)
+            } else {
+                let mut result = Array::new();
+                result.extend(array.drain(array.len() - len..));
+                result
             }
-        } else if start as usize >= array.len() {
+        } else if start >= array.len() {
             Array::new()
         } else {
             let mut result = Array::new();
@@ -424,16 +389,7 @@ pub mod array_functions {
             return Ok(-1);
         }
 
-        let start = if start < 0 {
+        let (start, _) = calc_offset_len(array.len(), start, 0);
-            let arr_len = array.len();
-            start
-                .checked_abs()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
-            return Ok(-1);
-        } else {
-            start as usize
-        };
 
         for (i, item) in array.iter_mut().enumerate().skip(start) {
             if ctx
@@ -489,16 +445,7 @@ pub mod array_functions {
             return Ok(-1);
         }
 
-        let start = if start < 0 {
+        let (start, _) = calc_offset_len(array.len(), start, 0);
-            let arr_len = array.len();
-            start
-                .checked_abs()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
-            return Ok(-1);
-        } else {
-            start as usize
-        };
 
         for (i, item) in array.iter().enumerate().skip(start) {
             if filter
@@ -943,26 +890,13 @@ pub mod array_functions {
             return Array::new();
         }
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(array.len(), start, len);
-            let arr_len = array.len();
-            start
-                .checked_abs()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
-            return Array::new();
-        } else {
-            start as usize
-        };
 
-        let len = if len <= 0 {
+        if len == 0 {
-            0
+            Array::new()
-        } else if len as usize > array.len() - start {
-            array.len() - start
         } else {
-            len as usize
+            array.drain(start..start + len).collect()
-        };
+        }
-
-        array.drain(start..start + len).collect()
     }
     #[rhai_fn(return_raw)]
     pub fn retain(ctx: NativeCallContext, array: &mut Array, filter: FnPtr) -> RhaiResultOf<Array> {
@@ -1033,29 +967,16 @@ pub mod array_functions {
             return Array::new();
         }
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(array.len(), start, len);
-            let arr_len = array.len();
+
-            start
+        if len == 0 {
-                .checked_abs()
+            Array::new()
-                .map_or(0, |n| arr_len - usize::min(n as usize, arr_len))
-        } else if start as usize >= array.len() {
-            return mem::take(array);
         } else {
-            start as usize
+            let mut drained: Array = array.drain(..start).collect();
-        };
+            drained.extend(array.drain(len..));
 
-        let len = if len < 0 {
+            drained
-            0
+        }
-        } else if len as usize > array.len() - start {
-            array.len() - start
-        } else {
-            len as usize
-        };
-
-        let mut drained: Array = array.drain(..start).collect();
-        drained.extend(array.drain(len..));
-
-        drained
     }
     #[rhai_fn(name = "==", return_raw, pure)]
     pub fn equals(ctx: NativeCallContext, array1: &mut Array, array2: Array) -> RhaiResultOf<bool> {

src/packages/bit_field.rs

@@ -1,7 +1,8 @@
 #![allow(non_snake_case)]
 
+use crate::eval::calc_index;
 use crate::plugin::*;
-use crate::{def_package, ExclusiveRange, InclusiveRange, Position, RhaiResultOf, ERR, INT};
+use crate::{def_package, ExclusiveRange, InclusiveRange, Position, RhaiResultOf, ERR, INT, UINT};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 
@@ -19,62 +20,27 @@ mod bit_field_functions {
     const BITS: usize = std::mem::size_of::<INT>() * 8;
 
     #[rhai_fn(return_raw)]
-    pub fn get_bit(value: INT, index: INT) -> RhaiResultOf<bool> {
+    pub fn get_bit(value: INT, bit: INT) -> RhaiResultOf<bool> {
-        if index >= 0 {
+        let bit = calc_index(BITS, bit, true, || {
-            let offset = index as usize;
+            ERR::ErrorBitFieldBounds(BITS, bit, Position::NONE)
+        })?;
 
-            if offset >= BITS {
+        Ok((value & (1 << bit)) != 0)
-                Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
-            } else {
-                Ok((value & (1 << offset)) != 0)
-            }
-        } else if let Some(abs_index) = index.checked_abs() {
-            let offset = abs_index as usize;
-
-            // Count from end if negative
-            if offset > BITS {
-                Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
-            } else {
-                Ok((value & (1 << (BITS - offset))) != 0)
-            }
-        } else {
-            Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
-        }
     }
     #[rhai_fn(return_raw)]
-    pub fn set_bit(value: &mut INT, index: INT, new_value: bool) -> RhaiResultOf<()> {
+    pub fn set_bit(value: &mut INT, bit: INT, new_value: bool) -> RhaiResultOf<()> {
-        if index >= 0 {
+        let bit = calc_index(BITS, bit, true, || {
-            let offset = index as usize;
+            ERR::ErrorBitFieldBounds(BITS, bit, Position::NONE)
+        })?;
 
-            if offset >= BITS {
+        let mask = 1 << bit;
-                Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
+        if new_value {
-            } else {
+            *value |= mask;
-                let mask = 1 << offset;
-                if new_value {
-                    *value |= mask;
-                } else {
-                    *value &= !mask;
-                }
-                Ok(())
-            }
-        } else if let Some(abs_index) = index.checked_abs() {
-            let offset = abs_index as usize;
-
-            // Count from end if negative
-            if offset > BITS {
-                Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
-            } else {
-                let mask = 1 << offset;
-                if new_value {
-                    *value |= mask;
-                } else {
-                    *value &= !mask;
-                }
-                Ok(())
-            }
         } else {
-            Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into())
+            *value &= !mask;
         }
+
+        Ok(())
     }
     #[rhai_fn(name = "get_bits", return_raw)]
     pub fn get_bits_range(value: INT, range: ExclusiveRange) -> RhaiResultOf<INT> {
@@ -89,46 +55,29 @@ mod bit_field_functions {
         get_bits(value, from, to - from + 1)
     }
     #[rhai_fn(return_raw)]
-    pub fn get_bits(value: INT, index: INT, bits: INT) -> RhaiResultOf<INT> {
+    pub fn get_bits(value: INT, bit: INT, bits: INT) -> RhaiResultOf<INT> {
-        if bits < 1 {
+        if bits <= 0 {
             return Ok(0);
         }
 
-        let offset = if index >= 0 {
+        let bit = calc_index(BITS, bit, true, || {
-            let offset = index as usize;
+            ERR::ErrorBitFieldBounds(BITS, bit, Position::NONE)
+        })?;
 
-            if offset >= BITS {
+        let bits = if bit + bits as usize > BITS {
-                return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
+            BITS - bit
-            }
-
-            offset
-        } else if let Some(abs_index) = index.checked_abs() {
-            let offset = abs_index as usize;
-
-            // Count from end if negative
-            if offset > BITS {
-                return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
-            }
-            BITS - offset
-        } else {
-            return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
-        };
-
-        let bits = if offset + bits as usize > BITS {
-            BITS - offset
         } else {
             bits as usize
         };
 
-        let mut base = 1;
+        if bit == 0 && bits == BITS {
-        let mut mask = 0;
+            return Ok(value);
-
-        for _ in 0..bits {
-            mask |= base;
-            base <<= 1;
         }
 
-        Ok(((value & (mask << index)) >> index) & mask)
+        // 2^bits - 1
+        let mask = ((2 as UINT).pow(bits as u32) - 1) as crate::INT;
+
+        Ok(((value & (mask << bit)) >> bit) & mask)
     }
     #[rhai_fn(name = "set_bits", return_raw)]
     pub fn set_bits_range(
@@ -151,47 +100,31 @@ mod bit_field_functions {
         set_bits(value, from, to - from + 1, new_value)
     }
     #[rhai_fn(return_raw)]
-    pub fn set_bits(value: &mut INT, index: INT, bits: INT, new_value: INT) -> RhaiResultOf<()> {
+    pub fn set_bits(value: &mut INT, bit: INT, bits: INT, new_value: INT) -> RhaiResultOf<()> {
-        if bits < 1 {
+        if bits <= 0 {
             return Ok(());
         }
 
-        let offset = if index >= 0 {
+        let bit = calc_index(BITS, bit, true, || {
-            let offset = index as usize;
+            ERR::ErrorBitFieldBounds(BITS, bit, Position::NONE)
+        })?;
 
-            if offset >= BITS {
+        let bits = if bit + bits as usize > BITS {
-                return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
+            BITS - bit
-            }
-
-            offset
-        } else if let Some(abs_index) = index.checked_abs() {
-            let offset = abs_index as usize;
-
-            // Count from end if negative
-            if offset > BITS {
-                return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
-            }
-            BITS - offset
-        } else {
-            return Err(ERR::ErrorBitFieldBounds(BITS, index, Position::NONE).into());
-        };
-
-        let bits = if offset + bits as usize > BITS {
-            BITS - offset
         } else {
             bits as usize
         };
 
-        let mut base = 1;
+        if bit == 0 && bits == BITS {
-        let mut mask = 0;
+            *value = new_value;
-
+            return Ok(());
-        for _ in 0..bits {
-            mask |= base;
-            base <<= 1;
         }
 
-        *value &= !(mask << index);
+        // 2^bits - 1
-        *value |= (new_value & mask) << index;
+        let mask = ((2 as UINT).pow(bits as u32) - 1) as crate::INT;
+
+        *value &= !(mask << bit);
+        *value |= (new_value & mask) << bit;
 
         Ok(())
     }

src/packages/blob_basic.rs

@@ -1,6 +1,7 @@
 #![cfg(not(feature = "no_index"))]
 #![allow(non_snake_case)]
 
+use crate::eval::calc_offset_len;
 use crate::plugin::*;
 use crate::{
     def_package, Blob, Dynamic, ExclusiveRange, InclusiveRange, NativeCallContext, Position,
@@ -86,25 +87,20 @@ pub mod blob_functions {
             blob1
         }
     }
-    pub fn insert(blob: &mut Blob, position: INT, item: INT) {
+    pub fn insert(blob: &mut Blob, index: INT, item: INT) {
         let item = (item & 0x000000ff) as u8;
 
         if blob.is_empty() {
             blob.push(item);
-        } else if position < 0 {
+            return;
-            if let Some(n) = position.checked_abs() {
+        }
-                if n as usize > blob.len() {
+
-                    blob.insert(0, item);
+        let (index, _) = calc_offset_len(blob.len(), index, 0);
-                } else {
+
-                    blob.insert(blob.len() - n as usize, item);
+        if index >= blob.len() {
-                }
-            } else {
-                blob.insert(0, item);
-            }
-        } else if (position as usize) >= blob.len() {
             blob.push(item);
         } else {
-            blob.insert(position as usize, item);
+            blob.insert(index, item);
         }
     }
     #[rhai_fn(return_raw)]
@@ -196,28 +192,14 @@ pub mod blob_functions {
             *blob = replace;
             return;
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             blob.extend(replace);
-            return;
         } else {
-            start as usize
+            blob.splice(start..start + len, replace);
-        };
+        }
-
-        let len = if len < 0 {
-            0
-        } else if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
-
-        blob.splice(start..start + len, replace.into_iter());
     }
     #[rhai_fn(name = "extract")]
     pub fn extract_range(blob: &mut Blob, range: ExclusiveRange) -> Blob {
@@ -235,25 +217,14 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return Blob::new();
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
+            Blob::new()
-        } else if start as usize >= blob_len {
-            return Blob::new();
         } else {
-            start as usize
+            blob[start..start + len].to_vec()
-        };
+        }
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
-
-        blob[start..start + len].to_vec()
     }
     #[rhai_fn(name = "extract")]
     pub fn extract_tail(blob: &mut Blob, start: INT) -> Blob {
@@ -262,20 +233,20 @@ pub mod blob_functions {
     #[rhai_fn(name = "split")]
     pub fn split_at(blob: &mut Blob, start: INT) -> Blob {
         if blob.is_empty() {
-            Blob::new()
+            return Blob::new();
-        } else if start < 0 {
+        }
-            if let Some(n) = start.checked_abs() {
+
-                if n as usize > blob.len() {
+        let (start, len) = calc_offset_len(blob.len(), start, INT::MAX);
-                    mem::take(blob)
+
-                } else {
+        if start == 0 {
-                    let mut result = Blob::new();
+            if len > blob.len() {
-                    result.extend(blob.drain(blob.len() - n as usize..));
-                    result
-                }
-            } else {
                 mem::take(blob)
+            } else {
+                let mut result = Blob::new();
+                result.extend(blob.drain(blob.len() - len..));
+                result
             }
-        } else if start as usize >= blob.len() {
+        } else if start >= blob.len() {
             Blob::new()
         } else {
             let mut result = Blob::new();
@@ -299,25 +270,14 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return Blob::new();
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
+            Blob::new()
-        } else if start as usize >= blob_len {
-            return Blob::new();
         } else {
-            start as usize
+            blob.drain(start..start + len).collect()
-        };
+        }
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
-
-        blob.drain(start..start + len).collect()
     }
     #[rhai_fn(name = "retain")]
     pub fn retain_range(blob: &mut Blob, range: ExclusiveRange) -> Blob {
@@ -335,28 +295,17 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return Blob::new();
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
+            mem::take(blob)
-        } else if start as usize >= blob_len {
-            return mem::take(blob);
         } else {
-            start as usize
+            let mut drained: Blob = blob.drain(..start).collect();
-        };
+            drained.extend(blob.drain(len..));
 
-        let len = if len as usize > blob_len - start {
+            drained
-            blob_len - start
+        }
-        } else {
-            len as usize
-        };
-
-        let mut drained: Blob = blob.drain(..start).collect();
-        drained.extend(blob.drain(len..));
-
-        drained
     }
 
     #[inline]
@@ -364,23 +313,11 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return 0;
         }
-        let blob_len = blob.len();
+        let (start, len) = calc_offset_len(blob.len(), start, len);
 
-        let start = if start < 0 {
+        if len == 0 {
-            start
-                .checked_abs()
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             return 0;
-        } else {
+        }
-            start as usize
-        };
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
 
         const INT_BYTES: usize = mem::size_of::<INT>();
 
@@ -433,23 +370,12 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return;
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             return;
-        } else {
+        }
-            start as usize
-        };
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
 
         const INT_BYTES: usize = mem::size_of::<INT>();
 
@@ -502,23 +428,12 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return 0.0;
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             return 0.0;
-        } else {
+        }
-            start as usize
-        };
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
 
         const FLOAT_BYTES: usize = mem::size_of::<FLOAT>();
 
@@ -578,23 +493,12 @@ pub mod blob_functions {
         if blob.is_empty() || len <= 0 {
             return;
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             return;
-        } else {
+        }
-            start as usize
-        };
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
 
         const FLOAT_BYTES: usize = mem::size_of::<FLOAT>();
 
@@ -650,23 +554,12 @@ pub mod blob_functions {
         if len <= 0 || blob.is_empty() || string.is_empty() {
             return;
         }
-        let blob_len = blob.len();
 
-        let start = if start < 0 {
+        let (start, len) = calc_offset_len(blob.len(), start, len);
-            start
+
-                .checked_abs()
+        if len == 0 {
-                .map_or(0, |n| blob_len - usize::min(n as usize, blob_len))
-        } else if start as usize >= blob_len {
             return;
-        } else {
+        }
-            start as usize
-        };
-
-        let len = if len as usize > blob_len - start {
-            blob_len - start
-        } else {
-            len as usize
-        };
 
         let len = usize::min(len, string.len());
 

src/packages/iter_basic.rs

@@ -1,3 +1,4 @@
+use crate::eval::calc_index;
 use crate::plugin::*;
 use crate::types::dynamic::Variant;
 use crate::{def_package, ExclusiveRange, InclusiveRange, RhaiResultOf, INT};
@@ -120,30 +121,9 @@ const BITS: usize = std::mem::size_of::<INT>() * 8;
 
 impl BitRange {
     pub fn new(value: INT, from: INT, len: INT) -> RhaiResultOf<Self> {
-        let from = if from >= 0 {
+        let from = calc_index(BITS, from, true, || {
-            let offset = from as usize;
+            crate::ERR::ErrorBitFieldBounds(BITS, from, Position::NONE)
-
+        })?;
-            #[cfg(not(feature = "unchecked"))]
-            if offset >= BITS {
-                return Err(crate::ERR::ErrorBitFieldBounds(BITS, from, Position::NONE).into());
-            }
-            offset
-        } else {
-            #[cfg(not(feature = "unchecked"))]
-            if let Some(abs_from) = from.checked_abs() {
-                if (abs_from as usize) > BITS {
-                    return Err(crate::ERR::ErrorBitFieldBounds(BITS, from, Position::NONE).into());
-                }
-                BITS - (abs_from as usize)
-            } else {
-                return Err(crate::ERR::ErrorBitFieldBounds(BITS, from, Position::NONE).into());
-            }
-
-            #[cfg(feature = "unchecked")]
-            {
-                BITS - (from.abs() as usize)
-            }
-        };
 
         let len = if len < 0 {
             0

src/packages/map_basic.rs

@@ -77,8 +77,9 @@ mod map_functions {
             for (m1, v1) in map1.iter_mut() {
                 if let Some(v2) = map2.get_mut(m1) {
                     let equals = ctx
-                        .call_fn_raw(OP_EQUALS, true, false, &mut [v1, v2])
+                        .call_fn_raw(OP_EQUALS, true, false, &mut [v1, v2])?
-                        .map(|v| v.as_bool().unwrap_or(false))?;
+                        .as_bool()
+                        .unwrap_or(false);
 
                     if !equals {
                         return Ok(false);

src/packages/math_basic.rs

@@ -1,7 +1,7 @@
 #![allow(non_snake_case)]
 
 use crate::plugin::*;
-use crate::{def_package, Position, RhaiResultOf, ERR, INT, UNSIGNED_INT};
+use crate::{def_package, Position, RhaiResultOf, ERR, INT, UINT};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 
@@ -118,7 +118,7 @@ mod int_functions {
             .into());
         }
 
-        UNSIGNED_INT::from_str_radix(string.trim(), radix as u32)
+        UINT::from_str_radix(string.trim(), radix as u32)
             .map(|v| v as INT)
             .map_err(|err| {
                 ERR::ErrorArithmetic(

src/tokenizer.rs

@@ -5,7 +5,7 @@ use crate::engine::{
     KEYWORD_FN_PTR_CURRY, KEYWORD_IS_DEF_VAR, KEYWORD_PRINT, KEYWORD_THIS, KEYWORD_TYPE_OF,
 };
 use crate::func::native::OnParseTokenCallback;
-use crate::{Engine, LexError, StaticVec, INT, UNSIGNED_INT};
+use crate::{Engine, LexError, StaticVec, INT, UINT};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{
@@ -1530,7 +1530,7 @@ fn get_next_token_inner(
                             .filter(|&&c| c != NUMBER_SEPARATOR)
                             .collect();
 
-                        UNSIGNED_INT::from_str_radix(&out, radix)
+                        UINT::from_str_radix(&out, radix)
                             .map(|v| v as INT)
                             .map(Token::IntegerConstant)
                             .unwrap_or_else(|_| {

tests/arrays.rs

@@ -28,6 +28,30 @@ fn test_arrays() -> Result<(), Box<EvalAltResult>> {
             .into_typed_array::<INT>()?,
         [1, 6, 3]
     );
+    assert_eq!(
+        engine
+            .eval::<Dynamic>("let y = [1, 2, 3]; extract(y, 1, 10)")?
+            .into_typed_array::<INT>()?,
+        vec![2, 3]
+    );
+    assert_eq!(
+        engine
+            .eval::<Dynamic>("let y = [1, 2, 3]; extract(y, -3, 1)")?
+            .into_typed_array::<INT>()?,
+        vec![1]
+    );
+    assert_eq!(
+        engine
+            .eval::<Dynamic>("let y = [1, 2, 3]; extract(y, -99, 2)")?
+            .into_typed_array::<INT>()?,
+        vec![1, 2]
+    );
+    assert_eq!(
+        engine
+            .eval::<Dynamic>("let y = [1, 2, 3]; extract(y, 99, 1)")?
+            .into_typed_array::<INT>()?,
+        vec![] as Vec<INT>
+    );
 
     #[cfg(not(feature = "no_object"))]
     {

tests/bit_fields.rs

@@ -27,10 +27,24 @@ fn test_bit_fields() -> Result<(), Box<EvalAltResult>> {
     );
     assert_eq!(engine.eval::<INT>("let x = 10; get_bits(x, 1, 3)")?, 5);
     assert_eq!(engine.eval::<INT>("let x = 10; x[1..=3]")?, 5);
+    assert!(engine.eval::<INT>("let x = 10; x[1..99]").is_err());
+    assert!(engine.eval::<INT>("let x = 10; x[-1..3]").is_err());
     assert_eq!(
         engine.eval::<INT>("let x = 10; set_bits(x, 1, 3, 7); x")?,
         14
     );
+    #[cfg(target_pointer_width = "64")]
+    {
+        assert_eq!(engine.eval::<INT>("let x = 255; get_bits(x, -60, 2)")?, 3);
+        assert_eq!(
+            engine.eval::<INT>("let x = 0; set_bits(x, -64, 1, 15); x")?,
+            1
+        );
+        assert_eq!(
+            engine.eval::<INT>("let x = 0; set_bits(x, -60, 2, 15); x")?,
+            0b00110000
+        );
+    }
     assert_eq!(engine.eval::<INT>("let x = 10; x[1..4] = 7; x")?, 14);
     assert_eq!(
         engine.eval::<INT>(