rhaiscript / rhai

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

Compiling a script with imports co-compiled. #325

Closed stustd closed 3 years ago

stustd commented 3 years ago

In a rhaiscript the import statement is dynamic, and it loads whenever it executes. Theoretically the engine can supply a different module every time This means that Engine::compile_file(...) compiles a main script into an into an AST without co-compiling its imports is not sufficient for evaluation purposes (the imported modules are just references in the main script's AST) -- thus, the imported functionality is not co-compiled.

Compiling a script with Module::eval_ast_as_new(...) however co-compiles the entire imported module tree. This compiled module however cannot be used as main AST for evaluation and must be registered with Engine::register_static_module(name, module_shared). The combination of Engine::compile_file(...) the main script and Module::eval_ast_as_new(...) for the imported modules works. Find below the Rust code that facilitates this:

use rhai as scripting;

fn load_rhai_script(
    slf: &mut Conduit,
    script_name: &str,
) {
    let script_name = script_name.to_string();
    let script_path = format!("{}/{}.rhai", CFG.ses.scripts.dir, script_name);  // full path

    if std::fs::metadata(&script_path).is_ok() {
        println!("Loading script: {}", script_path);
        slf.script_path = script_path;
        let ast = slf.script_eng.compile_file(slf.script_path.clone().into()).unwrap();
        println!("{:#?}", ast);
        println!("Statements: {:#?}", ast.statements());

        // Fn to register script/module with engine.
        fn register_module(path: &str, name: &str, conduit: &mut Conduit) {
            let path = format!("{}/{}.rhai", CFG.ses.dir, path);
            let ast = conduit.script_eng.compile_file(path.into()).unwrap();
            let module = rhai::Module::eval_ast_as_new(
                conduit.script_scope.clone(), &ast, &conduit.script_eng
            ).unwrap();
            let module_shared: scripting::Shared<rhai::Module> = module.into();
            conduit.script_eng.register_static_module(name, module_shared);
        }

        // Find and compile top-level imports.
        ast.statements().iter()
            .for_each(
                |stmt| {
                    match stmt {
                        scriping::Stmt::Import(expr, ident, _pos) => {
                            let import_path = expr.get_constant_value().unwrap().as_str().unwrap().to_string();
                            let import_name = match ident {
                                Some(id) => id.name.as_str().to_string(),
                                None => "".to_string()
                            };
                            register_module(import_path.as_str(), import_name.as_str(), &mut slf)
                        },
                        _ => ()
                    }
                }
            );

        slf.script_ast = ast;
    }
}
schungx commented 3 years ago

This only works for modules import at the global level. It will not work for import statements within functions or a nested block.

Also, you'd need to remove the import statements, because otherwise the scripts will simply be imported again. It is working for you because you probably kept the scripts around. If they are no longer available, then evaluating the script will be an error.

For example, the statement: import "hello" as hello; will error even if hello is a valid namespace that has already been registered. You need to physically remove the statements, like this:

loop {
    let mut remove = None;
    for (i, stmt) in ast.statements().iter().enumerate() {
        match stmt {
            Stmt::Import(...) => {  ....  remove = Some(i); break; }
        }
    }

    if let Some(i) = remove {
        ast.statements_mut().remove_at(i);
    } else {
        break;
    }
}
schungx commented 3 years ago

I can make something like an AST::extract_imports() that both removes top-level import statements and return their info, thus simplifying what you plan to do.

But still, you'll have problems with import statements not at top-level, but maybe that's what your use case wants?

stustd commented 3 years ago

I'm only at the beginning of employing rhai scripts and only use (so far) import statements at the top-level. Nonetheless, from your perspective I would pursue a generic implementation; not just something that satisfies my beginner's needs.

Also, if my understanding is correct, removing the import statements of non top-level imports will require adding scoped module information to the AST - not sure how complicated this is..

schungx commented 3 years ago

Also, if my understanding is correct, removing the import statements of non top-level imports will require adding scoped module information to the AST - not sure how complicated this is..

That's correct.

An alternative is to walk the entire AST to find all import statements, then load all modules, and then push it as a module resolver on top of any existing one. Then any import statement will resolve to a pre-loaded module linked into the AST.

stustd commented 3 years ago

Update of above script (stripped from context, top-level import statements removed from AST):

pub static SCRIPT_EXT: &str = "rhai";

/** Compiles Rhai main script file and imported top-level script files into an AST.
`script_name` relative to `base_dir`.

Note:
1. top-level imports are removed from the AST (and thus won't be reloaded when evaluating AST);  
2. only top-level imports are handled (nested imports will be reloaded for each AST evaluation).  
*/
pub fn compile_file_inclusive(
    script_name: &str,
    base_dir: &str,
    scope: &mut Scope,
    engine: &mut Engine,
) -> Option<AST> {

    let base_dir = base_dir.to_string();
    let script_path = base_dir.clone() + "/" + script_name;  // with ext.
    if ! std::fs::metadata(&script_path).is_ok() {
        println!("No script file: {}", script_path);
        return None; // Nothing if not a proper script.
    }

    println!("Loading script: {}", script_path);
    // slf.script_ast = slf.script_eng.compile_file(slf.script_path.clone().into()).unwrap();
    let mut ast = engine.compile_file(script_path.clone().into()).unwrap();
    println!("{:#?}", ast);
    println!("Statements: {:#?}", ast.statements());

    fn register_module(path: &str, name: &str, scope: Scope, engine: &mut Engine) {
        let ast = engine.compile_file(path.into()).unwrap();
        let module = Module::eval_ast_as_new(scope, &ast, engine).unwrap();
        let shared: Shared<rhai::Module> = module.into();
        engine.register_static_module(name, shared);
    }

    // Compile imports.
    let mut imports = Vec::<usize>::new();
    ast.statements().iter().enumerate()
        .for_each(
            |(i_stmt, stmt)| {
                match stmt {
                    Stmt::Import(expr, ident, _pos) => {
                        let script_path = expr.get_constant_value()
                            .unwrap().as_str().unwrap().to_string();
                        let module_name = match ident {
                            Some(id) => id.name.as_str().to_string(),
                            None => "".to_string()
                        };
                        let import_path = format!(
                            "{}/{}.{}", base_dir, script_path, SCRIPT_EXT);
                        register_module(
                            import_path.as_str(), module_name.as_str(),
                            scope.clone(), engine);
                        //
                        imports.push(i_stmt - imports.len());  // len(): shift for removed stmts.
                    },
                    _ => ()
                }
            }
        );
    // Remove import statements.
    imports.iter().for_each(
        |i| { let _ = ast.statements_mut().remove(*i); } );

    println!("Imports removed:\n{:#?}", ast);

    Some(ast)
}
schungx commented 3 years ago

Glad it is working out for you...

I think of a way to have it work for all import statements, even ones within functions and blocks, and no need to muck up the Engine with static namespaces. There is also no need to remove the import statements.

The AST can be walked to isolate all import statements, then the modules loaded.

Then we make a StaticModuleResolver, adding all the modules into it.

We can then Engine::set_module_resolver.

When Rhai reaches an import statement, it asks your module resolver for a module, which you already have.

If we add a module resolver into an AST structure, then we can encapsulate everything inside the AST.

stustd commented 3 years ago

Glad to hear your outline for a comprehensive solution. I can't focus on that now and shall be happy with the top-level functionality as is.

schungx commented 3 years ago

We'll keep this as a TODO for the time being, as it is a niche feature and it can be done completely within the existing API without introducing new functionalities.

stustd commented 3 years ago

Good idea. Just a small question: why is it considered a "niche feature"? The reason I'm asking is that from the moment I started using rhai it occurred to me as a sort of "natural feature" to have... I also want to avoid becoming an unintended "fringe user"...

schungx commented 3 years ago

😆 Just because you're the first user to ask for it... but I agree, this is something that makes a lot of sense.

schungx commented 3 years ago

In this case, you've talked me into it. I'll add a new API to do this over the weekend. 👌

stustd commented 3 years ago

:-)

