From ba7f8c63912fa711b1f1ff57db756690e01e5d1b Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 30 Dec 2020 21:12:51 +0800 Subject: [PATCH] Propagate source info. --- RELEASES.md | 14 +++ src/bin/rhai-repl.rs | 4 +- src/engine.rs | 6 +- src/fn_call.rs | 209 ++++++++++++++++++++---------------- src/packages/array_basic.rs | 13 +++ src/result.rs | 36 ++++--- src/utils.rs | 6 ++ tests/fn_ptr.rs | 2 +- tests/modules.rs | 2 +- 9 files changed, 180 insertions(+), 112 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index bd6357b2..422a5ef5 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,6 +1,20 @@ Rhai Release Notes ================== +Version 0.19.10 +=============== + +Breaking changes +---------------- + +* The error variant `EvalAltResult::ErrorInFunctionCall` has a new parameter holding the _source_ of the function. + +Enhancements +------------ + +* Source information is provided when there is an error within a call to a function defined in another module. + + Version 0.19.9 ============== diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 54565405..13b968ef 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -89,7 +89,7 @@ fn main() { } } - let module = match engine + let mut module = match engine .compile(&contents) .map_err(|err| err.into()) .and_then(|ast| Module::eval_ast_as_new(Default::default(), &ast, &engine)) @@ -106,6 +106,8 @@ fn main() { Ok(m) => m, }; + module.set_id(Some(&filename)); + engine.register_global_module(module.into()); has_init_scripts = true; diff --git a/src/engine.rs b/src/engine.rs index bf36d028..1ae2d077 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -508,7 +508,11 @@ pub struct State { /// Number of modules loaded. pub modules: usize, /// Cached lookup values for function hashes. - pub functions_cache: HashMap, StraightHasherBuilder>, + pub functions_cache: HashMap< + NonZeroU64, + Option<(CallableFunction, Option)>, + StraightHasherBuilder, + >, } impl State { diff --git a/src/fn_call.rs b/src/fn_call.rs index 38bdeff6..0788dde7 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -175,11 +175,8 @@ impl Engine { ) -> Result<(Dynamic, bool), Box> { self.inc_operations(state, pos)?; - let func = state.functions_cache.get(&hash_fn).cloned(); - - let func = if let Some(ref f) = func { - f.as_ref() - } else { + // Check if function access already in the cache + if !state.functions_cache.contains_key(&hash_fn) { // Search for the native function // First search registered functions (can override packages) // Then search packages @@ -189,18 +186,24 @@ impl Engine { let f = 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)) + self.global_modules.iter().find_map(|m| { + m.get_fn(hash_fn, false) + .cloned() + .map(|f| (f, m.id_raw().clone())) + }) }) - .or_else(|| mods.get_fn(hash_fn)); + .or_else(|| mods.get_fn(hash_fn).cloned().map(|f| (f, None))); - state.functions_cache.insert(hash_fn, f.cloned()); - f - }; + // Store into cache + state.functions_cache.insert(hash_fn, f); + } - if let Some(func) = func { + let func = state.functions_cache.get(&hash_fn).unwrap(); + + if let Some((func, source)) = func { assert!(func.is_native()); // Calling pure function but the first argument is a reference? @@ -208,11 +211,16 @@ impl Engine { backup.change_first_arg_to_copy(is_ref && func.is_pure(), args); // Run external function + let source = if source.is_none() { + &state.source + } else { + source + }; let result = if func.is_plugin_fn() { func.get_plugin_fn() - .call((self, &state.source, mods, lib).into(), args) + .call((self, source, mods, lib).into(), args) } else { - func.get_native_fn()((self, &state.source, mods, lib).into(), args) + func.get_native_fn()((self, source, mods, lib).into(), args) }; // Restore the original reference @@ -413,9 +421,26 @@ impl Engine { .or_else(|err| match *err { // Convert return statement to return value EvalAltResult::Return(x, _) => Ok(x), - EvalAltResult::ErrorInFunctionCall(name, err, _) => { + EvalAltResult::ErrorInFunctionCall(name, src, err, _) => { EvalAltResult::ErrorInFunctionCall( - format!("{} > {}", fn_def.name, name), + format!( + "{}{} < {}", + name, + if src.is_empty() { + "".to_string() + } else { + format!(" @ '{}'", src) + }, + fn_def.name + ), + fn_def + .lib + .as_ref() + .map(|m| m.id()) + .flatten() + .or_else(|| state.source.as_ref().map(|s| s.as_str())) + .unwrap_or("") + .to_string(), err, pos, ) @@ -424,7 +449,20 @@ impl Engine { // System errors are passed straight-through err if err.is_system_exception() => Err(Box::new(err)), // Other errors are wrapped in `ErrorInFunctionCall` - _ => EvalAltResult::ErrorInFunctionCall(fn_def.name.to_string(), err, pos).into(), + _ => EvalAltResult::ErrorInFunctionCall( + fn_def.name.to_string(), + fn_def + .lib + .as_ref() + .map(|m| m.id()) + .flatten() + .or_else(|| state.source.as_ref().map(|s| s.as_str())) + .unwrap_or("") + .to_string(), + err, + pos, + ) + .into(), }); // Remove all local variables @@ -553,96 +591,81 @@ impl Engine { }) //.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, None)) + self.global_modules.iter().find_map(|m| { + m.get_fn(hash_script, false) + .map(|f| (f, m.id_raw().clone())) + }) }) //.or_else(|| mods.iter().find_map(|(_, m)| m.get_qualified_fn(hash_script).map(|f| (f, m.id_raw().clone())))) .unwrap(); - if func.is_script() { - let func = func.get_fn_def(); + assert!(func.is_script()); - let scope: &mut Scope = &mut Default::default(); + let func = func.get_fn_def(); - // Move captured variables into scope - #[cfg(not(feature = "no_closure"))] - if let Some(captured) = _capture_scope { - if !func.externals.is_empty() { - captured - .into_iter() - .filter(|(name, _, _)| func.externals.iter().any(|ex| ex == name)) - .for_each(|(name, value, _)| { - // Consume the scope values. - scope.push_dynamic(name, value); - }); - } + let scope: &mut Scope = &mut Default::default(); + + // Move captured variables into scope + #[cfg(not(feature = "no_closure"))] + if let Some(captured) = _capture_scope { + if !func.externals.is_empty() { + captured + .into_iter() + .filter(|(name, _, _)| func.externals.iter().any(|ex| ex == name)) + .for_each(|(name, value, _)| { + // Consume the scope values. + scope.push_dynamic(name, value); + }); } + } - let result = if _is_method { - // Method call of script function - map first argument to `this` - let (first, rest) = args.split_first_mut().unwrap(); + let result = if _is_method { + // 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); + mem::swap(&mut state.source, &mut source); - let level = _level + 1; + let level = _level + 1; - let result = self.call_script_fn( - scope, - mods, - state, - lib, - &mut Some(*first), - func, - rest, - pos, - level, - ); - - // Restore the original source - state.source = source; - - result? - } else { - // Normal call of script function - // The first argument is a reference? - let mut backup: ArgBackup = Default::default(); - backup.change_first_arg_to_copy(is_ref, args); - - mem::swap(&mut state.source, &mut source); - - let level = _level + 1; - - let result = self.call_script_fn( - scope, mods, state, lib, &mut None, func, args, pos, level, - ); - - // Restore the original source - state.source = source; - - // Restore the original reference - backup.restore_first_arg(args); - - result? - }; - - Ok((result, false)) - } else { - // If it is a native function, redirect it - self.call_native_fn( + let result = self.call_script_fn( + scope, mods, state, lib, - fn_name, - hash_script, - args, - is_ref, - pub_only, + &mut Some(*first), + func, + rest, pos, - def_val, - ) - } + level, + ); + + // Restore the original source + state.source = source; + + result? + } else { + // Normal call of script function + // The first argument is a reference? + let mut backup: ArgBackup = Default::default(); + backup.change_first_arg_to_copy(is_ref, args); + + mem::swap(&mut state.source, &mut source); + + let level = _level + 1; + + let result = self + .call_script_fn(scope, mods, state, lib, &mut None, func, args, pos, level); + + // Restore the original source + state.source = source; + + // Restore the original reference + backup.restore_first_arg(args); + + result? + }; + + Ok((result, false)) } // Normal native function call diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 4ab427db..6247e881 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -214,6 +214,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "map".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -245,6 +246,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "filter".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -278,6 +280,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "index_of".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -311,6 +314,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "some".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -344,6 +348,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "all".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -379,6 +384,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -397,6 +403,7 @@ mod array_functions { let mut result = initial.call_dynamic(ctx, None, []).map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -416,6 +423,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -446,6 +454,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce_rev".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -464,6 +473,7 @@ mod array_functions { let mut result = initial.call_dynamic(ctx, None, []).map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce_rev".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -483,6 +493,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "reduce_rev".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -553,6 +564,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "drain".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) @@ -612,6 +624,7 @@ mod array_functions { .map_err(|err| { Box::new(EvalAltResult::ErrorInFunctionCall( "retain".to_string(), + ctx.source().unwrap_or("").to_string(), err, Position::NONE, )) diff --git a/src/result.rs b/src/result.rs index afc45dec..0face85e 100644 --- a/src/result.rs +++ b/src/result.rs @@ -34,8 +34,8 @@ pub enum EvalAltResult { /// Call to an unknown function. Wrapped value is the function signature. ErrorFunctionNotFound(String, Position), /// An error has occurred inside a called function. - /// Wrapped values are the function name and the interior error. - ErrorInFunctionCall(String, Box, Position), + /// Wrapped values are the function name, function source, and the interior error. + ErrorInFunctionCall(String, String, Box, Position), /// Usage of an unknown [module][crate::Module]. Wrapped value is the [module][crate::Module] name. ErrorModuleNotFound(String, Position), /// An error has occurred while loading a [module][crate::Module]. @@ -98,7 +98,7 @@ impl EvalAltResult { #[allow(deprecated)] Self::ErrorSystem(_, s) => s.description(), Self::ErrorParsing(p, _) => p.desc(), - Self::ErrorInFunctionCall(_, _, _) => "Error in called function", + Self::ErrorInFunctionCall(_,_, _, _) => "Error in called function", Self::ErrorInModule(_, _, _) => "Error in module", Self::ErrorFunctionNotFound(_, _) => "Function not found", Self::ErrorUnboundThis(_) => "'this' is not bound", @@ -152,12 +152,19 @@ impl fmt::Display for EvalAltResult { Self::ErrorParsing(p, _) => write!(f, "Syntax error: {}", p)?, #[cfg(not(feature = "no_function"))] - Self::ErrorInFunctionCall(s, err, _) if crate::engine::is_anonymous_fn(s) => { - write!(f, "Error in call to closure: {}", err)? + Self::ErrorInFunctionCall(s, src, err, _) if crate::engine::is_anonymous_fn(s) => { + write!(f, "{}, in call to closure", err)?; + if !src.is_empty() { + write!(f, " @ '{}'", src)?; + } } - Self::ErrorInFunctionCall(s, err, _) => { - write!(f, "Error in call to function '{}': {}", s, err)? + Self::ErrorInFunctionCall(s, src, err, _) => { + write!(f, "{}, in call to function {}", err, s)?; + if !src.is_empty() { + write!(f, " @ '{}'", src)?; + } } + Self::ErrorInModule(s, err, _) if s.is_empty() => { write!(f, "Error in module: {}", err)? } @@ -165,8 +172,9 @@ impl fmt::Display for EvalAltResult { Self::ErrorFunctionNotFound(s, _) | Self::ErrorVariableNotFound(s, _) - | Self::ErrorDataRace(s, _) - | Self::ErrorModuleNotFound(s, _) => write!(f, "{}: '{}'", desc, s)?, + | Self::ErrorDataRace(s, _) => write!(f, "{}: {}", desc, s)?, + + Self::ErrorModuleNotFound(s, _) => write!(f, "{}: '{}'", desc, s)?, Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{}", s)?, @@ -187,9 +195,7 @@ impl fmt::Display for EvalAltResult { Self::ErrorRuntime(d, _) if d.is::<()>() => f.write_str(desc)?, Self::ErrorRuntime(d, _) => write!(f, "{}: {}", desc, d)?, - Self::ErrorAssignmentToConstant(s, _) => { - write!(f, "Cannot assign to constant '{}'", s)? - } + Self::ErrorAssignmentToConstant(s, _) => write!(f, "Cannot assign to constant {}", s)?, Self::ErrorMismatchOutputType(s, r, _) => { write!(f, "Output type is incorrect: {} (expecting {})", r, s)? } @@ -276,7 +282,7 @@ impl EvalAltResult { Self::ErrorParsing(_, _) => false, Self::ErrorFunctionNotFound(_, _) - | Self::ErrorInFunctionCall(_, _, _) + | Self::ErrorInFunctionCall(_, _, _, _) | Self::ErrorInModule(_, _, _) | Self::ErrorUnboundThis(_) | Self::ErrorMismatchDataType(_, _, _) @@ -334,7 +340,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) - | Self::ErrorInFunctionCall(_, _, pos) + | Self::ErrorInFunctionCall(_, _, _, pos) | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorMismatchDataType(_, _, pos) @@ -367,7 +373,7 @@ impl EvalAltResult { Self::ErrorParsing(_, pos) | Self::ErrorFunctionNotFound(_, pos) - | Self::ErrorInFunctionCall(_, _, pos) + | Self::ErrorInFunctionCall(_, _, _, pos) | Self::ErrorInModule(_, _, pos) | Self::ErrorUnboundThis(pos) | Self::ErrorMismatchDataType(_, _, pos) diff --git a/src/utils.rs b/src/utils.rs index 9043bb4a..c43a5aaf 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -212,6 +212,12 @@ impl From<&str> for ImmutableString { Self(value.to_string().into()) } } +impl From<&String> for ImmutableString { + #[inline(always)] + fn from(value: &String) -> Self { + Self(value.to_string().into()) + } +} impl From for ImmutableString { #[inline(always)] fn from(value: String) -> Self { diff --git a/tests/fn_ptr.rs b/tests/fn_ptr.rs index 6a37af62..9f175bd2 100644 --- a/tests/fn_ptr.rs +++ b/tests/fn_ptr.rs @@ -73,7 +73,7 @@ fn test_fn_ptr() -> Result<(), Box> { "# ) .expect_err("should error"), - EvalAltResult::ErrorInFunctionCall(fn_name, err, _) + EvalAltResult::ErrorInFunctionCall(fn_name, _, err, _) if fn_name == "foo" && matches!(*err, EvalAltResult::ErrorUnboundThis(_)) )); diff --git a/tests/modules.rs b/tests/modules.rs index 26c4af8c..eee696fb 100644 --- a/tests/modules.rs +++ b/tests/modules.rs @@ -227,7 +227,7 @@ fn test_module_resolver() -> Result<(), Box> { "# ) .expect_err("should error"), - EvalAltResult::ErrorInFunctionCall(fn_name, _, _) if fn_name == "foo" + EvalAltResult::ErrorInFunctionCall(fn_name, _, _, _) if fn_name == "foo" )); engine.set_max_modules(1000);