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

Unexpected result in For loop - bug in shared value assignment #366

Closed wxxxcxx closed 3 years ago

wxxxcxx commented 3 years ago

Version: 0.19.13

My rhai file:

let content=#{
        title:"Test Title",
        tags:["Hello","World"]
};
let tags = [];
for name in content.tags {
    if tags.some(|t| t == name) {
    } else {
        let group = #{
            name: name,
        };
        tags.push(group);
    };
}
print(tags) ;

Output:

[#{"name": "World"}, #{"name": "World"}]

Expected

[#{"name": "Hello"}, #{"name": "World"}]

If I use the following code

let content=#{
        title:"Test Title",
        tags:["Hello","World"]
};
let tags = [];
for name in content.tags {
    if tags.some(|t| t == name) {
    } else {
        let group = #{
            name: "" + name, // Modified here
        };
        tags.push(group);
    };
}
print(tags) ;

Or

let content=#{
        title:"Test Title",
        tags:["Hello","World"]
};
let tags = [];
for name in content.tags {
    let group = #{
        name: name, 
    };
    tags.push(group);
}
print(tags) ;

It works.

schungx commented 3 years ago

Looking at your code, it is probably a regression.

If you capture name in a closure, name becomes a captured variable, so all the name fields in all your groups point to the same variable with the last set value.

Try:

print(is_shared(tags[0].name));

That's why you get the intended behavior with "" + name because it then returns a new string.

Assignments should get a copy of the result instead of cloning the share. I'll try to hunt this down and fix it.

Thanks for the catch.

schungx commented 3 years ago

I fixed it in the latest master: https://github.com/rhaiscript/rhai/pull/367

It'll go into the next version. If you need it now, you can pull from this repo.

Thanks!

wxxxcxx commented 3 years ago

In the previous example, I made a mistake. The closure should compare with the tag's name

tags.some(|t| t.name == name)

But now, if I correct it, there will be panic (master branch, commit id ec0d2dd).

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "string"', /home/maf/.cargo/git/checkouts/rhai-f99f240b2f7f4b04/ec0d2dd/src/fn_builtin.rs:344:21
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b/library/core/src/panicking.rs:92:14
   2: core::option::expect_none_failed
             at /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b/library/core/src/option.rs:1268:5
   3: core::ops::function::FnOnce::call_once
   4: core::ops::function::Fn::call
   5: rhai::fn_call::<impl rhai::engine::Engine>::call_native_fn
   6: rhai::fn_call::<impl rhai::engine::Engine>::exec_fn_call
   7: rhai::fn_call::<impl rhai::engine::Engine>::make_function_call
   8: rhai::engine::Engine::eval_expr
   9: rhai::engine::Engine::eval_stmt
  10: rhai::fn_call::<impl rhai::engine::Engine>::call_script_fn
  11: rhai::fn_call::<impl rhai::engine::Engine>::exec_fn_call
  12: <rhai::packages::array_basic::array_functions::some_token as rhai::plugin::PluginFunction>::call
  13: rhai::fn_call::<impl rhai::engine::Engine>::call_native_fn
  14: rhai::fn_call::<impl rhai::engine::Engine>::exec_fn_call
  15: rhai::fn_call::<impl rhai::engine::Engine>::make_method_call
  16: rhai::engine::Engine::eval_dot_index_chain_helper
  17: rhai::engine::Engine::eval_dot_index_chain
  18: rhai::engine::Engine::eval_expr
  19: rhai::engine::Engine::eval_stmt
  20: rhai::engine::Engine::eval_stmt
  21: rhai::engine::Engine::eval_stmt_block
  22: core::service::site_service::SiteService::render_model
  23: core::service::site_service::SiteService::generate
  24: door::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The following code can reproduce this problem.

let tags = [#{
    name: "Hello"
},#{
    name: "World"
}];
let name = "Hello";
tags.some(|t| t.name == name); // pacic
// tags.some(|t| t.name == "Hello"); //It works
schungx commented 3 years ago

Ah, yes. Forgot it is shared. Bummer. Will fix!

schungx commented 3 years ago

Fixed it in the latest drop: https://github.com/rhaiscript/rhai/pull/370

schungx commented 3 years ago

Has this issue been fully resolved?

wxxxcxx commented 3 years ago

It works very well now. Thanks for your works.❤️