schungx commented 3 years ago

PR https://github.com/rhaiscript/rhai/pull/328 now has this feature you can try out.

Check out Engine::compile_into_self_contained.

schungx commented 3 years ago

Currently it is not recursive. Meaning that import statements within module scripts are not pre-resolved. I am still thinking of an elegant way to resolve this issue.

stustd commented 3 years ago

Indeed it works, thanks!

Re an elegant way to resolve the general issue, this 2019 Video about RustPython at 3:00 min may contain a clue.

schungx commented 3 years ago

Well, actually, the latest master already has a new fix that makes Engine::compile_into_self_contained recursive.

Please test it and let me know if it still works!

stustd commented 3 years ago

Doesn't seem to work yet but I think you're close... I send you the error msg, the top-level script file read & compiled and the AST produced. (Notice: in this test there are only top-level imports, so after solving this we still need to test for scoped imports.)

Err msg:

script: Incrementing... (increment)
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: ErrorInFunctionCall("increment", "", ErrorModuleNotFound("utils", 19:5), 0:0)', src/actor/lf.rs:196:10

Script read:

import "actors/usr/utils" as utils;
import "actors/usr/utils3" as utils3;

fn increment(value) {
    print("script: Incrementing... (increment)");
    utils::trace("incrementing... (trace)");
    utils3::trace("incrementing... (trace3)");
    value + 1
}

