rhaiscript / rhai

Rhai - An embedded scripting language for Rust.
https://crates.io/crates/rhai
Apache License 2.0
3.63k stars 174 forks source link

on_var not called for variables in closures #862

Closed justinpombrio closed 3 months ago

justinpombrio commented 3 months ago

The on_var callback seems to not be invoked for variables that appear in closures.

Rhai version: 1.18.0

Input

use rhai::{Dynamic, Engine};

fn main() {
    let mut engine = Engine::new();
    let shared_state = 173; // In reality, would be an Rc<RefCell<_>>

    #[allow(deprecated)]
    engine.on_var(move |name, _, _| {
        if name == "state" {
            Ok(Some(Dynamic::from(shared_state)))
        } else {
            Ok(None)                                     
        }
    });

    engine.run("state").unwrap(); // variable found
    engine.run("fn f() { state }; f()").unwrap(); // variable found
    engine.run("|| state").unwrap(); // variable not found
}

Expected: the on_var callback to be invoked in all three cases.

Actual: the on_var callback is not invoked for the closure, resulting in a "variable not found" error.

Comments: This makes on_var ineffective for our use case. Our goal is to expose a single global "runtime" for use everywhere. User written scripts should be able to access methods like runtime.do_thing() anywhere in any script. Here's what we've tried overall:

The final thing we'll try (if this issue isn't fixable) is to bind every method on Runtime as a function in Rhai. But wrap them all in a module, so that they're not all spilled into the global namespace (there may be hundreds of them all told).

schungx commented 3 months ago

Yes this seems to be a bug.

The reason: closures are actually syntactical sugar for function calls with capturing. So the script:

|| state    // 'state' is captured from outer scope here

is compiled into:

fn anon_fn(state) { state }        // the closure body

make_shared(state);       // make the 'state' variable shared

Fn("anon_fn").curry(state);   // curry it into the function pointer

So in reality state exists within the function as a parameter. The make_shared operation, however, should respect the on_var callback, which it obviously doesn't.

schungx commented 3 months ago

This is a relatively simple fix. Do you need this fix urgently? If so, I can release 1.18.1... otherwise you can pull from this repo until 1.19.0 comes out...

justinpombrio commented 3 months ago

Do you need this fix urgently?

Nope! We got our test case working with a Rhai module and FuncRegistration instead. We might (or might not) eventually switch to on_var instead, but not at all urgent.

Thanks for the quick response!

schungx commented 3 months ago

Thanks. The fix has landed here. You can pull from this repo to try it out.

justinpombrio commented 3 months ago

Confirmed fixed!

schungx commented 3 months ago

Closing this for now. Feel free to reopen if there are further problems.