Merge pull request #661 from schungx/master

Add loop expressions.

CHANGELOG.md

@@ -10,6 +10,7 @@ Bug fixes
 * `Engine::parse_json` now returns an error on unquoted keys to be consistent with JSON specifications.
 * `import` statements inside `eval` no longer cause errors in subsequent code.
 * Functions marked `global` in `import`ed modules with no alias names now work properly.
+* Incorrect loop optimizations that are too aggressive (e.g. unrolling a `do { ... } until true` with a `break` statement inside) and cause crashes are removed.
 
 Speed Improvements
 ------------------
@@ -19,6 +20,12 @@ Speed Improvements
 New features
 ------------
 
+### Loop expressions
+
+* Loops (such as `loop`, `do`, `while` and `for`) can now act as _expressions_, with the `break` statement returning an optional value.
+* Normal loops return `()` as the value.
+* Loop expressions can be enabled/disabled via `Engine::set_allow_loop_expressions`
+
 ### Stable hashing
 
 * It is now possible to specify a fixed _seed_ for use with the `ahash` hasher, via an environment variable, in order to force stable (i.e. deterministic) hashes for function signatures.

src/api/definitions/mod.rs

@@ -442,8 +442,8 @@ impl Module {
 
             if f.access != FnAccess::Private {
                 #[cfg(not(feature = "no_custom_syntax"))]
-                let operator = def.engine.custom_keywords.contains_key(&f.name)
+                let operator = def.engine.custom_keywords.contains_key(f.name.as_str())
-                    || (!f.name.contains('$') && !is_valid_function_name(&f.name));
+                    || (!f.name.contains('$') && !is_valid_function_name(f.name.as_str()));
 
                 #[cfg(feature = "no_custom_syntax")]
                 let operator = !f.name.contains('$') && !is_valid_function_name(&f.name);

src/api/eval.rs

@@ -217,7 +217,7 @@ impl Engine {
         level: usize,
     ) -> RhaiResult {
         let mut caches = Caches::new();
-        global.source = ast.source_raw().clone();
+        global.source = ast.source_raw().cloned();
 
         #[cfg(not(feature = "no_module"))]
         let orig_embedded_module_resolver = std::mem::replace(

src/api/options.rs

@@ -12,23 +12,25 @@ bitflags! {
         const IF_EXPR = 0b_0000_0000_0001;
         /// Is `switch` expression allowed?
         const SWITCH_EXPR = 0b_0000_0000_0010;
+        /// Are loop expressions allowed?
+        const LOOP_EXPR = 0b_0000_0000_0100;
         /// Is statement-expression allowed?
-        const STMT_EXPR = 0b_0000_0000_0100;
+        const STMT_EXPR = 0b_0000_0000_1000;
         /// Is anonymous function allowed?
         #[cfg(not(feature = "no_function"))]
-        const ANON_FN = 0b_0000_0000_1000;
+        const ANON_FN = 0b_0000_0001_0000;
         /// Is looping allowed?
-        const LOOPING = 0b_0000_0001_0000;
+        const LOOPING = 0b_0000_0010_0000;
         /// Is variables shadowing allowed?
-        const SHADOW = 0b_0000_0010_0000;
+        const SHADOW = 0b_0000_0100_0000;
         /// Strict variables mode?
-        const STRICT_VAR = 0b_0000_0100_0000;
+        const STRICT_VAR = 0b_0000_1000_0000;
         /// Raise error if an object map property does not exist?
         /// Returns `()` if `false`.
         #[cfg(not(feature = "no_object"))]
-        const FAIL_ON_INVALID_MAP_PROPERTY = 0b_0000_1000_0000;
+        const FAIL_ON_INVALID_MAP_PROPERTY = 0b_0001_0000_0000;
         /// Fast operators mode?
-        const FAST_OPS = 0b_0001_0000_0000;
+        const FAST_OPS = 0b_0010_0000_0000;
     }
 }
 
@@ -81,6 +83,18 @@ impl Engine {
     pub fn set_allow_switch_expression(&mut self, enable: bool) {
         self.options.set(LangOptions::SWITCH_EXPR, enable);
     }
+    /// Are loop expressions allowed?
+    /// Default is `true`.
+    #[inline(always)]
+    #[must_use]
+    pub const fn allow_loop_expressions(&self) -> bool {
+        self.options.contains(LangOptions::LOOP_EXPR)
+    }
+    /// Set whether loop expressions are allowed.
+    #[inline(always)]
+    pub fn set_allow_loop_expressions(&mut self, enable: bool) {
+        self.options.set(LangOptions::LOOP_EXPR, enable);
+    }
     /// Is statement-expression allowed?
     /// Default is `true`.
     #[inline(always)]

src/api/run.rs

@@ -113,7 +113,7 @@ impl Engine {
     pub fn run_ast_with_scope(&self, scope: &mut Scope, ast: &AST) -> RhaiResultOf<()> {
         let caches = &mut Caches::new();
         let global = &mut GlobalRuntimeState::new(self);
-        global.source = ast.source_raw().clone();
+        global.source = ast.source_raw().cloned();
 
         #[cfg(not(feature = "no_module"))]
         {

src/ast/ast.rs

@@ -1,7 +1,7 @@
 //! Module defining the AST (abstract syntax tree).
 
 use super::{ASTFlags, Expr, FnAccess, Stmt, StmtBlock, StmtBlockContainer};
-use crate::{Dynamic, FnNamespace, Identifier, Position};
+use crate::{Dynamic, FnNamespace, ImmutableString, Position};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{
@@ -20,8 +20,7 @@ use std::{
 #[derive(Clone)]
 pub struct AST {
     /// Source of the [`AST`].
-    /// No source if string is empty.
+    source: Option<ImmutableString>,
-    source: Identifier,
     /// [`AST`] documentation.
     #[cfg(feature = "metadata")]
     doc: crate::SmartString,
@@ -78,7 +77,7 @@ impl AST {
         #[cfg(not(feature = "no_function"))] functions: impl Into<crate::Shared<crate::Module>>,
     ) -> Self {
         Self {
-            source: Identifier::new_const(),
+            source: None,
             #[cfg(feature = "metadata")]
             doc: crate::SmartString::new_const(),
             body: StmtBlock::new(statements, Position::NONE, Position::NONE),
@@ -98,7 +97,7 @@ impl AST {
         #[cfg(not(feature = "no_function"))] functions: impl Into<crate::Shared<crate::Module>>,
     ) -> Self {
         Self {
-            source: Identifier::new_const(),
+            source: None,
             #[cfg(feature = "metadata")]
             doc: crate::SmartString::new_const(),
             body: StmtBlock::new(statements, Position::NONE, Position::NONE),
@@ -115,7 +114,7 @@ impl AST {
     pub(crate) fn new_with_source(
         statements: impl IntoIterator<Item = Stmt>,
         #[cfg(not(feature = "no_function"))] functions: impl Into<crate::Shared<crate::Module>>,
-        source: impl Into<Identifier>,
+        source: impl Into<ImmutableString>,
     ) -> Self {
         let mut ast = Self::new(
             statements,
@@ -133,7 +132,7 @@ impl AST {
     pub fn new_with_source(
         statements: impl IntoIterator<Item = Stmt>,
         #[cfg(not(feature = "no_function"))] functions: impl Into<crate::Shared<crate::Module>>,
-        source: impl Into<Identifier>,
+        source: impl Into<ImmutableString>,
     ) -> Self {
         let mut ast = Self::new(
             statements,
@@ -148,7 +147,7 @@ impl AST {
     #[must_use]
     pub fn empty() -> Self {
         Self {
-            source: Identifier::new_const(),
+            source: None,
             #[cfg(feature = "metadata")]
             doc: crate::SmartString::new_const(),
             body: StmtBlock::NONE,
@@ -159,36 +158,39 @@ impl AST {
         }
     }
     /// Get the source, if any.
-    #[inline]
+    #[inline(always)]
     #[must_use]
     pub fn source(&self) -> Option<&str> {
-        if self.source.is_empty() {
+        self.source.as_ref().map(|s| s.as_str())
-            None
-        } else {
-            Some(self.source.as_str())
-        }
     }
     /// Get a reference to the source.
     #[inline(always)]
     #[must_use]
-    pub(crate) const fn source_raw(&self) -> &Identifier {
+    pub(crate) const fn source_raw(&self) -> Option<&ImmutableString> {
-        &self.source
+        self.source.as_ref()
     }
     /// Set the source.
     #[inline]
-    pub fn set_source(&mut self, source: impl Into<Identifier>) -> &mut Self {
+    pub fn set_source(&mut self, source: impl Into<ImmutableString>) -> &mut Self {
         let source = source.into();
+
         #[cfg(not(feature = "no_function"))]
         crate::Shared::get_mut(&mut self.lib)
             .as_mut()
             .map(|m| m.set_id(source.clone()));
-        self.source = source;
+
+        if source.is_empty() {
+            self.source = None;
+        } else {
+            self.source = Some(source);
+        }
+
         self
     }
     /// Clear the source.
     #[inline(always)]
     pub fn clear_source(&mut self) -> &mut Self {
-        self.source.clear();
+        self.source = None;
         self
     }
     /// Get the documentation (if any).
@@ -559,18 +561,18 @@ impl AST {
             lib
         };
 
-        let mut _ast = if other.source.is_empty() {
+        let mut _ast = if let Some(ref source) = other.source {
-            Self::new(
-                merged,
-                #[cfg(not(feature = "no_function"))]
-                lib,
-            )
-        } else {
             Self::new_with_source(
                 merged,
                 #[cfg(not(feature = "no_function"))]
                 lib,
-                other.source.clone(),
+                source.clone(),
+            )
+        } else {
+            Self::new(
+                merged,
+                #[cfg(not(feature = "no_function"))]
+                lib,
             )
         };
 

src/ast/expr.rs

@@ -609,7 +609,7 @@ impl Expr {
                 let mut s = SmartString::new_const();
                 for segment in x.iter() {
                     let v = segment.get_literal_value().unwrap();
-                    write!(&mut s, "{}", v).unwrap();
+                    write!(&mut s, "{v}").unwrap();
                 }
                 s.into()
             }
@@ -619,7 +619,7 @@ impl Expr {
                 if !x.is_qualified() && x.args.len() == 1 && x.name == KEYWORD_FN_PTR =>
             {
                 if let Self::StringConstant(ref s, ..) = x.args[0] {
-                    FnPtr::new(s).ok()?.into()
+                    FnPtr::new(s.clone()).ok()?.into()
                 } else {
                     return None;
                 }

src/ast/stmt.rs

@@ -581,14 +581,14 @@ pub enum Stmt {
     TryCatch(Box<TryCatchBlock>, Position),
     /// [expression][Expr]
     Expr(Box<Expr>),
-    /// `continue`/`break`
+    /// `continue`/`break` expr
     ///
     /// ### Flags
     ///
     /// * [`NONE`][ASTFlags::NONE] = `continue`
     /// * [`BREAK`][ASTFlags::BREAK] = `break`
-    BreakLoop(ASTFlags, Position),
+    BreakLoop(Option<Box<Expr>>, ASTFlags, Position),
-    /// `return`/`throw`
+    /// `return`/`throw` expr
     ///
     /// ### Flags
     ///
@@ -605,19 +605,16 @@ pub enum Stmt {
     /// Not available under `no_module`.
     #[cfg(not(feature = "no_module"))]
     Export(Box<(Ident, Ident)>, Position),
-    /// Convert a variable to shared.
+    /// Convert a list of variables to shared.
     ///
     /// Not available under `no_closure`.
     ///
     /// # Notes
     ///
     /// This variant does not map to any language structure.  It is currently only used only to
-    /// convert a normal variable into a shared variable when the variable is _captured_ by a closure.
+    /// convert normal variables into shared variables when they are _captured_ by a closure.
     #[cfg(not(feature = "no_closure"))]
-    Share(
+    Share(Box<crate::FnArgsVec<(crate::ImmutableString, Option<NonZeroUsize>, Position)>>),
-        Box<(crate::ImmutableString, Option<NonZeroUsize>)>,
-        Position,
-    ),
 }
 
 impl Default for Stmt {
@@ -684,7 +681,7 @@ impl Stmt {
             Self::Export(.., pos) => *pos,
 
             #[cfg(not(feature = "no_closure"))]
-            Self::Share(.., pos) => *pos,
+            Self::Share(x) => x[0].2,
         }
     }
     /// Override the [position][Position] of this statement.
@@ -716,7 +713,7 @@ impl Stmt {
             Self::Export(.., pos) => *pos = new_pos,
 
             #[cfg(not(feature = "no_closure"))]
-            Self::Share(.., pos) => *pos = new_pos,
+            Self::Share(x) => x.iter_mut().for_each(|(_, _, pos)| *pos = new_pos),
         }
 
         self

src/bin/rhai-dbg.rs

@@ -516,7 +516,7 @@ fn debug_callback(
 
                     if range.contains(&n) {
                         let bp = rhai::debugger::BreakPoint::AtPosition {
-                            source: source.unwrap_or("").into(),
+                            source: source.map(|s| s.into()),
                             pos: Position::new(n as u16, 0),
                             enabled: true,
                         };
@@ -546,7 +546,7 @@ fn debug_callback(
                 #[cfg(not(feature = "no_position"))]
                 ["break" | "b"] => {
                     let bp = rhai::debugger::BreakPoint::AtPosition {
-                        source: source.unwrap_or("").into(),
+                        source: source.map(|s| s.into()),
                         pos,
                         enabled: true,
                     };

src/bin/rhai-repl.rs

@@ -309,13 +309,7 @@ fn main() {
     }
 
     // Register sample functions
-    engine
+    engine.register_global_module(exported_module!(sample_functions).into());
-        .register_global_module(exported_module!(sample_functions).into())
-        .register_get_set(
-            "test",
-            |x: &mut INT| *x % 2 == 0,
-            |x: &mut INT, y: bool| if y { *x *= 2 } else { *x /= 2 },
-        );
 
     // Create scope
     let mut scope = Scope::new();

src/engine.rs

@@ -239,8 +239,10 @@ impl Engine {
             engine.print = Box::new(|s| println!("{s}"));
             engine.debug = Box::new(|s, source, pos| match (source, pos) {
                 (Some(source), crate::Position::NONE) => println!("{source} | {s}"),
+                #[cfg(not(feature = "no_position"))]
                 (Some(source), pos) => println!("{source} @ {pos:?} | {s}"),
                 (None, crate::Position::NONE) => println!("{s}"),
+                #[cfg(not(feature = "no_position"))]
                 (None, pos) => println!("{pos:?} | {s}"),
             });
         }

src/eval/cache.rs

@@ -2,7 +2,7 @@
 
 use crate::func::{CallableFunction, StraightHashMap};
 use crate::types::BloomFilterU64;
-use crate::{Identifier, StaticVec};
+use crate::{ImmutableString, StaticVec};
 use std::marker::PhantomData;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
@@ -14,7 +14,7 @@ pub struct FnResolutionCacheEntry {
     /// Function.
     pub func: CallableFunction,
     /// Optional source.
-    pub source: Option<Box<Identifier>>,
+    pub source: Option<ImmutableString>,
 }
 
 /// _(internals)_ A function resolution cache with a bloom filter.

src/eval/chaining.rs

@@ -45,8 +45,8 @@ impl Engine {
         target: &mut Target,
         root: (&str, Position),
         _parent: &Expr,
+        parent_options: ASTFlags,
         rhs: &Expr,
-        _parent_options: ASTFlags,
         idx_values: &mut FnArgsVec<Dynamic>,
         chain_type: ChainType,
         level: usize,
@@ -61,7 +61,7 @@ impl Engine {
             #[cfg(not(feature = "no_index"))]
             ChainType::Indexing => {
                 // Check for existence with the null conditional operator
-                if _parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() {
+                if parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() {
                     return Ok((Dynamic::UNIT, false));
                 }
 
@@ -70,7 +70,7 @@ impl Engine {
                 match rhs {
                     // xxx[idx].expr... | xxx[idx][expr]...
                     Expr::Dot(x, options, x_pos) | Expr::Index(x, options, x_pos)
-                        if !_parent_options.contains(ASTFlags::BREAK) =>
+                        if !parent_options.contains(ASTFlags::BREAK) =>
                     {
                         #[cfg(feature = "debugging")]
                         self.run_debugger(scope, global, lib, this_ptr, _parent, level)?;
@@ -88,8 +88,8 @@ impl Engine {
                             let obj_ptr = &mut obj;
 
                             match self.eval_dot_index_chain_helper(
-                                global, caches, lib, this_ptr, obj_ptr, root, rhs, &x.rhs,
+                                global, caches, lib, this_ptr, obj_ptr, root, rhs, *options,
-                                *options, idx_values, rhs_chain, level, new_val,
+                                &x.rhs, idx_values, rhs_chain, level, new_val,
                             ) {
                                 Ok((result, true)) if is_obj_temp_val => {
                                     (Some(obj.take_or_clone()), (result, true))
@@ -190,7 +190,7 @@ impl Engine {
             #[cfg(not(feature = "no_object"))]
             ChainType::Dotting => {
                 // Check for existence with the Elvis operator
-                if _parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() {
+                if parent_options.contains(ASTFlags::NEGATED) && target.is::<()>() {
                     return Ok((Dynamic::UNIT, false));
                 }
 
@@ -407,7 +407,7 @@ impl Engine {
                         let rhs_chain = rhs.into();
 
                         self.eval_dot_index_chain_helper(
-                            global, caches, lib, this_ptr, val_target, root, rhs, &x.rhs, *options,
+                            global, caches, lib, this_ptr, val_target, root, rhs, *options, &x.rhs,
                             idx_values, rhs_chain, level, new_val,
                         )
                         .map_err(|err| err.fill_position(*x_pos))
@@ -455,8 +455,8 @@ impl Engine {
 
                                 let (result, may_be_changed) = self
                                     .eval_dot_index_chain_helper(
-                                        global, caches, lib, this_ptr, val, root, rhs, &x.rhs,
+                                        global, caches, lib, this_ptr, val, root, rhs, *options,
-                                        *options, idx_values, rhs_chain, level, new_val,
+                                        &x.rhs, idx_values, rhs_chain, level, new_val,
                                     )
                                     .map_err(|err| err.fill_position(*x_pos))?;
 
@@ -525,8 +525,8 @@ impl Engine {
                                 let val = &mut val.into();
 
                                 self.eval_dot_index_chain_helper(
-                                    global, caches, lib, this_ptr, val, root, rhs, &x.rhs,
+                                    global, caches, lib, this_ptr, val, root, rhs, *options,
-                                    *options, idx_values, rhs_chain, level, new_val,
+                                    &x.rhs, idx_values, rhs_chain, level, new_val,
                                 )
                                 .map_err(|err| err.fill_position(pos))
                             }
@@ -612,7 +612,7 @@ impl Engine {
                 let root = (x.3.as_str(), *var_pos);
 
                 self.eval_dot_index_chain_helper(
-                    global, caches, lib, &mut None, obj_ptr, root, expr, rhs, options, idx_values,
+                    global, caches, lib, &mut None, obj_ptr, root, expr, options, rhs, idx_values,
                     chain_type, level, new_val,
                 )
             }
@@ -627,7 +627,7 @@ impl Engine {
                 let root = ("", expr.start_position());
 
                 self.eval_dot_index_chain_helper(
-                    global, caches, lib, this_ptr, obj_ptr, root, expr, rhs, options, idx_values,
+                    global, caches, lib, this_ptr, obj_ptr, root, expr, options, rhs, idx_values,
                     chain_type, level, new_val,
                 )
             }
@@ -646,7 +646,7 @@ impl Engine {
         this_ptr: &mut Option<&mut Dynamic>,
         expr: &Expr,
         parent_options: ASTFlags,
-        _parent_chain_type: ChainType,
+        parent_chain_type: ChainType,
         idx_values: &mut FnArgsVec<Dynamic>,
         level: usize,
     ) -> RhaiResultOf<()> {
@@ -655,7 +655,7 @@ impl Engine {
         match expr {
             #[cfg(not(feature = "no_object"))]
             Expr::MethodCall(x, ..)
-                if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
+                if parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
             {
                 for arg_expr in &x.args {
                     idx_values.push(
@@ -666,12 +666,12 @@ impl Engine {
                 }
             }
             #[cfg(not(feature = "no_object"))]
-            Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => {
+            Expr::MethodCall(..) if parent_chain_type == ChainType::Dotting => {
                 unreachable!("function call in dot chain should not be namespace-qualified")
             }
 
             #[cfg(not(feature = "no_object"))]
-            Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (),
+            Expr::Property(..) if parent_chain_type == ChainType::Dotting => (),
             Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"),
 
             Expr::Index(x, options, ..) | Expr::Dot(x, options, ..)
@@ -684,12 +684,12 @@ impl Engine {
                 // Evaluate in left-to-right order
                 match lhs {
                     #[cfg(not(feature = "no_object"))]
-                    Expr::Property(..) if _parent_chain_type == ChainType::Dotting => (),
+                    Expr::Property(..) if parent_chain_type == ChainType::Dotting => (),
                     Expr::Property(..) => unreachable!("unexpected Expr::Property for indexing"),
 
                     #[cfg(not(feature = "no_object"))]
                     Expr::MethodCall(x, ..)
-                        if _parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
+                        if parent_chain_type == ChainType::Dotting && !x.is_qualified() =>
                     {
                         for arg_expr in &x.args {
                             _arg_values.push(
@@ -702,15 +702,15 @@ impl Engine {
                         }
                     }
                     #[cfg(not(feature = "no_object"))]
-                    Expr::MethodCall(..) if _parent_chain_type == ChainType::Dotting => {
+                    Expr::MethodCall(..) if parent_chain_type == ChainType::Dotting => {
                         unreachable!("function call in dot chain should not be namespace-qualified")
                     }
                     #[cfg(not(feature = "no_object"))]
-                    expr if _parent_chain_type == ChainType::Dotting => {
+                    expr if parent_chain_type == ChainType::Dotting => {
                         unreachable!("invalid dot expression: {:?}", expr);
                     }
                     #[cfg(not(feature = "no_index"))]
-                    _ if _parent_chain_type == ChainType::Indexing => {
+                    _ if parent_chain_type == ChainType::Indexing => {
                         _arg_values.push(
                             self.eval_expr(scope, global, caches, lib, this_ptr, lhs, level)?
                                 .flatten(),
@@ -733,11 +733,11 @@ impl Engine {
             }
 
             #[cfg(not(feature = "no_object"))]
-            _ if _parent_chain_type == ChainType::Dotting => {
+            _ if parent_chain_type == ChainType::Dotting => {
                 unreachable!("invalid dot expression: {:?}", expr);
             }
             #[cfg(not(feature = "no_index"))]
-            _ if _parent_chain_type == ChainType::Indexing => idx_values.push(
+            _ if parent_chain_type == ChainType::Indexing => idx_values.push(
                 self.eval_expr(scope, global, caches, lib, this_ptr, expr, level)?
                     .flatten(),
             ),

src/eval/data_check.rs

@@ -110,6 +110,7 @@ impl Engine {
     }
 
     /// Check whether the size of a [`Dynamic`] is within limits.
+    #[inline]
     pub(crate) fn check_data_size(&self, value: &Dynamic, pos: Position) -> RhaiResultOf<()> {
         // If no data size limits, just return
         if !self.has_data_size_limit() {

src/eval/debugger.rs

@@ -3,7 +3,9 @@
 
 use super::{EvalContext, GlobalRuntimeState};
 use crate::ast::{ASTNode, Expr, Stmt};
-use crate::{Dynamic, Engine, EvalAltResult, Identifier, Module, Position, RhaiResultOf, Scope};
+use crate::{
+    Dynamic, Engine, EvalAltResult, ImmutableString, Module, Position, RhaiResultOf, Scope,
+};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{fmt, iter::repeat, mem};
@@ -100,12 +102,10 @@ pub enum BreakPoint {
     /// Break at a particular position under a particular source.
     ///
     /// Not available under `no_position`.
-    ///
-    /// Source is empty if not available.
     #[cfg(not(feature = "no_position"))]
     AtPosition {
         /// Source (empty if not available) of the break-point.
-        source: Identifier,
+        source: Option<ImmutableString>,
         /// [Position] of the break-point.
         pos: Position,
         /// Is the break-point enabled?
@@ -114,14 +114,14 @@ pub enum BreakPoint {
     /// Break at a particular function call.
     AtFunctionName {
         /// Function name.
-        name: Identifier,
+        name: ImmutableString,
         /// Is the break-point enabled?
         enabled: bool,
     },
     /// Break at a particular function call with a particular number of arguments.
     AtFunctionCall {
         /// Function name.
-        name: Identifier,
+        name: ImmutableString,
         /// Number of arguments.
         args: usize,
         /// Is the break-point enabled?
@@ -133,7 +133,7 @@ pub enum BreakPoint {
     #[cfg(not(feature = "no_object"))]
     AtProperty {
         /// Property name.
-        name: Identifier,
+        name: ImmutableString,
         /// Is the break-point enabled?
         enabled: bool,
     },
@@ -148,7 +148,7 @@ impl fmt::Display for BreakPoint {
                 pos,
                 enabled,
             } => {
-                if !source.is_empty() {
+                if let Some(ref source) = source {
                     write!(f, "{source} ")?;
                 }
                 write!(f, "@ {pos:?}")?;
@@ -223,11 +223,11 @@ impl BreakPoint {
 #[derive(Debug, Clone, Hash)]
 pub struct CallStackFrame {
     /// Function name.
-    pub fn_name: Identifier,
+    pub fn_name: ImmutableString,
     /// Copies of function call arguments, if any.
     pub args: crate::StaticVec<Dynamic>,
-    /// Source of the function, empty if none.
+    /// Source of the function.
-    pub source: Identifier,
+    pub source: Option<ImmutableString>,
     /// [Position][`Position`] of the function call.
     pub pos: Position,
 }
@@ -243,8 +243,8 @@ impl fmt::Display for CallStackFrame {
         fp.finish()?;
 
         if !self.pos.is_none() {
-            if !self.source.is_empty() {
+            if let Some(ref source) = self.source {
-                write!(f, ": {}", self.source)?;
+                write!(f, ": {source}")?;
             }
             write!(f, " @ {:?}", self.pos)?;
         }
@@ -293,15 +293,15 @@ impl Debugger {
     #[inline(always)]
     pub(crate) fn push_call_stack_frame(
         &mut self,
-        fn_name: impl Into<Identifier>,
+        fn_name: ImmutableString,
         args: crate::StaticVec<Dynamic>,
-        source: impl Into<Identifier>,
+        source: Option<ImmutableString>,
         pos: Position,
     ) {
         self.call_stack.push(CallStackFrame {
             fn_name: fn_name.into(),
             args,
-            source: source.into(),
+            source,
             pos,
         });
     }
@@ -328,7 +328,7 @@ impl Debugger {
     }
     /// Returns the first break-point triggered by a particular [`AST` Node][ASTNode].
     #[must_use]
-    pub fn is_break_point(&self, src: &str, node: ASTNode) -> Option<usize> {
+    pub fn is_break_point(&self, src: Option<&str>, node: ASTNode) -> Option<usize> {
         let _src = src;
 
         self.break_points()
@@ -340,11 +340,12 @@ impl Debugger {
                 BreakPoint::AtPosition { pos, .. } if pos.is_none() => false,
                 #[cfg(not(feature = "no_position"))]
                 BreakPoint::AtPosition { source, pos, .. } if pos.is_beginning_of_line() => {
-                    node.position().line().unwrap_or(0) == pos.line().unwrap() && _src == source
+                    node.position().line().unwrap_or(0) == pos.line().unwrap()
+                        && _src == source.as_ref().map(|s| s.as_str())
                 }
                 #[cfg(not(feature = "no_position"))]
                 BreakPoint::AtPosition { source, pos, .. } => {
-                    node.position() == *pos && _src == source
+                    node.position() == *pos && _src == source.as_ref().map(|s| s.as_str())
                 }
                 BreakPoint::AtFunctionName { name, .. } => match node {
                     ASTNode::Expr(Expr::FnCall(x, ..)) | ASTNode::Stmt(Stmt::FnCall(x, ..)) => {
@@ -487,7 +488,7 @@ impl Engine {
 
         let event = match event {
             Some(e) => e,
-            None => match global.debugger.is_break_point(&global.source, node) {
+            None => match global.debugger.is_break_point(global.source(), node) {
                 Some(bp) => DebuggerEvent::BreakPoint(bp),
                 None => return Ok(None),
             },
@@ -512,17 +513,12 @@ impl Engine {
         event: DebuggerEvent,
         level: usize,
     ) -> Result<Option<DebuggerStatus>, Box<crate::EvalAltResult>> {
-        let source = global.source.clone();
+        let src = global.source_raw().cloned();
-        let source = if source.is_empty() {
+        let src = src.as_ref().map(|s| s.as_str());
-            None
-        } else {
-            Some(source.as_str())
-        };
-
         let context = crate::EvalContext::new(self, scope, global, None, lib, this_ptr, level);
 
         if let Some((.., ref on_debugger)) = self.debugger {
-            let command = on_debugger(context, event, node, source, node.position())?;
+            let command = on_debugger(context, event, node, src, node.position())?;
 
             match command {
                 DebuggerCommand::Continue => {

src/eval/eval_context.rs

@@ -58,11 +58,7 @@ impl<'a, 's, 'ps, 'g, 'pg, 'c, 'pc, 't, 'pt> EvalContext<'a, 's, 'ps, 'g, 'pg, '
     #[inline(always)]
     #[must_use]
     pub fn source(&self) -> Option<&str> {
-        if self.global.source.is_empty() {
+        self.global.source()
-            None
-        } else {
-            Some(self.global.source.as_str())
-        }
     }
     /// The current [`Scope`].
     #[inline(always)]

src/eval/global_state.rs

@@ -1,6 +1,6 @@
 //! Global runtime state.
 
-use crate::{Dynamic, Engine, Identifier};
+use crate::{Dynamic, Engine, ImmutableString};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{fmt, marker::PhantomData};
@@ -9,7 +9,7 @@ use std::{fmt, marker::PhantomData};
 #[cfg(not(feature = "no_module"))]
 #[cfg(not(feature = "no_function"))]
 pub type GlobalConstants =
-    crate::Shared<crate::Locked<std::collections::BTreeMap<crate::ImmutableString, Dynamic>>>;
+    crate::Shared<crate::Locked<std::collections::BTreeMap<ImmutableString, Dynamic>>>;
 
 /// _(internals)_ Global runtime states.
 /// Exported under the `internals` feature only.
@@ -25,14 +25,14 @@ pub type GlobalConstants =
 pub struct GlobalRuntimeState<'a> {
     /// Names of imported [modules][crate::Module].
     #[cfg(not(feature = "no_module"))]
-    imports: crate::StaticVec<crate::ImmutableString>,
+    imports: crate::StaticVec<ImmutableString>,
     /// Stack of imported [modules][crate::Module].
     #[cfg(not(feature = "no_module"))]
     modules: crate::StaticVec<crate::Shared<crate::Module>>,
     /// Source of the current context.
     ///
     /// No source if the string is empty.
-    pub source: Identifier,
+    pub source: Option<ImmutableString>,
     /// Number of operations performed.
     pub num_operations: u64,
     /// Number of modules loaded.
@@ -84,7 +84,7 @@ impl GlobalRuntimeState<'_> {
             imports: crate::StaticVec::new_const(),
             #[cfg(not(feature = "no_module"))]
             modules: crate::StaticVec::new_const(),
-            source: Identifier::new_const(),
+            source: None,
             num_operations: 0,
             #[cfg(not(feature = "no_module"))]
             num_modules_loaded: 0,
@@ -168,7 +168,7 @@ impl GlobalRuntimeState<'_> {
     #[inline(always)]
     pub fn push_import(
         &mut self,
-        name: impl Into<crate::ImmutableString>,
+        name: impl Into<ImmutableString>,
         module: impl Into<crate::Shared<crate::Module>>,
     ) {
         self.imports.push(name.into());
@@ -202,7 +202,7 @@ impl GlobalRuntimeState<'_> {
     #[inline]
     pub(crate) fn iter_imports_raw(
         &self,
-    ) -> impl Iterator<Item = (&crate::ImmutableString, &crate::Shared<crate::Module>)> {
+    ) -> impl Iterator<Item = (&ImmutableString, &crate::Shared<crate::Module>)> {
         self.imports.iter().zip(self.modules.iter()).rev()
     }
     /// Get an iterator to the stack of globally-imported [modules][crate::Module] in forward order.
@@ -212,7 +212,7 @@ impl GlobalRuntimeState<'_> {
     #[inline]
     pub fn scan_imports_raw(
         &self,
-    ) -> impl Iterator<Item = (&crate::ImmutableString, &crate::Shared<crate::Module>)> {
+    ) -> impl Iterator<Item = (&ImmutableString, &crate::Shared<crate::Module>)> {
         self.imports.iter().zip(self.modules.iter())
     }
     /// Can the particular function with [`Dynamic`] parameter(s) exist in the stack of
@@ -247,11 +247,11 @@ impl GlobalRuntimeState<'_> {
     pub fn get_qualified_fn(
         &self,
         hash: u64,
-    ) -> Option<(&crate::func::CallableFunction, Option<&str>)> {
+    ) -> Option<(&crate::func::CallableFunction, Option<&ImmutableString>)> {
         self.modules
             .iter()
             .rev()
-            .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id())))
+            .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw())))
     }
     /// Does the specified [`TypeId`][std::any::TypeId] iterator exist in the stack of
     /// globally-imported [modules][crate::Module]?
@@ -278,14 +278,16 @@ impl GlobalRuntimeState<'_> {
             .find_map(|m| m.get_qualified_iter(id))
     }
     /// Get the current source.
-    #[inline]
+    #[inline(always)]
     #[must_use]
     pub fn source(&self) -> Option<&str> {
-        if self.source.is_empty() {
+        self.source.as_ref().map(|s| s.as_str())
-            None
+    }
-        } else {
+    /// Get the current source.
-            Some(self.source.as_str())
+    #[inline(always)]
-        }
+    #[must_use]
+    pub(crate) const fn source_raw(&self) -> Option<&ImmutableString> {
+        self.source.as_ref()
     }
     /// Get the pre-calculated index getter hash.
     #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))]
@@ -317,10 +319,10 @@ impl GlobalRuntimeState<'_> {
 
 #[cfg(not(feature = "no_module"))]
 impl IntoIterator for GlobalRuntimeState<'_> {
-    type Item = (crate::ImmutableString, crate::Shared<crate::Module>);
+    type Item = (ImmutableString, crate::Shared<crate::Module>);
     type IntoIter = std::iter::Rev<
         std::iter::Zip<
-            smallvec::IntoIter<[crate::ImmutableString; crate::STATIC_VEC_INLINE_SIZE]>,
+            smallvec::IntoIter<[ImmutableString; crate::STATIC_VEC_INLINE_SIZE]>,
             smallvec::IntoIter<[crate::Shared<crate::Module>; crate::STATIC_VEC_INLINE_SIZE]>,
         >,
     >;
@@ -332,10 +334,10 @@ impl IntoIterator for GlobalRuntimeState<'_> {
 
 #[cfg(not(feature = "no_module"))]
 impl<'a> IntoIterator for &'a GlobalRuntimeState<'_> {
-    type Item = (&'a crate::ImmutableString, &'a crate::Shared<crate::Module>);
+    type Item = (&'a ImmutableString, &'a crate::Shared<crate::Module>);
     type IntoIter = std::iter::Rev<
         std::iter::Zip<
-            std::slice::Iter<'a, crate::ImmutableString>,
+            std::slice::Iter<'a, ImmutableString>,
             std::slice::Iter<'a, crate::Shared<crate::Module>>,
         >,
     >;
@@ -346,7 +348,7 @@ impl<'a> IntoIterator for &'a GlobalRuntimeState<'_> {
 }
 
 #[cfg(not(feature = "no_module"))]
-impl<K: Into<crate::ImmutableString>, M: Into<crate::Shared<crate::Module>>> Extend<(K, M)>
+impl<K: Into<ImmutableString>, M: Into<crate::Shared<crate::Module>>> Extend<(K, M)>
     for GlobalRuntimeState<'_>
 {
     #[inline]

src/eval/stmt.rs

@@ -493,7 +493,7 @@ impl Engine {
                             Ok(_) => (),
                             Err(err) => match *err {
                                 ERR::LoopBreak(false, ..) => (),
-                                ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT),
+                                ERR::LoopBreak(true, value, ..) => break Ok(value),
                                 _ => break Err(err),
                             },
                         }
@@ -524,7 +524,7 @@ impl Engine {
                                 Ok(_) => (),
                                 Err(err) => match *err {
                                     ERR::LoopBreak(false, ..) => (),
-                                    ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT),
+                                    ERR::LoopBreak(true, value, ..) => break Ok(value),
                                     _ => break Err(err),
                                 },
                             }
@@ -547,7 +547,7 @@ impl Engine {
                             Ok(_) => (),
                             Err(err) => match *err {
                                 ERR::LoopBreak(false, ..) => continue,
-                                ERR::LoopBreak(true, ..) => break Ok(Dynamic::UNIT),
+                                ERR::LoopBreak(true, value, ..) => break Ok(value),
                                 _ => break Err(err),
                             },
                         }
@@ -614,7 +614,7 @@ impl Engine {
 
                         let loop_result = func(iter_obj)
                             .enumerate()
-                            .try_for_each(|(x, iter_value)| {
+                            .try_fold(Dynamic::UNIT, |_, (x, iter_value)| {
                                 // Increment counter
                                 if counter_index < usize::MAX {
                                     // As the variable increments from 0, this should always work
@@ -644,26 +644,26 @@ impl Engine {
                                 self.track_operation(global, statements.position())?;
 
                                 if statements.is_empty() {
-                                    return Ok(());
+                                    return Ok(Dynamic::UNIT);
                                 }
 
                                 self.eval_stmt_block(
                                     scope, global, caches, lib, this_ptr, statements, true, level,
                                 )
-                                .map(|_| ())
+                                .map(|_| Dynamic::UNIT)
                                 .or_else(|err| match *err {
-                                    ERR::LoopBreak(false, ..) => Ok(()),
+                                    ERR::LoopBreak(false, ..) => Ok(Dynamic::UNIT),
                                     _ => Err(err),
                                 })
                             })
                             .or_else(|err| match *err {
-                                ERR::LoopBreak(true, ..) => Ok(()),
+                                ERR::LoopBreak(true, value, ..) => Ok(value),
                                 _ => Err(err),
                             });
 
                         scope.rewind(orig_scope_len);
 
-                        loop_result.map(|_| Dynamic::UNIT)
+                        loop_result
                     } else {
                         Err(ERR::ErrorFor(expr.start_position()).into())
                     }
@@ -673,8 +673,15 @@ impl Engine {
             }
 
             // Continue/Break statement
-            Stmt::BreakLoop(options, pos) => {
+            Stmt::BreakLoop(expr, options, pos) => {
-                Err(ERR::LoopBreak(options.contains(ASTFlags::BREAK), *pos).into())
+                let is_break = options.contains(ASTFlags::BREAK);
+
+                if let Some(ref expr) = expr {
+                    self.eval_expr(scope, global, caches, lib, this_ptr, expr, level)
+                        .and_then(|v| ERR::LoopBreak(is_break, v, *pos).into())
+                } else {
+                    Err(ERR::LoopBreak(is_break, Dynamic::UNIT, *pos).into())
+                }
             }
 
             // Try/Catch statement
@@ -712,8 +719,8 @@ impl Engine {
 
                                 err_map.insert("message".into(), err.to_string().into());
 
-                                if !global.source.is_empty() {
+                                if let Some(ref source) = global.source {
-                                    err_map.insert("source".into(), global.source.clone().into());
+                                    err_map.insert("source".into(), source.into());
                                 }
 
                                 if !err_pos.is_none() {
@@ -970,23 +977,25 @@ impl Engine {
 
             // Share statement
             #[cfg(not(feature = "no_closure"))]
-            Stmt::Share(x, pos) => {
+            Stmt::Share(x) => {
-                let (name, index) = &**x;
+                x.iter()
+                    .try_for_each(|(name, index, pos)| {
+                        if let Some(index) = index
+                            .map(|n| scope.len() - n.get())
+                            .or_else(|| scope.search(name))
+                        {
+                            let val = scope.get_mut_by_index(index);
 
-                if let Some(index) = index
+                            if !val.is_shared() {
-                    .map(|n| scope.len() - n.get())
+                                // Replace the variable with a shared value.
-                    .or_else(|| scope.search(name))
+                                *val = std::mem::take(val).into_shared();
-                {
+                            }
-                    let val = scope.get_mut_by_index(index);
+                            Ok(())
-
+                        } else {
-                    if !val.is_shared() {
+                            Err(ERR::ErrorVariableNotFound(name.to_string(), *pos).into())
-                        // Replace the variable with a shared value.
+                        }
-                        *val = std::mem::take(val).into_shared();
+                    })
-                    }
+                    .map(|_| Dynamic::UNIT)
-                    Ok(Dynamic::UNIT)
-                } else {
-                    Err(ERR::ErrorVariableNotFound(name.to_string(), *pos).into())
-                }
             }
 
             _ => unreachable!("statement cannot be evaluated: {:?}", stmt),

src/func/call.rs

@@ -218,7 +218,7 @@ impl Engine {
                         .iter()
                         .copied()
                         .chain(self.global_modules.iter().map(|m| m.as_ref()))
-                        .find_map(|m| m.get_fn(hash).map(|f| (f, m.id())));
+                        .find_map(|m| m.get_fn(hash).map(|f| (f, m.id_raw())));
 
                     #[cfg(not(feature = "no_module"))]
                     let func = if args.is_none() {
@@ -228,7 +228,7 @@ impl Engine {
                         func.or_else(|| _global.get_qualified_fn(hash)).or_else(|| {
                             self.global_sub_modules
                                 .values()
-                                .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id())))
+                                .find_map(|m| m.get_qualified_fn(hash).map(|f| (f, m.id_raw())))
                         })
                     };
 
@@ -236,7 +236,7 @@ impl Engine {
                         // Specific version found
                         let new_entry = Some(FnResolutionCacheEntry {
                             func: f.clone(),
-                            source: s.map(|s| Box::new(s.into())),
+                            source: s.cloned(),
                         });
                         return if cache.filter.is_absent_and_set(hash) {
                             // Do not cache "one-hit wonders"
@@ -358,7 +358,6 @@ impl Engine {
     ) -> RhaiResultOf<(Dynamic, bool)> {
         self.track_operation(global, pos)?;
 
-        let parent_source = global.source.clone();
         let op_assign = if is_op_assign {
             Token::lookup_from_syntax(name)
         } else {
@@ -398,24 +397,19 @@ impl Engine {
                     backup.change_first_arg_to_copy(args);
                 }
 
-                let source = match (source, parent_source.as_str()) {
-                    (None, "") => None,
-                    (None, s) => Some(s),
-                    (Some(s), ..) => Some(s.as_str()),
-                };
-
                 #[cfg(feature = "debugging")]
                 if self.debugger.is_some() {
                     global.debugger.push_call_stack_frame(
-                        name,
+                        self.get_interned_string(name),
                         args.iter().map(|v| (*v).clone()).collect(),
-                        source.unwrap_or(""),
+                        source.clone().or_else(|| global.source.clone()),
                         pos,
                     );
                 }
 
                 // Run external function
-                let context = (self, name, source, &*global, lib, pos, level).into();
+                let src = source.as_ref().map(|s| s.as_str());
+                let context = (self, name, src, &*global, lib, pos, level).into();
 
                 let result = if func.is_plugin_fn() {
                     let f = func.get_plugin_fn().unwrap();
@@ -484,12 +478,7 @@ impl Engine {
                         let t = self.map_type_name(type_name::<ImmutableString>()).into();
                         ERR::ErrorMismatchOutputType(t, typ.into(), pos)
                     })?;
-                    let source = if global.source.is_empty() {
+                    ((*self.debug)(&text, global.source(), pos).into(), false)
-                        None
-                    } else {
-                        Some(global.source.as_str())
-                    };
-                    ((*self.debug)(&text, source, pos).into(), false)
                 }
                 _ => (result, is_method),
             });
@@ -685,12 +674,7 @@ impl Engine {
                     }
                 };
 
-                let orig_source = mem::replace(
+                let orig_source = mem::replace(&mut global.source, source.clone());
-                    &mut global.source,
-                    source
-                        .as_ref()
-                        .map_or(crate::Identifier::new_const(), |s| (**s).clone()),
-                );
 
                 let result = if _is_method_call {
                     // Method call of script function - map first argument to `this`
@@ -1172,7 +1156,7 @@ impl Engine {
                 return result.map_err(|err| {
                     ERR::ErrorInFunctionCall(
                         KEYWORD_EVAL.to_string(),
-                        global.source.to_string(),
+                        global.source().unwrap_or("").to_string(),
                         err,
                         pos,
                     )
@@ -1416,14 +1400,13 @@ impl Engine {
             Some(f) if f.is_script() => {
                 let fn_def = f.get_script_fn_def().expect("script-defined function");
                 let new_scope = &mut Scope::new();
-                let mut source = module.id_raw().clone();
+                let orig_source = mem::replace(&mut global.source, module.id_raw().cloned());
-                mem::swap(&mut global.source, &mut source);
 
                 let result = self.call_script_fn(
                     new_scope, global, caches, lib, &mut None, fn_def, &mut args, true, pos, level,
                 );
 
-                global.source = source;
+                global.source = orig_source;
 
                 result
             }

src/func/script.rs

@@ -54,7 +54,7 @@ impl Engine {
 
             Err(ERR::ErrorInFunctionCall(
                 name,
-                source.unwrap_or_else(|| global.source.to_string()),
+                source.unwrap_or_else(|| global.source().unwrap_or("").to_string()),
                 err,
                 pos,
             )

src/module/mod.rs

@@ -72,7 +72,7 @@ pub struct FuncInfo {
     /// Function access mode.
     pub access: FnAccess,
     /// Function name.
-    pub name: Identifier,
+    pub name: ImmutableString,
     /// Number of parameters.
     pub num_params: usize,
     /// Parameter types (if applicable).
@@ -160,8 +160,7 @@ pub fn calc_native_fn_hash<'a>(
 #[derive(Clone)]
 pub struct Module {
     /// ID identifying the module.
-    /// No ID if string is empty.
+    id: Option<ImmutableString>,
-    id: Identifier,
     /// Module documentation.
     #[cfg(feature = "metadata")]
     doc: crate::SmartString,
@@ -292,7 +291,7 @@ impl Module {
     #[must_use]
     pub fn with_capacity(capacity: usize) -> Self {
         Self {
-            id: Identifier::new_const(),
+            id: None,
             #[cfg(feature = "metadata")]
             doc: crate::SmartString::new_const(),
             internal: false,
@@ -324,18 +323,14 @@ impl Module {
     #[inline]
     #[must_use]
     pub fn id(&self) -> Option<&str> {
-        if self.id_raw().is_empty() {
+        self.id.as_ref().map(|s| s.as_str())
-            None
-        } else {
-            Some(self.id_raw())
-        }
     }
 
     /// Get the ID of the [`Module`] as an [`Identifier`], if any.
     #[inline(always)]
     #[must_use]
-    pub(crate) const fn id_raw(&self) -> &Identifier {
+    pub(crate) const fn id_raw(&self) -> Option<&ImmutableString> {
-        &self.id
+        self.id.as_ref()
     }
 
     /// Set the ID of the [`Module`].
@@ -351,8 +346,15 @@ impl Module {
     /// assert_eq!(module.id(), Some("hello"));
     /// ```
     #[inline(always)]
-    pub fn set_id(&mut self, id: impl Into<Identifier>) -> &mut Self {
+    pub fn set_id(&mut self, id: impl Into<ImmutableString>) -> &mut Self {
-        self.id = id.into();
+        let id = id.into();
+
+        if id.is_empty() {
+            self.id = None;
+        } else {
+            self.id = Some(id);
+        }
+
         self
     }
 
@@ -370,7 +372,7 @@ impl Module {
     /// ```
     #[inline(always)]
     pub fn clear_id(&mut self) -> &mut Self {
-        self.id.clear();
+        self.id = None;
         self
     }
 
@@ -434,7 +436,7 @@ impl Module {
     /// Clear the [`Module`].
     #[inline(always)]
     pub fn clear(&mut self) {
-        self.id.clear();
+        self.id = None;
         #[cfg(feature = "metadata")]
         self.doc.clear();
         self.internal = false;
@@ -2078,7 +2080,7 @@ impl Module {
                 });
         }
 
-        module.set_id(ast.source_raw().clone());
+        module.id = ast.source_raw().cloned();
 
         #[cfg(feature = "metadata")]
         module.set_doc(ast.doc());

src/optimizer.rs

@@ -8,7 +8,7 @@ use crate::engine::{KEYWORD_DEBUG, KEYWORD_EVAL, KEYWORD_FN_PTR, KEYWORD_PRINT,
 use crate::eval::{Caches, GlobalRuntimeState};
 use crate::func::builtin::get_builtin_binary_op_fn;
 use crate::func::hashing::get_hasher;
-use crate::tokenizer::{Span, Token};
+use crate::tokenizer::Token;
 use crate::types::dynamic::AccessMode;
 use crate::{
     calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, Engine, FnPtr, Identifier,
@@ -785,50 +785,6 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
                 *condition = Expr::Unit(*pos);
             }
             **body = optimize_stmt_block(mem::take(&mut **body), state, false, true, false);
-
-            if body.len() == 1 {
-                match body[0] {
-                    // while expr { break; } -> { expr; }
-                    Stmt::BreakLoop(options, pos) if options.contains(ASTFlags::BREAK) => {
-                        // Only a single break statement - turn into running the guard expression once
-                        state.set_dirty();
-                        if condition.is_unit() {
-                            *stmt = Stmt::Noop(pos);
-                        } else {
-                            let mut statements = vec![Stmt::Expr(mem::take(condition).into())];
-                            if preserve_result {
-                                statements.push(Stmt::Noop(pos));
-                            }
-                            *stmt = (statements, Span::new(pos, Position::NONE)).into();
-                        };
-                    }
-                    _ => (),
-                }
-            }
-        }
-        // do { block } until true -> { block }
-        Stmt::Do(x, options, ..)
-            if matches!(x.0, Expr::BoolConstant(true, ..))
-                && options.contains(ASTFlags::NEGATED) =>
-        {
-            state.set_dirty();
-            *stmt = (
-                optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false),
-                x.1.span(),
-            )
-                .into();
-        }
-        // do { block } while false -> { block }
-        Stmt::Do(x, options, ..)
-            if matches!(x.0, Expr::BoolConstant(false, ..))
-                && !options.contains(ASTFlags::NEGATED) =>
-        {
-            state.set_dirty();
-            *stmt = (
-                optimize_stmt_block(mem::take(&mut *x.1), state, false, true, false),
-                x.1.span(),
-            )
-                .into();
         }
         // do { block } while|until expr
         Stmt::Do(x, ..) => {
@@ -916,6 +872,9 @@ fn optimize_stmt(stmt: &mut Stmt, state: &mut OptimizerState, preserve_result: b
             }
         }
 
+        // break expr;
+        Stmt::BreakLoop(Some(ref mut expr), ..) => optimize_expr(expr, state, false),
+
         // return expr;
         Stmt::Return(Some(ref mut expr), ..) => optimize_expr(expr, state, false),
 

src/packages/debugging.rs

@@ -40,7 +40,7 @@ mod debugging_functions {
                 .iter()
                 .rev()
                 .filter(|crate::debugger::CallStackFrame { fn_name, args, .. }| {
-                    fn_name != "back_trace" || !args.is_empty()
+                    fn_name.as_str() != "back_trace" || !args.is_empty()
                 })
                 .map(
                     |frame @ crate::debugger::CallStackFrame {
@@ -62,8 +62,8 @@ mod debugging_functions {
                                     Dynamic::from_array(_args.clone().to_vec()),
                                 );
                             }
-                            if !_source.is_empty() {
+                            if let Some(source) = _source {
-                                map.insert("source".into(), _source.into());
+                                map.insert("source".into(), source.into());
                             }
                             if !_pos.is_none() {
                                 map.insert(

src/packages/fn_basic.rs

@@ -27,7 +27,7 @@ mod fn_ptr_functions {
     /// ```
     #[rhai_fn(name = "name", get = "name", pure)]
     pub fn name(fn_ptr: &mut FnPtr) -> ImmutableString {
-        fn_ptr.fn_name_raw().into()
+        fn_ptr.fn_name_raw().clone()
     }
 
     /// Return `true` if the function is an anonymous function.

src/packages/iter_basic.rs

@@ -122,7 +122,7 @@ impl<T: Debug + Copy + PartialOrd> FusedIterator for StepRange<T> {}
 
 // Bit-field iterator with step
 #[derive(Debug, Clone, Hash, Eq, PartialEq)]
-pub struct BitRange(INT, INT, usize);
+pub struct BitRange(INT, usize);
 
 impl BitRange {
     pub fn new(value: INT, from: INT, len: INT) -> RhaiResultOf<Self> {
@@ -138,7 +138,7 @@ impl BitRange {
             len as usize
         };
 
-        Ok(Self(value, 1 << from, len))
+        Ok(Self(value >> from, len))
     }
 }
 
@@ -146,19 +146,19 @@ impl Iterator for BitRange {
     type Item = bool;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.2 == 0 {
+        if self.1 == 0 {
             None
         } else {
-            let r = (self.0 & self.1) != 0;
+            let r = (self.0 & 0x0001) != 0;
-            self.1 <<= 1;
+            self.0 >>= 1;
-            self.2 -= 1;
+            self.1 -= 1;
             Some(r)
         }
     }
 
     #[inline(always)]
     fn size_hint(&self) -> (usize, Option<usize>) {
-        (self.2, Some(self.2))
+        (self.1, Some(self.1))
     }
 }
 
@@ -167,7 +167,7 @@ impl FusedIterator for BitRange {}
 impl ExactSizeIterator for BitRange {
     #[inline(always)]
     fn len(&self) -> usize {
-        self.2
+        self.1
     }
 }
 

src/packages/time_basic.rs

@@ -2,7 +2,7 @@
 
 use super::arithmetic::make_err as make_arithmetic_err;
 use crate::plugin::*;
-use crate::{def_package, Dynamic, EvalAltResult, RhaiResult, RhaiResultOf, INT};
+use crate::{def_package, Dynamic, RhaiResult, RhaiResultOf, INT};
 
 #[cfg(not(feature = "no_float"))]
 use crate::FLOAT;

src/parser.rs

@@ -1380,6 +1380,23 @@ impl Engine {
                 self.parse_if(input, state, lib, settings.level_up())?
                     .into(),
             )),
+            // Loops are allowed to act as expressions
+            Token::While | Token::Loop if settings.options.contains(LangOptions::LOOP_EXPR) => {
+                Expr::Stmt(Box::new(
+                    self.parse_while_loop(input, state, lib, settings.level_up())?
+                        .into(),
+                ))
+            }
+            Token::Do if settings.options.contains(LangOptions::LOOP_EXPR) => Expr::Stmt(Box::new(
+                self.parse_do(input, state, lib, settings.level_up())?
+                    .into(),
+            )),
+            Token::For if settings.options.contains(LangOptions::LOOP_EXPR) => {
+                Expr::Stmt(Box::new(
+                    self.parse_for(input, state, lib, settings.level_up())?
+                        .into(),
+                ))
+            }
             // Switch statement is allowed to act as expressions
             Token::Switch if settings.options.contains(LangOptions::SWITCH_EXPR) => {
                 Expr::Stmt(Box::new(
@@ -3411,11 +3428,26 @@ impl Engine {
 
             Token::Continue if self.allow_looping() && settings.is_breakable => {
                 let pos = eat_token(input, Token::Continue);
-                Ok(Stmt::BreakLoop(ASTFlags::NONE, pos))
+                Ok(Stmt::BreakLoop(None, ASTFlags::NONE, pos))
             }
             Token::Break if self.allow_looping() && settings.is_breakable => {
                 let pos = eat_token(input, Token::Break);
-                Ok(Stmt::BreakLoop(ASTFlags::BREAK, pos))
+
+                let expr = match input.peek().expect(NEVER_ENDS) {
+                    // `break` at <EOF>
+                    (Token::EOF, ..) => None,
+                    // `break` at end of block
+                    (Token::RightBrace, ..) => None,
+                    // `break;`
+                    (Token::SemiColon, ..) => None,
+                    // `break`  with expression
+                    _ => Some(
+                        self.parse_expr(input, state, lib, settings.level_up())?
+                            .into(),
+                    ),
+                };
+
+                Ok(Stmt::BreakLoop(expr, ASTFlags::BREAK, pos))
             }
             Token::Continue | Token::Break if self.allow_looping() => {
                 Err(PERR::LoopBreak.into_err(token_pos))
@@ -3707,15 +3739,17 @@ impl Engine {
 
         // Convert the entire expression into a statement block, then insert the relevant
         // [`Share`][Stmt::Share] statements.
-        let mut statements = StaticVec::with_capacity(externals.len() + 1);
+        let mut statements = StaticVec::with_capacity(2);
-        statements.extend(
+        statements.push(Stmt::Share(
             externals
                 .into_iter()
                 .map(|crate::ast::Ident { name, pos }| {
                     let (index, _) = parent.access_var(&name, lib, pos);
-                    Stmt::Share((name, index).into(), pos)
+                    (name, index, pos)
-                }),
+                })
-        );
+                .collect::<crate::FnArgsVec<_>>()
+                .into(),
+        ));
         statements.push(Stmt::Expr(expr.into()));
         Expr::Stmt(crate::ast::StmtBlock::new(statements, pos, Position::NONE).into())
     }
@@ -3840,7 +3874,7 @@ impl Engine {
         let mut functions = StraightHashMap::default();
 
         let mut options = self.options;
-        options.remove(LangOptions::STMT_EXPR);
+        options.remove(LangOptions::STMT_EXPR | LangOptions::LOOP_EXPR);
         #[cfg(not(feature = "no_function"))]
         options.remove(LangOptions::ANON_FN);
 

src/tests.rs

@@ -35,7 +35,7 @@ fn check_struct_sizes() {
     #[cfg(target_pointer_width = "64")]
     {
         assert_eq!(size_of::<Scope>(), 536);
-        assert_eq!(size_of::<FnPtr>(), 80);
+        assert_eq!(size_of::<FnPtr>(), 64);
         assert_eq!(size_of::<LexError>(), 56);
         assert_eq!(
             size_of::<ParseError>(),

src/types/error.rs

@@ -117,7 +117,7 @@ pub enum EvalAltResult {
     /// Breaking out of loops - not an error if within a loop.
     /// The wrapped value, if true, means breaking clean out of the loop (i.e. a `break` statement).
     /// The wrapped value, if false, means breaking the current context (i.e. a `continue` statement).
-    LoopBreak(bool, Position),
+    LoopBreak(bool, Dynamic, Position),
     /// Not an error: Value returned from a script via the `return` keyword.
     /// Wrapped value is the result value.
     Return(Dynamic, Position),

src/types/fn_ptr.rs

@@ -3,7 +3,7 @@
 use crate::tokenizer::is_valid_identifier;
 use crate::types::dynamic::Variant;
 use crate::{
-    Dynamic, Engine, FuncArgs, Identifier, Module, NativeCallContext, Position, RhaiError,
+    Dynamic, Engine, FuncArgs, ImmutableString, Module, NativeCallContext, Position, RhaiError,
     RhaiResult, RhaiResultOf, StaticVec, AST, ERR,
 };
 #[cfg(feature = "no_std")]
@@ -18,7 +18,7 @@ use std::{
 /// to be passed onto a function during a call.
 #[derive(Clone, Hash)]
 pub struct FnPtr {
-    name: Identifier,
+    name: ImmutableString,
     curry: StaticVec<Dynamic>,
 }
 
@@ -42,13 +42,16 @@ impl fmt::Debug for FnPtr {
 impl FnPtr {
     /// Create a new function pointer.
     #[inline(always)]
-    pub fn new(name: impl Into<Identifier>) -> RhaiResultOf<Self> {
+    pub fn new(name: impl Into<ImmutableString>) -> RhaiResultOf<Self> {
         name.into().try_into()
     }
     /// Create a new function pointer without checking its parameters.
     #[inline(always)]
     #[must_use]
-    pub(crate) fn new_unchecked(name: impl Into<Identifier>, curry: StaticVec<Dynamic>) -> Self {
+    pub(crate) fn new_unchecked(
+        name: impl Into<ImmutableString>,
+        curry: StaticVec<Dynamic>,
+    ) -> Self {
         Self {
             name: name.into(),
             curry,
@@ -63,13 +66,13 @@ impl FnPtr {
     /// Get the name of the function.
     #[inline(always)]
     #[must_use]
-    pub(crate) const fn fn_name_raw(&self) -> &Identifier {
+    pub(crate) const fn fn_name_raw(&self) -> &ImmutableString {
         &self.name
     }
     /// Get the underlying data of the function pointer.
     #[inline(always)]
     #[must_use]
-    pub(crate) fn take_data(self) -> (Identifier, StaticVec<Dynamic>) {
+    pub(crate) fn take_data(self) -> (ImmutableString, StaticVec<Dynamic>) {
         (self.name, self.curry)
     }
     /// Get the curried arguments.
@@ -246,11 +249,11 @@ impl fmt::Display for FnPtr {
     }
 }
 
-impl TryFrom<Identifier> for FnPtr {
+impl TryFrom<ImmutableString> for FnPtr {
     type Error = RhaiError;
 
-    #[inline]
+    #[inline(always)]
-    fn try_from(value: Identifier) -> RhaiResultOf<Self> {
+    fn try_from(value: ImmutableString) -> RhaiResultOf<Self> {
         if is_valid_identifier(value.chars()) {
             Ok(Self {
                 name: value,
@@ -261,43 +264,3 @@ impl TryFrom<Identifier> for FnPtr {
         }
     }
 }
-
-impl TryFrom<crate::ImmutableString> for FnPtr {
-    type Error = RhaiError;
-
-    #[inline(always)]
-    fn try_from(value: crate::ImmutableString) -> RhaiResultOf<Self> {
-        let s: Identifier = value.into();
-        Self::try_from(s)
-    }
-}
-
-impl TryFrom<String> for FnPtr {
-    type Error = RhaiError;
-
-    #[inline(always)]
-    fn try_from(value: String) -> RhaiResultOf<Self> {
-        let s: Identifier = value.into();
-        Self::try_from(s)
-    }
-}
-
-impl TryFrom<Box<str>> for FnPtr {
-    type Error = RhaiError;
-
-    #[inline(always)]
-    fn try_from(value: Box<str>) -> RhaiResultOf<Self> {
-        let s: Identifier = value.into();
-        Self::try_from(s)
-    }
-}
-
-impl TryFrom<&str> for FnPtr {
-    type Error = RhaiError;
-
-    #[inline(always)]
-    fn try_from(value: &str) -> RhaiResultOf<Self> {
-        let s: Identifier = value.into();
-        Self::try_from(s)
-    }
-}

src/types/scope.rs

@@ -1,7 +1,7 @@
 //! Module that defines the [`Scope`] type representing a function call-stack scope.
 
 use super::dynamic::{AccessMode, Variant};
-use crate::{Dynamic, Identifier};
+use crate::{Dynamic, Identifier, ImmutableString};
 use smallvec::SmallVec;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
@@ -75,7 +75,7 @@ pub struct Scope<'a, const N: usize = SCOPE_ENTRIES_INLINED> {
     /// Name of the entry.
     names: SmallVec<[Identifier; SCOPE_ENTRIES_INLINED]>,
     /// Aliases of the entry.
-    aliases: SmallVec<[Vec<Identifier>; SCOPE_ENTRIES_INLINED]>,
+    aliases: SmallVec<[Vec<ImmutableString>; SCOPE_ENTRIES_INLINED]>,
     /// Phantom to keep the lifetime parameter in order not to break existing code.
     dummy: PhantomData<&'a ()>,
 }
@@ -125,7 +125,7 @@ impl Clone for Scope<'_> {
 }
 
 impl IntoIterator for Scope<'_> {
-    type Item = (String, Dynamic, Vec<Identifier>);
+    type Item = (String, Dynamic, Vec<ImmutableString>);
     type IntoIter = Box<dyn Iterator<Item = Self::Item>>;
 
     #[must_use]
@@ -140,7 +140,7 @@ impl IntoIterator for Scope<'_> {
 }
 
 impl<'a> IntoIterator for &'a Scope<'_> {
-    type Item = (&'a Identifier, &'a Dynamic, &'a Vec<Identifier>);
+    type Item = (&'a Identifier, &'a Dynamic, &'a Vec<ImmutableString>);
     type IntoIter = Box<dyn Iterator<Item = Self::Item> + 'a>;
 
     #[must_use]
@@ -669,7 +669,7 @@ impl Scope<'_> {
     /// Panics if the index is out of bounds.
     #[cfg(not(feature = "no_module"))]
     #[inline]
-    pub(crate) fn add_alias_by_index(&mut self, index: usize, alias: Identifier) -> &mut Self {
+    pub(crate) fn add_alias_by_index(&mut self, index: usize, alias: ImmutableString) -> &mut Self {
         let aliases = self.aliases.get_mut(index).unwrap();
         if aliases.is_empty() || !aliases.contains(&alias) {
             aliases.push(alias);
@@ -690,11 +690,11 @@ impl Scope<'_> {
     pub fn set_alias(
         &mut self,
         name: impl AsRef<str> + Into<Identifier>,
-        alias: impl Into<Identifier>,
+        alias: impl Into<ImmutableString>,
     ) {
         if let Some(index) = self.search(name.as_ref()) {
             let alias = match alias.into() {
-                x if x.is_empty() => name.into(),
+                x if x.is_empty() => name.into().into(),
                 x => x,
             };
             self.add_alias_by_index(index, alias);
@@ -727,7 +727,9 @@ impl Scope<'_> {
     }
     /// Get an iterator to entries in the [`Scope`].
     #[allow(dead_code)]
-    pub(crate) fn into_iter(self) -> impl Iterator<Item = (Identifier, Dynamic, Vec<Identifier>)> {
+    pub(crate) fn into_iter(
+        self,
+    ) -> impl Iterator<Item = (Identifier, Dynamic, Vec<ImmutableString>)> {
         self.names
             .into_iter()
             .zip(self.values.into_iter().zip(self.aliases.into_iter()))

tests/expressions.rs

@@ -55,10 +55,13 @@ fn test_expressions() -> Result<(), Box<EvalAltResult>> {
         )
         .is_err());
 
-    assert!(engine.eval_expression::<()>("40 + 2;").is_err());
+    assert!(engine.compile_expression("40 + 2;").is_err());
-    assert!(engine.eval_expression::<()>("40 + { 2 }").is_err());
+    assert!(engine.compile_expression("40 + { 2 }").is_err());
-    assert!(engine.eval_expression::<()>("x = 42").is_err());
+    assert!(engine.compile_expression("x = 42").is_err());
     assert!(engine.compile_expression("let x = 42").is_err());
+    assert!(engine
+        .compile_expression("do { break 42; } while true")
+        .is_err());
 
     engine.compile("40 + { let x = 2; x }")?;
 

tests/for.rs

@@ -231,6 +231,25 @@ fn test_for_loop() -> Result<(), Box<EvalAltResult>> {
         );
     }
 
+    #[cfg(not(feature = "no_index"))]
+    #[cfg(not(feature = "no_object"))]
+    #[cfg(not(feature = "no_float"))]
+    assert_eq!(
+        engine.eval::<INT>(
+            r#"
+                let a = [123, 999, 42, 0, true, "hello", "world!", 987.654];
+
+                for (item, count) in a {
+                    switch item.type_of() {
+                        "i64" | "i32" if item.is_even => break count,
+                        "f64" | "f32" if item.to_int().is_even => break count,
+                    }
+                }
+            "#
+        )?,
+        2
+    );
+
     Ok(())
 }
 

tests/modules.rs

@@ -578,7 +578,7 @@ fn test_module_context() -> Result<(), Box<EvalAltResult>> {
             let new_context = NativeCallContext::new_with_all_fields(
                 engine,
                 &fn_name,
-                source.as_ref().map(|s| s.as_str()),
+                source.as_ref().map(String::as_str),
                 &global,
                 &lib,
                 pos,

tests/optimizer.rs

@@ -89,21 +89,21 @@ fn test_optimizer_parse() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [Expr(123 @ 1:53)] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [Expr(123 @ 1:53)] }"#
     );
 
     let ast = engine.compile("const DECISION = false; if DECISION { 42 } else { 123 }")?;
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [Var(("DECISION" @ 1:7, false @ 1:18, None), CONSTANT, 1:1), Expr(123 @ 1:51)] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [Var(("DECISION" @ 1:7, false @ 1:18, None), CONSTANT, 1:1), Expr(123 @ 1:51)] }"#
     );
 
     let ast = engine.compile("if 1 == 2 { 42 }")?;
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [] }"#
     );
 
     engine.set_optimization_level(OptimizationLevel::Full);
@@ -112,14 +112,14 @@ fn test_optimizer_parse() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"#
     );
 
     let ast = engine.compile("NUMBER")?;
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [Expr(Variable(NUMBER) @ 1:1)] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [Expr(Variable(NUMBER) @ 1:1)] }"#
     );
 
     let mut module = Module::new();
@@ -131,7 +131,7 @@ fn test_optimizer_parse() -> Result<(), Box<EvalAltResult>> {
 
     assert_eq!(
         format!("{ast:?}"),
-        r#"AST { source: "", doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"#
+        r#"AST { source: None, doc: "", resolver: None, body: [Expr(42 @ 1:1)] }"#
     );
 
     Ok(())

tests/while_loop.rs

@@ -22,6 +22,22 @@ fn test_while() -> Result<(), Box<EvalAltResult>> {
         6
     );
 
+    assert_eq!(
+        engine.eval::<INT>(
+            "
+                let x = 0;
+
+                while x < 10 {
+                    x += 1;
+                    if x > 5 { break x * 2; }
+                    if x > 3 { continue; }
+                    x += 3;
+                }
+            ",
+        )?,
+        12
+    );
+
     Ok(())
 }
 
@@ -46,6 +62,25 @@ fn test_do() -> Result<(), Box<EvalAltResult>> {
         )?,
         6
     );
+    assert_eq!(
+        engine.eval::<INT>(
+            "
+                let x = 0;
+
+                do {
+                    x += 1;
+                    if x > 5 { break x * 2; }
+                    if x > 3 { continue; }
+                    x += 3;
+                } while x < 10;
+            ",
+        )?,
+        12
+    );
+
+    engine.run("do {} while false")?;
+
+    assert_eq!(engine.eval::<INT>("do { break 42; } while false")?, 42);
 
     Ok(())
 }