AST produced:

AST {
    source: None,
    statements: [
        Import(
            StringConstant(
                "actors/usr/utils",
                4:8,
            ),
            Some(
                Ident("utils" @ 4:30),
            ),
            4:1,
        ),
        Import(
            StringConstant(
                "actors/usr/utils3",
                5:8,
            ),
            Some(
                Ident("utils3" @ 5:31),
            ),
            5:1,
        ),
    ],
    functions: Module(
        modules: 
        vars: 
        functions: increment(value)
    ),
    resolver: Some(
        StaticModuleResolver(
            {
                "actors/usr/utils2": Module(id: "actors/usr/utils2"
                    modules: 
                    vars: 
                    functions: trace(msg)
                ),
                "actors/usr/utils": Module(id: "actors/usr/utils"
                    modules: utils2
                    vars: 
                    functions: trace(msg)
                ),
                "actors/usr/utils3": Module(id: "actors/usr/utils3"
                    modules: 
                    vars: 
                    functions: trace(msg)
                ),
            },
        ),
    ),
}
schungx commented 3 years ago

import statements are scoped. Function bodies can never access the global scope.

Therefore you need to put the import statements inside the function:

fn increment(value) {
    import "actors/usr/utils" as utils;
    import "actors/usr/utils3" as utils3;

    print("script: Incrementing... (increment)");
    utils::trace("incrementing... (trace)");
    utils3::trace("incrementing... (trace3)");
    value + 1
}
stustd commented 3 years ago

Indeed, that works, congrats!

If I'm not mistaken, 'conventionally' (e.g. Rust, Python), functions do have access to modules in global scope, why is that different in rhai?

schungx commented 3 years ago

In order to access a different scope, there must be a "scope chain". Searching a scope chain is slow (right now Rhai does a straight index lookup for variables without having to search through a scope chain). That's why early JavaScript engines are so slow.

Rhai has a different fundamental design, one of them is pure functions. The other one is a unified scope without scope chains.

schungx commented 3 years ago

I'd appreciate it if you'd help test out the following combination:

utils.rhai: add function that imports:

fn trace_indirect(x) {
    import "utils4" as u4;
    u4::trace(x);
}

utils4.rhai:

fn trace(x) { ... }

Let's see if the following works without resolving the imports at run time:

import "actors/usr/utils" as utils;

utils::trace_indirect("tracing indirectly");

If the recursive feature of Engine::compile_into_self_contained works properly, all those import statements will be pre-resolved.

stustd commented 3 years ago

Yes that works with the following small modification:

fn trace_indirect(x) {                                                                                                                                                                                                     
    import "actors/usr/utils4" as u4;      // path relative to base_path.                                                                                                                                                                                      
    u4::trace("indirectly " + x);                                                                                                                                                                                          
}                                                                                                                                                                                                                          
schungx commented 3 years ago

Great! So we've wrapped up a new feature. Thanks for bringing it up!

stustd commented 3 years ago

With pleasure. Thanks for the effort!

On Tue, 12 Jan 2021, 02:30 Stephen Chung, notifications@github.com wrote:

Great! So we've wrapped up a new feature. Thanks for bringing it up!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rhaiscript/rhai/issues/325#issuecomment-758352994, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5WVT7YQCW4NF3NRPAEAZTSZOX5BANCNFSM4VXRJCNA .