Merge pull request #342 from schungx/master

Improve function resolution.

Cargo.toml

@@ -6,7 +6,7 @@ members = [
 
 [package]
 name = "rhai"
-version = "0.19.11"
+version = "0.19.12"
 edition = "2018"
 authors = ["Jonathan Turner", "Lukáš Hozda", "Stephen Chung", "jhwgh1968"]
 description = "Embedded scripting for Rust"
@@ -20,7 +20,7 @@ include = [
     "**/*.md",
     "Cargo.toml"
 ]
-keywords = [ "scripting", "scripting-engine", "scripting language", "embedded" ]
+keywords = [ "scripting", "scripting-engine", "scripting-language", "embedded" ]
 categories = [ "no-std", "embedded", "wasm", "parser-implementations" ]
 
 [dependencies]

RELEASES.md

@@ -1,6 +1,20 @@
 Rhai Release Notes
 ==================
 
+Version 0.19.12
+===============
+
+Breaking changes
+----------------
+
+* `Dynamic::into_shared` is no longer available under `no_closure`. It used to panic.
+
+Enhancements
+------------
+
+* Functions resolution cache is used in more cases, making repeated function calls faster.
+
+
 Version 0.19.11
 ===============
 

src/ast.rs

@@ -1305,7 +1305,7 @@ impl Expr {
             _ => return None,
         })
     }
