refactor(value): migrate Value::Object to Object storage abstraction (#736)

Builds on #57. Swap Value::Object's payload from Rc<BTreeMap<Value, Value>>
to Rc<Object> and migrate all call sites to the Object API.

as_object / as_object_mut keep their names but return &Object / &mut Object.
The mutable accessor handles Rc::make_mut internally, so callers no longer
do it themselves. Object grows into_value() and From<Object> for Value.
Value's serializer now delegates to Object::serialize, dropping a duplicate
non-string-key stringification path.

RVM IterationState::Object is rewritten around ObjectCursor: O(log n)
steps over a shared Rc<Object>, no eager pair snapshot. Snapshot
independence is preserved by Rc copy-on-write; setup_next_iteration
advances the cursor inline and advance() becomes a no-op for this variant.
A new iteration_state_object_is_snapshot_independent_of_source test
covers CoW against a mutated alias.

Value::Set still wraps Rc<BTreeSet<Value>>; the matching Set abstraction
and its swap ship in follow-up PRs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Anand Krishnamoorthi
2026-06-05 18:04:37 -05:00
committed by GitHub
parent bd90453dd3
commit ed6ae465b0
30 changed files with 439 additions and 372 deletions
+89 -10
View File
@@ -3,8 +3,9 @@
use crate::rvm::instructions::{ComprehensionMode, LoopMode};
use crate::value::Value;
use crate::value::{Object, ObjectCursor};
use crate::Rc;
use alloc::collections::{BTreeMap, BTreeSet};
use alloc::collections::BTreeSet;
use alloc::vec::Vec;
/// Loop execution context for managing iteration state
@@ -24,7 +25,18 @@ pub struct LoopContext {
pub current_iteration_failed: bool, // Track if current iteration had condition failures
}
/// Iterator state for different collection types
/// Iterator state for different collection types.
///
/// Snapshot independence for `Object` is provided by the shared
/// `Rc<Object>` — `Rc::make_mut` on an aliased Rc allocates a new
/// collection, leaving the iterator's Rc pointing at the original
/// pre-mutation state. The `ObjectCursor` is opaque and resumes in
/// O(log n) for the BTree backend.
///
/// `Set` continues to use the pre-existing snapshot-by-cloned-key
/// approach (`current_item` + `first_iteration`); migration of `Set`
/// to a cursor-based iterator ships with the `Set` storage abstraction
/// in a follow-up PR.
#[derive(Debug, Clone)]
pub enum IterationState {
Array {
@@ -32,9 +44,8 @@ pub enum IterationState {
index: usize,
},
Object {
obj: Rc<BTreeMap<Value, Value>>,
current_key: Option<Value>,
first_iteration: bool,
obj: Rc<Object>,
cursor: ObjectCursor,
},
Set {
items: Rc<BTreeSet<Value>>,
@@ -64,11 +75,11 @@ impl IterationState {
);
*index = index.saturating_add(1);
}
Self::Object {
ref mut first_iteration,
..
}
| Self::Set {
// For Object the cursor advances inside `setup_next_iteration`
// when it pulls the next item via `Object::next`, so `advance`
// is a no-op for the cursor-backed Object variant.
Self::Object { .. } => {}
Self::Set {
ref mut first_iteration,
..
} => {
@@ -121,3 +132,71 @@ pub(super) struct ComprehensionContext {
/// Resume location for the parent frame once this comprehension completes
pub(super) resume_pc: usize,
}
#[cfg(test)]
#[allow(
clippy::expect_used,
clippy::unwrap_used,
clippy::unreachable,
clippy::pattern_type_mismatch,
clippy::shadow_unrelated,
clippy::panic
)]
mod tests {
use super::*;
use crate::value::Object;
/// IterationState::Object holds an `Rc<Object>` plus an opaque cursor.
/// Mutating an aliased Rc via `Rc::make_mut` allocates a new collection
/// (CoW) so the in-flight iterator's source is unaffected.
#[test]
fn iteration_state_object_is_snapshot_independent_of_source() {
let mut obj = Object::new();
obj.insert(Value::from("a"), Value::from(1));
obj.insert(Value::from("b"), Value::from(2));
obj.insert(Value::from("c"), Value::from(3));
let source = Value::Object(Rc::new(obj));
let snapshot_obj = match &source {
Value::Object(o) => Rc::clone(o),
_ => unreachable!(),
};
let state = IterationState::Object {
obj: Rc::clone(&snapshot_obj),
cursor: snapshot_obj.cursor(),
};
// Mutate a clone of the source mid-iteration.
let mut alias = source.clone();
let inner = alias.as_object_mut().expect("object");
inner.insert(Value::from("a"), Value::from(999));
inner.insert(Value::from("d"), Value::from(4));
inner.remove(&Value::from("b"));
// Drain the snapshot via the cursor — must still report the original
// 3 entries with original values.
let mut collected: Vec<(Value, Value)> = Vec::new();
if let IterationState::Object {
ref obj,
mut cursor,
} = state
{
while let Some((k, v)) = obj.next(&mut cursor) {
collected.push((k.clone(), v.clone()));
}
} else {
unreachable!();
}
assert_eq!(collected.len(), 3);
assert!(collected.contains(&(Value::from("a"), Value::from(1))));
assert!(collected.contains(&(Value::from("b"), Value::from(2))));
assert!(collected.contains(&(Value::from("c"), Value::from(3))));
assert!(!collected.iter().any(|kv| kv.0 == Value::from("d")));
// The original source Value (untouched) is also unchanged.
let src_obj = source.as_object().expect("object");
assert_eq!(src_obj.len(), 3);
assert_eq!(src_obj.get(&Value::from("a")), Some(&Value::from(1)));
}
}