Skip to content

Commit 18dc385

Browse files
committed
Clone a chain index before the getter consumes it.
`index_by_reference` cloned the index for the write-back setter after calling `get_indexed_mut`, which binds a native's by-value parameter by `take` — so the setter was called with `()`, found nothing, and `call_indexer_set` swallowed the error. The write was dropped silently. Rhai clones before the same call (`eval/chaining.rs:706`). Only the paths that can write pay for it.
1 parent 7a32157 commit 18dc385

2 files changed

Lines changed: 21 additions & 1 deletion

File tree

src/grain/vm/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,6 +1440,13 @@ impl<'e> Vm<'e> {
14401440
let assigning = last && value.is_some();
14411441
let mut detached = Scope::new();
14421442

1443+
// Cloned *before* the call, as Rhai clones it (`eval/chaining.rs:706`).
1444+
// `get_indexed_mut` may reach a custom indexer, and a native's by-value
1445+
// parameter is bound by `take` (`func/register.rs:69`) — so afterwards
1446+
// there is nothing left to address the setter with. Only the paths that
1447+
// can write need it; a read returns below without ever looking.
1448+
let index_for_setter = (!last || value.is_some()).then(|| idx.clone());
1449+
14431450
let mut item = match self.engine.get_indexed_mut(
14441451
&mut self.global,
14451452
&mut self.caches,
@@ -1497,7 +1504,7 @@ impl<'e> Vm<'e> {
14971504
// The element was a temporary — a custom indexer's — so the setter
14981505
// is the only way back (`eval/chaining.rs:744`).
14991506
let mut updated = item.take_or_clone();
1500-
let mut index = idx.clone();
1507+
let mut index = index_for_setter.expect("a read returns before here");
15011508
self.call_indexer_set(target, &mut index, &mut updated, bracket)?;
15021509
}
15031510

tests/grain/corpus/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ pub fn engine() -> rhai::Engine {
9797
.register_type_with_name::<Holder>("Holder")
9898
.register_fn("holder", |level: INT| Holder { inner: Widget { level, cells: vec![1, 2, 3] } })
9999
.register_get_set("inner", |h: &mut Holder| h.inner.clone(), |h: &mut Holder, w: Widget| h.inner = w);
100+
101+
// The same `Widget`, behind an *indexer* rather than a property. The
102+
// getter above is reached with the property name as a static operand;
103+
// this one is reached with an index the walk has to evaluate, and a
104+
// native's by-value parameter is bound by `take` — so the index a
105+
// write-back needs is gone by the time the setter wants it.
106+
#[cfg(not(feature = "no_index"))]
107+
engine.register_indexer_get_set(|h: &mut Holder, _: INT| h.inner.clone(), |h: &mut Holder, _: INT, w: Widget| h.inner = w);
100108
}
101109

102110
engine
@@ -227,6 +235,7 @@ pub fn applies_to_this_build(name: &str) -> bool {
227235
| "host_index_get"
228236
| "host_index_set"
229237
| "host_mutation_before_a_failure_survives_in_an_array"
238+
| "host_index_temp_set"
230239
| "host_temp_index_set"
231240
| "index_assign_array"
232241
| "index_assign_nested"
@@ -726,6 +735,10 @@ pub const CASES: &[Case] = &[
726735
// Two levels, so the middle one is a temporary.
727736
case("host_temp_set", "let h = holder(3); h.inner.level = 8; h.inner.level"),
728737
case("host_temp_index_set", "let h = holder(3); h.inner[0] = 7; h.inner[0]"),
738+
// The mirror of it: an *index* step handing back the temporary, with the
739+
// property below. The index has to survive the getter to address the setter
740+
// with afterwards.
741+
case("host_index_temp_set", "let h = holder(3); h[0].level = 8; h.inner.level"),
729742
// A mutating call on a temporary: Rhai writes it back, so the change
730743
// survives.
731744
case("host_temp_mutates", "let h = holder(3); h.inner.bump(); h.inner.level"),

0 commit comments

Comments
 (0)