-    /// Is the expression a simple variable access?
+    /// Return the variable name if the expression a simple variable access.
     #[inline(always)]
     pub(crate) fn get_variable_access(&self, non_qualified: bool) -> Option<&str> {
         match self {

src/dynamic.rs

@@ -761,6 +761,8 @@ impl Dynamic {
     /// [`Arc`][std::sync::Arc]`<`[`RwLock`][std::sync::RwLock]`<`[`Dynamic`]`>>`
     /// depending on the `sync` feature.
     ///
+    /// Not available under [`no_closure`].
+    ///
     /// Shared [`Dynamic`] values are relatively cheap to clone as they simply increment the
     /// reference counts.
     ///
@@ -772,11 +774,11 @@ impl Dynamic {
     /// # Panics
     ///
     /// Panics under the `no_closure` feature.
+    #[cfg(not(feature = "no_closure"))]
     #[inline(always)]
     pub fn into_shared(self) -> Self {
         let _access = self.access_mode();
 
-        #[cfg(not(feature = "no_closure"))]
         return match self.0 {
             Union::Shared(_, _) => self,
             _ => Self(Union::Shared(crate::Locked::new(self).into(), _access)),

src/engine.rs

@@ -520,10 +520,12 @@ pub struct State {
     #[cfg(not(feature = "no_module"))]
     pub resolver: Option<Shared<crate::module::resolvers::StaticModuleResolver>>,
     /// Cached lookup values for function hashes.
-    pub functions_cache: HashMap<
+    pub functions_caches: StaticVec<
-        NonZeroU64,
+        HashMap<
-        Option<(CallableFunction, Option<ImmutableString>)>,
+            NonZeroU64,
-        StraightHasherBuilder,
+            Option<(CallableFunction, Option<ImmutableString>)>,
+            StraightHasherBuilder,
+        >,
     >,
 }
 
@@ -1856,6 +1858,7 @@ impl Engine {
         statements: impl IntoIterator<Item = &'a Stmt>,
         level: usize,
     ) -> Result<Dynamic, Box<EvalAltResult>> {
+        let mut has_imports = false;
         let prev_always_search = state.always_search;
         let prev_scope_len = scope.len();
         let prev_mods_len = mods.len();
@@ -1864,13 +1867,27 @@ impl Engine {
         let result = statements
             .into_iter()
             .try_fold(Default::default(), |_, stmt| {
+                match stmt {
+                    #[cfg(not(feature = "no_module"))]
+                    Stmt::Import(_, _, _) => {
+                        // When imports list is modified, clear the functions lookup cache
+                        if has_imports {
+                            state.functions_caches.last_mut().map(|c| c.clear());
+                        } else {
+                            state.functions_caches.push(Default::default());
+                        }
+                        has_imports = true;
+                    }
+                    _ => (),
+                }
+
                 self.eval_stmt(scope, mods, state, lib, this_ptr, stmt, level)
             });
 
         scope.rewind(prev_scope_len);
-        if mods.len() != prev_mods_len {
+        if has_imports {
-            // If imports list is modified, clear the functions lookup cache
+            // If imports list is modified, pop the functions lookup cache
-            state.functions_cache.clear();
+            state.functions_caches.pop();
         }
         mods.truncate(prev_mods_len);
         state.scope_level -= 1;
@@ -2365,8 +2382,6 @@ impl Engine {
                         } else {
                             mods.push(name_def.name.clone(), module);
                         }
-                        // When imports list is modified, clear the functions lookup cache
-                        state.functions_cache.clear();
                     }
 
                     state.modules += 1;

src/fn_call.rs

@@ -176,28 +176,37 @@ impl Engine {
         self.inc_operations(state, pos)?;
 
         // Check if function access already in the cache
-        let func = &*state.functions_cache.entry(hash_fn).or_insert_with(|| {
+        if state.functions_caches.is_empty() {
-            // Search for the native function
+            state.functions_caches.push(Default::default());
-            // First search registered functions (can override packages)
+        }
-            // Then search packages
-            // Finally search modules
 
-            //lib.get_fn(hash_fn, pub_only)
+        let func = &*state
-            self.global_namespace
+            .functions_caches
-                .get_fn(hash_fn, pub_only)
+            .last_mut()
-                .cloned()
+            .unwrap()
-                .map(|f| (f, None))
+            .entry(hash_fn)
-                .or_else(|| {
+            .or_insert_with(|| {
-                    self.global_modules.iter().find_map(|m| {
+                // Search for the native function
-                        m.get_fn(hash_fn, false)
+                // First search registered functions (can override packages)
-                            .map(|f| (f.clone(), m.id_raw().cloned()))
+                // Then search packages
+                // Finally search modules
+
+                //lib.get_fn(hash_fn, pub_only)
+                self.global_namespace
+                    .get_fn(hash_fn, pub_only)
+                    .cloned()
+                    .map(|f| (f, None))
+                    .or_else(|| {
+                        self.global_modules.iter().find_map(|m| {
+                            m.get_fn(hash_fn, false)
+                                .map(|f| (f.clone(), m.id_raw().cloned()))
+                        })
                     })
-                })
+                    .or_else(|| {
-                .or_else(|| {
+                        mods.get_fn(hash_fn)
-                    mods.get_fn(hash_fn)
+                            .map(|(f, source)| (f.clone(), source.cloned()))
-                        .map(|(f, source)| (f.clone(), source.cloned()))
+                    })
-                })
+            });
-        });
 
         if let Some((func, source)) = func {
             assert!(func.is_native());
@@ -392,11 +401,11 @@ impl Engine {
 
         // Merge in encapsulated environment, if any
         let mut lib_merged: StaticVec<_>;
+        let mut unified = false;
 
         let unified_lib = if let Some(ref env_lib) = fn_def.lib {
-            // If the library is modified, clear the functions lookup cache
+            unified = true;
-            state.functions_cache.clear();
+            state.functions_caches.push(Default::default());
-
             lib_merged = Default::default();
             lib_merged.push(env_lib.as_ref());
             lib_merged.extend(lib.iter().cloned());
@@ -467,6 +476,10 @@ impl Engine {
         mods.truncate(prev_mods_len);
         state.scope_level = orig_scope_level;
 
+        if unified {
+            state.functions_caches.pop();
+        }
+
         result
     }
 
@@ -475,6 +488,7 @@ impl Engine {
     pub(crate) fn has_override_by_name_and_arguments(
         &self,
         mods: Option<&Imports>,
+        state: Option<&mut State>,
         lib: &[&Module],
         fn_name: &str,
         arg_types: impl AsRef<[TypeId]>,
@@ -484,7 +498,7 @@ impl Engine {
         let hash_fn = calc_native_fn_hash(empty(), fn_name, arg_types.iter().cloned());
         let hash_script = calc_script_fn_hash(empty(), fn_name, arg_types.len());
 
-        self.has_override(mods, lib, hash_fn, hash_script, pub_only)
+        self.has_override(mods, state, lib, hash_fn, hash_script, pub_only)
     }
 
     // Has a system function an override?
@@ -492,23 +506,68 @@ impl Engine {
     pub(crate) fn has_override(
         &self,
         mods: Option<&Imports>,
+        mut state: Option<&mut State>,
         lib: &[&Module],
         hash_fn: Option<NonZeroU64>,
         hash_script: Option<NonZeroU64>,
         pub_only: bool,
     ) -> bool {
+        // Check if it is already in the cache
+        if let Some(state) = state.as_mut() {
+            if let Some(hash) = hash_script {
+                match state.functions_caches.last().map_or(None, |c| c.get(&hash)) {
+                    Some(v) => return v.is_some(),
+                    None => (),
+                }
+            }
+            if let Some(hash) = hash_fn {
+                match state.functions_caches.last().map_or(None, |c| c.get(&hash)) {
+                    Some(v) => return v.is_some(),
+                    None => (),
+                }
+            }
+        }
+
         // First check script-defined functions
-        hash_script.map(|hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))).unwrap_or(false)
+        let r = hash_script.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only)))
-            //|| hash_fn.map(|hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only))).unwrap_or(false)
+            //|| hash_fn.map_or(false, |hash| lib.iter().any(|&m| m.contains_fn(hash, pub_only)))
             // Then check registered functions
-            //|| hash_script.map(|hash| self.global_namespace.contains_fn(hash, pub_only)).unwrap_or(false)
+            //|| hash_script.map_or(false, |hash| self.global_namespace.contains_fn(hash, pub_only))
-            || hash_fn.map(|hash| self.global_namespace.contains_fn(hash, false)).unwrap_or(false)
+            || hash_fn.map_or(false, |hash| self.global_namespace.contains_fn(hash, false))
             // Then check packages
-            || hash_script.map(|hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))).unwrap_or(false)
+            || hash_script.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false)))
-            || hash_fn.map(|hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false))).unwrap_or(false)
+            || hash_fn.map_or(false, |hash| self.global_modules.iter().any(|m| m.contains_fn(hash, false)))
             // Then check imported modules
-            || hash_script.map(|hash| mods.map(|m| m.contains_fn(hash)).unwrap_or(false)).unwrap_or(false)
+            || hash_script.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash)))
-            || hash_fn.map(|hash| mods.map(|m| m.contains_fn(hash)).unwrap_or(false)).unwrap_or(false)
+            || hash_fn.map_or(false, |hash| mods.map_or(false, |m| m.contains_fn(hash)));
+
+        // If there is no override, put that information into the cache
+        if !r {
+            if let Some(state) = state.as_mut() {
+                if let Some(hash) = hash_script {
+                    if state.functions_caches.is_empty() {
+                        state.functions_caches.push(Default::default());
+                    }
+                    state
+                        .functions_caches
+                        .last_mut()
+                        .unwrap()
+                        .insert(hash, None);
+                }
+                if let Some(hash) = hash_fn {
+                    if state.functions_caches.is_empty() {
+                        state.functions_caches.push(Default::default());
+                    }
+                    state
+                        .functions_caches
+                        .last_mut()
+                        .unwrap()
+                        .insert(hash, None);
+                }
+            }
+        }
+
+        r
     }
 
     /// Perform an actual function call, native Rust or scripted, taking care of special functions.
@@ -547,7 +606,14 @@ impl Engine {
             // type_of
             KEYWORD_TYPE_OF
                 if args.len() == 1
-                    && !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) =>
+                    && !self.has_override(
+                        Some(mods),
+                        Some(state),
+                        lib,
+                        hash_fn,
+                        hash_script,
+                        pub_only,
+                    ) =>
             {
                 Ok((
                     self.map_type_name(args[0].type_name()).to_string().into(),
@@ -559,7 +625,14 @@ impl Engine {
             //           by a function pointer so it isn't caught at parse time.
             KEYWORD_FN_PTR | KEYWORD_EVAL
                 if args.len() == 1
-                    && !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) =>
+                    && !self.has_override(
+                        Some(mods),
+                        Some(state),
+                        lib,
+                        hash_fn,
+                        hash_script,
+                        pub_only,
+                    ) =>
             {
                 EvalAltResult::ErrorRuntime(
                     format!(
@@ -575,25 +648,37 @@ impl Engine {
             // Script-like function found
             #[cfg(not(feature = "no_function"))]
             _ if hash_script.is_some()
-                && self.has_override(Some(mods), lib, None, hash_script, pub_only) =>
+                && self.has_override(Some(mods), Some(state), lib, None, hash_script, pub_only) =>
             {
                 let hash_script = hash_script.unwrap();
 
-                // Get function
+                // Check if function access already in the cache
-                let (func, mut source) = lib
+                if state.functions_caches.is_empty() {
-                    .iter()
+                    state.functions_caches.push(Default::default());
-                    .find_map(|&m| {
+                }
-                        m.get_fn(hash_script, pub_only)
+
-                            .map(|f| (f, m.id_raw().cloned()))
+                let (func, source) = state
+                    .functions_caches
+                    .last_mut()
+                    .unwrap()
+                    .entry(hash_script)
+                    .or_insert_with(|| {
+                        lib.iter()
+                            .find_map(|&m| {
+                                m.get_fn(hash_script, pub_only)
+                                    .map(|f| (f.clone(), m.id_raw().cloned()))
+                            })
+                            //.or_else(|| self.global_namespace.get_fn(hash_script, pub_only))
+                            .or_else(|| {
+                                self.global_modules.iter().find_map(|m| {
+                                    m.get_fn(hash_script, false)
+                                        .map(|f| (f.clone(), m.id_raw().cloned()))
+                                })
+                            })
+                        //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.id_raw().clone()))))
                     })
-                    //.or_else(|| self.global_namespace.get_fn(hash_script, pub_only))
+                    .as_ref()
-                    .or_else(|| {
+                    .map(|(f, s)| (f.clone(), s.clone()))
-                        self.global_modules.iter().find_map(|m| {
-                            m.get_fn(hash_script, false)
-                                .map(|f| (f, m.id_raw().cloned()))
-                        })
-                    })
-                    //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.id_raw().clone()))))
                     .unwrap();
 
                 assert!(func.is_script());
@@ -620,7 +705,8 @@ impl Engine {
                     // Method call of script function - map first argument to `this`
                     let (first, rest) = args.split_first_mut().unwrap();
 
-                    mem::swap(&mut state.source, &mut source);
+                    let orig_source = mem::take(&mut state.source);
+                    state.source = source;
 
                     let level = _level + 1;
 
@@ -637,7 +723,7 @@ impl Engine {
                     );
 
                     // Restore the original source
-                    state.source = source;
+                    state.source = orig_source;
 
                     result?
                 } else {
@@ -646,7 +732,8 @@ impl Engine {
                     let mut backup: ArgBackup = Default::default();
                     backup.change_first_arg_to_copy(is_ref, args);
 
-                    mem::swap(&mut state.source, &mut source);
+                    let orig_source = mem::take(&mut state.source);
+                    state.source = source;
 
                     let level = _level + 1;
 
@@ -654,7 +741,7 @@ impl Engine {
                         .call_script_fn(scope, mods, state, lib, &mut None, func, args, pos, level);
 
                     // Restore the original source
-                    state.source = source;
+                    state.source = orig_source;
 
                     // Restore the original reference
                     backup.restore_first_arg(args);
@@ -914,59 +1001,6 @@ impl Engine {
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         let args_expr = args_expr.as_ref();
 
-        // Handle Fn()
-        if fn_name == KEYWORD_FN_PTR && args_expr.len() == 1 {
-            let hash_fn =
-                calc_native_fn_hash(empty(), fn_name, once(TypeId::of::<ImmutableString>()));
-
-            if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) {
-                // Fn - only in function call style
-                return self
-                    .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?
-                    .take_immutable_string()
-                    .map_err(|typ| {
-                        self.make_type_mismatch_err::<ImmutableString>(typ, args_expr[0].position())
-                    })
-                    .and_then(|s| FnPtr::try_from(s))
-                    .map(Into::<Dynamic>::into)
-                    .map_err(|err| err.fill_position(args_expr[0].position()));
-            }
-        }
-
-        // Handle curry()
-        if fn_name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 {
-            let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
-
-            if !fn_ptr.is::<FnPtr>() {
-                return Err(self.make_type_mismatch_err::<FnPtr>(
-                    self.map_type_name(fn_ptr.type_name()),
-                    args_expr[0].position(),
-                ));
-            }
-
-            let (fn_name, mut fn_curry) = fn_ptr.cast::<FnPtr>().take_data();
-
-            // Append the new curried arguments to the existing list.
-
-            args_expr
-                .iter()
-                .skip(1)
-                .try_for_each(|expr| -> Result<(), Box<EvalAltResult>> {
-                    fn_curry.push(self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?);
-                    Ok(())
-                })?;
-
-            return Ok(FnPtr::new_unchecked(fn_name, fn_curry).into());
-        }
-
-        // Handle is_shared()
-        #[cfg(not(feature = "no_closure"))]
-        if fn_name == crate::engine::KEYWORD_IS_SHARED && args_expr.len() == 1 {
-            let value = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
-
-            return Ok(value.is_shared().into());
-        }
-
         // Handle call() - Redirect function call
         let redirected;
         let mut args_expr = args_expr.as_ref();
@@ -975,7 +1009,7 @@ impl Engine {
 
         if name == KEYWORD_FN_PTR_CALL
             && args_expr.len() >= 1
-            && !self.has_override(Some(mods), lib, None, hash_script, pub_only)
+            && !self.has_override(Some(mods), Some(state), lib, None, hash_script, pub_only)
         {
             let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
 
@@ -1001,11 +1035,63 @@ impl Engine {
             hash_script = calc_script_fn_hash(empty(), name, args_len);
         }
 
+        // Handle Fn()
+        if name == KEYWORD_FN_PTR && args_expr.len() == 1 {
+            let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::<ImmutableString>()));
+
+            if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) {
+                // Fn - only in function call style
+                return self
+                    .eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?
+                    .take_immutable_string()
+                    .map_err(|typ| {
+                        self.make_type_mismatch_err::<ImmutableString>(typ, args_expr[0].position())
+                    })
+                    .and_then(|s| FnPtr::try_from(s))
+                    .map(Into::<Dynamic>::into)
+                    .map_err(|err| err.fill_position(args_expr[0].position()));
+            }
+        }
+
+        // Handle curry()
+        if name == KEYWORD_FN_PTR_CURRY && args_expr.len() > 1 {
+            let fn_ptr = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
+
+            if !fn_ptr.is::<FnPtr>() {
+                return Err(self.make_type_mismatch_err::<FnPtr>(
+                    self.map_type_name(fn_ptr.type_name()),
+                    args_expr[0].position(),
+                ));
+            }
+
+            let (name, mut fn_curry) = fn_ptr.cast::<FnPtr>().take_data();
+
+            // Append the new curried arguments to the existing list.
+
+            args_expr
+                .iter()
+                .skip(1)
+                .try_for_each(|expr| -> Result<(), Box<EvalAltResult>> {
+                    fn_curry.push(self.eval_expr(scope, mods, state, lib, this_ptr, expr, level)?);
+                    Ok(())
+                })?;
+
+            return Ok(FnPtr::new_unchecked(name, fn_curry).into());
+        }
+
+        // Handle is_shared()
+        #[cfg(not(feature = "no_closure"))]
+        if name == crate::engine::KEYWORD_IS_SHARED && args_expr.len() == 1 {
+            let value = self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
+
+            return Ok(value.is_shared().into());
+        }
+
         // Handle is_def_var()
         if name == KEYWORD_IS_DEF_VAR && args_expr.len() == 1 {
             let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::<ImmutableString>()));
 
-            if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) {
+            if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) {
                 let var_name =
                     self.eval_expr(scope, mods, state, lib, this_ptr, &args_expr[0], level)?;
                 let var_name = var_name.as_str().map_err(|err| {
@@ -1019,7 +1105,7 @@ impl Engine {
         if name == KEYWORD_EVAL && args_expr.len() == 1 {
             let hash_fn = calc_native_fn_hash(empty(), name, once(TypeId::of::<ImmutableString>()));
 
-            if !self.has_override(Some(mods), lib, hash_fn, hash_script, pub_only) {
+            if !self.has_override(Some(mods), Some(state), lib, hash_fn, hash_script, pub_only) {
                 // eval - only in function call style
                 let prev_len = scope.len();
                 let script =
@@ -1055,8 +1141,9 @@ impl Engine {
             // No arguments
             args = Default::default();
         } else {
-            // If the first argument is a variable, and there is no curried arguments, convert to method-call style
+            // If the first argument is a variable, and there is no curried arguments,
-            // in order to leverage potential &mut first argument and avoid cloning the value
+            // convert to method-call style in order to leverage potential &mut first argument and
+            // avoid cloning the value
             if curry.is_empty() && args_expr[0].get_variable_access(false).is_some() {
                 // func(x, ...) -> x.func(...)
                 arg_values = args_expr

src/module/mod.rs

@@ -619,8 +619,7 @@ impl Module {
         if public_only {
             self.functions
                 .get(&hash_fn)
-                .map(|FuncInfo { access, .. }| access.is_public())
+                .map_or(false, |FuncInfo { access, .. }| access.is_public())
-                .unwrap_or(false)
         } else {
             self.functions.contains_key(&hash_fn)
         }

src/optimize.rs

@@ -674,7 +674,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) {
             let arg_types: StaticVec<_> = arg_values.iter().map(Dynamic::type_id).collect();
 
             // Search for overloaded operators (can override built-in).
-            if !state.engine.has_override_by_name_and_arguments(Some(&state.mods), state.lib, x.name.as_ref(), arg_types.as_ref(), false) {
+            if !state.engine.has_override_by_name_and_arguments(Some(&state.mods), None, state.lib, x.name.as_ref(), arg_types.as_ref(), false) {
                 if let Some(result) = run_builtin_binary_op(x.name.as_ref(), &arg_values[0], &arg_values[1])
                                         .ok().flatten()
                                         .and_then(|result| map_dynamic_to_expr(result, *pos))

src/packages/fn_basic.rs

@@ -31,8 +31,9 @@ mod fn_ptr_functions {
                 false
             } else {
                 let hash_script = calc_script_fn_hash(empty(), fn_name, num_params as usize);
+
                 ctx.engine()
-                    .has_override(ctx.mods, ctx.lib, None, hash_script, true)
+                    .has_override(ctx.mods, None, ctx.lib, None, hash_script, true)
             }
         }
     }

src/parser.rs

@@ -1036,9 +1036,9 @@ fn parse_primary(
             match input.peek().unwrap().0 {
                 // Function call
                 Token::LeftParen | Token::Bang => {
-                    // Once the identifier consumed we must enable next variables capturing
                     #[cfg(not(feature = "no_closure"))]
                     {
+                        // Once the identifier consumed we must enable next variables capturing
                         state.allow_capture = true;
                     }
                     let var_name_def = Ident {
@@ -1050,9 +1050,9 @@ fn parse_primary(
                 // Namespace qualification
                 #[cfg(not(feature = "no_module"))]
                 Token::DoubleColon => {
-                    // Once the identifier consumed we must enable next variables capturing
                     #[cfg(not(feature = "no_closure"))]
                     {
+                        // Once the identifier consumed we must enable next variables capturing
                         state.allow_capture = true;
                     }
                     let var_name_def = Ident {
@@ -1082,36 +1082,30 @@ fn parse_primary(
 
             match input.peek().unwrap().0 {
                 // Function call is allowed to have reserved keyword
-                Token::LeftParen | Token::Bang => {
+                Token::LeftParen | Token::Bang if is_keyword_function(&s) => {
-                    if is_keyword_function(&s) {
+                    let var_name_def = Ident {
-                        let var_name_def = Ident {
+                        name: state.get_interned_string(s),
-                            name: state.get_interned_string(s),
+                        pos: settings.pos,
-                            pos: settings.pos,
+                    };
-                        };
+                    Expr::Variable(Box::new((None, None, var_name_def)))
-                        Expr::Variable(Box::new((None, None, var_name_def)))
-                    } else {
-                        return Err(PERR::Reserved(s).into_err(settings.pos));
-                    }
                 }
-                // Access to `this` as a variable is OK
+                // Access to `this` as a variable is OK within a function scope
+                _ if s == KEYWORD_THIS && settings.is_function_scope => {
+                    let var_name_def = Ident {
+                        name: state.get_interned_string(s),
+                        pos: settings.pos,
+                    };
+                    Expr::Variable(Box::new((None, None, var_name_def)))
+                }
+                // Cannot access to `this` as a variable not in a function scope
                 _ if s == KEYWORD_THIS => {
-                    if !settings.is_function_scope {
+                    let msg = format!("'{}' can only be used in functions", s);
-                        let msg = format!("'{}' can only be used in functions", s);
+                    return Err(LexError::ImproperSymbol(s, msg).into_err(settings.pos));
-                        return Err(LexError::ImproperSymbol(s, msg).into_err(settings.pos));
-                    } else {
-                        let var_name_def = Ident {
-                            name: state.get_interned_string(s),
-                            pos: settings.pos,
-                        };
-                        Expr::Variable(Box::new((None, None, var_name_def)))
-                    }
                 }
                 _ if is_valid_identifier(s.chars()) => {
-                    return Err(PERR::Reserved(s).into_err(settings.pos));
+                    return Err(PERR::Reserved(s).into_err(settings.pos))
-                }
-                _ => {
-                    return Err(LexError::UnexpectedInput(s).into_err(settings.pos));
                 }
+                _ => return Err(LexError::UnexpectedInput(s).into_err(settings.pos)),
             }
         }
 
@@ -1125,9 +1119,7 @@ fn parse_primary(
         }
 
         _ => {
-            return Err(
+            return Err(LexError::UnexpectedInput(token.syntax().to_string()).into_err(settings.pos))
-                LexError::UnexpectedInput(token.syntax().to_string()).into_err(settings.pos)
-            );
         }
     };
 
@@ -1739,8 +1731,7 @@ fn parse_binary_op(
                     .engine
                     .custom_keywords
                     .get(c)
-                    .map(Option::is_some)
+                    .map_or(false, Option::is_some)
-                    .unwrap_or(false)
                 {
                     state.engine.custom_keywords.get(c).unwrap().unwrap().get()
                 } else {
@@ -1771,8 +1762,7 @@ fn parse_binary_op(
                     .engine
                     .custom_keywords
                     .get(c)
-                    .map(Option::is_some)
+                    .map_or(false, Option::is_some)
-                    .unwrap_or(false)
                 {
                     state.engine.custom_keywords.get(c).unwrap().unwrap().get()
                 } else {
@@ -1883,8 +1873,7 @@ fn parse_binary_op(
                     .engine
                     .custom_keywords
                     .get(&s)
-                    .map(Option::is_some)
+                    .map_or(false, Option::is_some) =>
-                    .unwrap_or(false) =>
             {
                 let hash_script = if is_valid_identifier(s.chars()) {
                     // Accept non-native functions for custom operators
@@ -2275,6 +2264,12 @@ fn parse_let(
         (_, pos) => return Err(PERR::VariableExpected.into_err(pos)),
     };
 
+    let name = state.get_interned_string(name);
+    let var_def = Ident {
+        name: name.clone(),
+        pos,
+    };
+
     // let name = ...
     let expr = if match_token(input, Token::Equals).0 {
         // let name = expr
@@ -2283,21 +2278,13 @@ fn parse_let(
         None
     };
 
+    state.stack.push((name, var_type));
+
     match var_type {
         // let name = expr
-        AccessMode::ReadWrite => {
+        AccessMode::ReadWrite => Ok(Stmt::Let(Box::new(var_def), expr, export, settings.pos)),
-            let name = state.get_interned_string(name);
-            state.stack.push((name.clone(), AccessMode::ReadWrite));
-            let var_def = Ident { name, pos };
-            Ok(Stmt::Let(Box::new(var_def), expr, export, settings.pos))
-        }
         // const name = { expr:constant }
-        AccessMode::ReadOnly => {
+        AccessMode::ReadOnly => Ok(Stmt::Const(Box::new(var_def), expr, export, settings.pos)),
-            let name = state.get_interned_string(name);
-            state.stack.push((name.clone(), AccessMode::ReadOnly));
-            let var_def = Ident { name, pos };
-            Ok(Stmt::Const(Box::new(var_def), expr, export, settings.pos))
-        }
     }
 }
 

src/syntax.rs

@@ -149,8 +149,7 @@ impl Engine {
                 s if segments.is_empty()
                     && token
                         .as_ref()
-                        .map(|v| v.is_keyword() || v.is_reserved())
+                        .map_or(false, |v| v.is_keyword() || v.is_reserved()) =>
-                        .unwrap_or(false) =>
                 {
                     return Err(LexError::ImproperSymbol(
                         s.to_string(),