Skip to content

Commit 5897aa2

Browse files
committed
Refine new_frame API and add CI test.
1 parent 897a020 commit 5897aa2

5 files changed

Lines changed: 95 additions & 15 deletions

File tree

src/eval/eval_context.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,17 @@ impl<'a, 's, 'ps, 'g, 'c, 't> EvalContext<'a, 's, 'ps, 'g, 'c, 't> {
134134
pub fn namespaces(&self) -> &[crate::SharedModule] {
135135
&self.global.lib
136136
}
137+
/// _(internals)_ Mutable reference to the current set of namespaces containing definitions of all script-defined functions.
138+
/// Exported under the `internals` feature only.
139+
///
140+
/// Not available under `no_function`.
141+
#[cfg(not(feature = "no_function"))]
142+
#[cfg(feature = "internals")]
143+
#[inline(always)]
144+
#[must_use]
145+
pub fn namespaces_mut(&mut self) -> &mut [crate::SharedModule] {
146+
self.global.lib.as_mut()
147+
}
137148
/// The current bound `this` pointer, if any.
138149
#[inline(always)]
139150
#[must_use]
@@ -417,7 +428,7 @@ impl<'a, 's, 'ps, 'g, 'c, 't> EvalContext<'a, 's, 'ps, 'g, 'c, 't> {
417428
/// let context = context.new_frame()
418429
/// .rewind_scope(true) // rewinds the scope...
419430
/// .with_source("new source") // new source...
420-
/// .with_module(new_module) // add new module...
431+
/// .with_namespace(new_module) // add new namespace module...
421432
/// .up_call_level();
422433
///
423434
/// // Call a function with the modified context...
@@ -431,8 +442,8 @@ impl<'a, 's, 'ps, 'g, 'c, 't> EvalContext<'a, 's, 'ps, 'g, 'c, 't> {
431442
///
432443
/// Upon `Drop`, the following fields will be automatically restored to the previous values:
433444
///
434-
/// * the stack of imported [modules][crate::Module] will be rewound to the original depth if more [modules][crate::Module] have been added via [`EvalContextFrameGuard::with_import`].
435-
/// * the stack of global [modules][crate::Module] will be rewound to the original depth if more [modules][crate::Module] have been added via [`EvalContextFrameGuard::with_module`].
445+
/// * the stack of imported [modules][crate::Module] will be rewound to the original depth if more have been added via [`EvalContextFrameGuard::with_import`].
446+
/// * the stack of scripted function [modules][crate::Module] will be rewound to the original depth if more have been added via [`EvalContextFrameGuard::with_namespace`].
436447
/// * the original functions resolution cache will be restored if a new caching layer was created via [`EvalContextFrameGuard::with_new_caching_layer`].
437448
/// * the original [scope][EvalContext::scope] will be rewound if [`EvalContextFrameGuard::rewind_scope`] was set to `true`.
438449
/// * the [source][GlobalRuntimeState::source] will be restored if a new source was set via [`EvalContextFrameGuard::with_source`] or cleared via [`EvalContextFrameGuard::clear_source`].
@@ -658,7 +669,7 @@ impl<'t> EvalContextFrameGuard<'_, '_, '_, '_, '_, '_, 't> {
658669
#[cfg(not(feature = "no_function"))]
659670
#[inline(always)]
660671
#[must_use]
661-
pub fn with_module(mut self, module: impl Into<crate::SharedModule>) -> Self {
672+
pub fn with_namespace(mut self, module: impl Into<crate::SharedModule>) -> Self {
662673
self.lib_len = Some(self.context.global.lib.len());
663674
self.context.global.lib.push(module.into());
664675
self

src/eval/global_state.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ impl GlobalRuntimeState {
152152
module: impl Into<crate::SharedModule>,
153153
) {
154154
self.imports.push(name.into());
155-
156155
self.modules.push(module.into());
157156
}
158157
/// Truncate the stack of globally-imported [modules][crate::Module] to a particular length.
@@ -164,7 +163,8 @@ impl GlobalRuntimeState {
164163
self.imports.truncate(size);
165164
self.modules.truncate(size);
166165
}
167-
/// Get an iterator to the stack of globally-imported [modules][crate::Module] in reverse order.
166+
/// Get an iterator to the stack of globally-imported [modules][crate::Module] in reverse order
167+
/// (i.e. modules imported last come first).
168168
///
169169
/// Not available under `no_module`.
170170
#[cfg(not(feature = "no_module"))]
@@ -176,7 +176,8 @@ impl GlobalRuntimeState {
176176
.zip(self.modules.iter().rev())
177177
.map(|(name, module)| (name.as_str(), &**module))
178178
}
179-
/// Get an iterator to the stack of globally-imported [modules][crate::Module] in reverse order.
179+
/// Get an iterator to the stack of globally-imported [modules][crate::Module] in reverse order
180+
/// (i.e. modules imported last come first).
180181
///
181182
/// Not available under `no_module`.
182183
#[cfg(not(feature = "no_module"))]

src/func/native.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,17 +256,16 @@ impl<'a> NativeCallContext<'a> {
256256
pub const fn tag(&self) -> Option<&Dynamic> {
257257
Some(&self.global.tag)
258258
}
259-
/// Get an iterator over the current set of modules imported via `import` statements
260-
/// in reverse order.
259+
/// Get an iterator over the current set of modules imported via `import` statements in reverse order
260+
/// (i.e. modules imported last come first).
261261
///
262262
/// Not available under `no_module`.
263263
#[cfg(not(feature = "no_module"))]
264264
#[inline]
265265
pub fn iter_imports(&self) -> impl Iterator<Item = (&str, &crate::Module)> {
266266
self.global.iter_imports()
267267
}
268-
/// Get an iterator over the namespaces containing definitions of all script-defined functions
269-
/// in reverse order (i.e. parent namespaces are iterated after child namespaces).
268+
/// Get an iterator over the namespaces containing definitions of all script-defined functions.
270269
///
271270
/// Not available under `no_function`.
272271
#[cfg(not(feature = "no_function"))]

src/func/script.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,22 +227,22 @@ impl Engine {
227227
}
228228

229229
// First check script-defined functions
230-
let res = global.lib.iter().any(|m| m.contains_fn(hash_script))
230+
let result = global.lib.iter().any(|m| m.contains_fn(hash_script))
231231
// Then check the global namespace and packages
232232
|| self.global_modules.iter().any(|m| m.contains_fn(hash_script));
233233

234234
#[cfg(not(feature = "no_module"))]
235-
let res = res ||
235+
let result = result ||
236236
// Then check imported modules
237237
global.contains_qualified_fn(hash_script)
238238
// Then check sub-modules
239239
|| self.global_sub_modules.values().any(|m| m.contains_qualified_fn(hash_script));
240240

241-
if !res && !cache.bloom_filter.is_absent_and_set(hash_script) {
241+
if !result && !cache.bloom_filter.is_absent_and_set(hash_script) {
242242
// Do not cache "one-hit wonders"
243243
cache.dict.insert(hash_script, None);
244244
}
245245

246-
res
246+
result
247247
}
248248
}

tests/functions.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,3 +726,72 @@ fn test_missing_function_is_method_call_flag() {
726726
let _: String = engine.eval(r#"let x = 42; x.greet()"#).unwrap();
727727
assert!(saw_method.load(Ordering::SeqCst), "method-style call should set is_method_call=true");
728728
}
729+
730+
#[test]
731+
#[cfg(feature = "internals")]
732+
#[cfg(not(feature = "no_index"))]
733+
#[cfg(not(feature = "no_object"))]
734+
fn on_missing_function_isolates_nested_cache_frame() {
735+
use std::sync::atomic::{AtomicUsize, Ordering};
736+
737+
// Two modules, each registering `step(target, n) -> INT` under the same
738+
// name and argument types. Same `(name, arg_types)` produces the same
739+
// fn-resolution cache hash.
740+
let mut module_a = Module::new();
741+
module_a.set_native_fn("step", |_target: &mut INT, n: INT| Ok(n * 2));
742+
let module_a = Shared::new(module_a);
743+
744+
let mut module_b = Module::new();
745+
module_b.set_native_fn("step", |_target: &mut INT, n: INT| Ok(n * 10));
746+
let module_b = Shared::new(module_b);
747+
748+
// Counter picks module_a for invocations 0-1, module_b for invocation 2+.
749+
// Three invocations are needed to exercise the cache lifecycle: call 1
750+
// sets the bloom filter bit (no dict entry yet), call 2 populates the
751+
// dict against module_a, call 3 (with module_b pushed) must hit a FRESH
752+
// resolution in its isolated frame — if isolation is missing it would
753+
// hit the stale dict entry from call 2 and return module_a's result.
754+
let call_idx = Shared::new(AtomicUsize::new(0));
755+
756+
let a = module_a.clone();
757+
let b = module_b.clone();
758+
let call_idx_clone = call_idx.clone();
759+
760+
let mut engine = Engine::new();
761+
762+
#[allow(deprecated)]
763+
engine.on_missing_function(move |name, args, _is_method_call, mut ctx| {
764+
if name != "step" {
765+
return Ok(None);
766+
}
767+
768+
let idx = call_idx_clone.fetch_add(1, Ordering::SeqCst);
769+
let module = if idx < 2 { a.clone() } else { b.clone() };
770+
771+
// Isolate the nested dispatch in a fresh cache frame. Without this,
772+
// invocation 2 would cache the resolution against module_a, and
773+
// invocation 3 would hit that stale cache entry after module_b had
774+
// been pushed.
775+
ctx.new_frame().with_new_caching_layer().with_namespace(module).call_fn_raw(name, true, false, args).map(Some)
776+
});
777+
778+
// Three method-style calls with identical name and argument types so
779+
// they all hash to the same fn-resolution cache key.
780+
let script = r#"
781+
let x = 0;
782+
[x.step(3), x.step(3), x.step(3)]
783+
"#;
784+
785+
let results: rhai::Array = engine.eval(script).expect("eval must succeed");
786+
let results: Vec<INT> = results.into_iter().map(|v| v.as_int().expect("array element must be INT")).collect();
787+
788+
assert_eq!(
789+
results,
790+
vec![6, 6, 30],
791+
"third invocation must dispatch to module_b (3*10=30), not a stale \
792+
cached entry from module_a (3*2=6); without push/rewind_fn_resolution_cache \
793+
the third element would be 6."
794+
);
795+
796+
assert_eq!(call_idx.load(Ordering::SeqCst), 3, "on_missing_function must fire exactly three times");
797+
}

0 commit comments

Comments
 (0)