Skip to content

Commit 50770ce

Browse files
anakrishCopilot
andcommitted
refactor(value): migrate Value::Object to Object storage abstraction
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>
1 parent 11940dd commit 50770ce

30 files changed

Lines changed: 429 additions & 372 deletions

File tree

src/builtins/azure_policy/helpers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ pub fn resolve_path(root: &Value, path: &str) -> Value {
319319
match &current {
320320
Value::Object(map) => {
321321
let mut next = None;
322-
for (key, value) in map.iter() {
322+
for (key, value) in map.iter_sorted() {
323323
if let Value::String(ref key_str) = *key {
324324
if strings::keys::eq(key_str, &segment) {
325325
next = Some(value.clone());

src/builtins/azure_policy/template_functions_collection.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
use crate::ast::{Expr, Ref};
99
use crate::builtins;
1010
use crate::lexer::Span;
11+
use crate::value::Object;
1112
use crate::value::Value;
1213
use crate::Rc;
1314

14-
use alloc::collections::BTreeMap;
1515
use alloc::vec::Vec;
1616
use anyhow::Result;
1717

@@ -72,7 +72,7 @@ fn fn_intersection(
7272
// Intersection of objects: keep key-value pairs from the first
7373
// object only when the key exists in every other object AND
7474
// the value is equal across all of them.
75-
let mut result: BTreeMap<Value, Value> = first.as_ref().clone();
75+
let mut result: Object = first.as_ref().clone();
7676
for arg in rest {
7777
let Value::Object(ref other) = *arg else {
7878
return Ok(Value::Undefined);
@@ -114,7 +114,7 @@ fn fn_union(_span: &Span, _params: &[Ref<Expr>], args: &[Value], _strict: bool)
114114
Value::Object(_) => {
115115
// Union of objects: recursive merge. Nested objects are merged
116116
// recursively; all other types (including arrays) use last-writer-wins.
117-
let mut result = BTreeMap::<Value, Value>::new();
117+
let mut result = Object::new();
118118
for arg in args {
119119
let Value::Object(ref obj) = *arg else {
120120
return Ok(Value::Undefined);
@@ -264,7 +264,7 @@ fn fn_create_object(
264264
);
265265
}
266266

267-
let mut map = BTreeMap::<Value, Value>::new();
267+
let mut map = Object::new();
268268

269269
for pair in args.chunks(2) {
270270
#[allow(clippy::pattern_type_mismatch)]
@@ -280,9 +280,9 @@ fn fn_create_object(
280280

281281
/// Recursively merge two objects. Nested objects are merged; everything
282282
/// else (including arrays) uses the value from `incoming`.
283-
fn merge_objects(base: &BTreeMap<Value, Value>, overlay: &BTreeMap<Value, Value>) -> Value {
283+
fn merge_objects(base: &Object, overlay: &Object) -> Value {
284284
let mut result = base.clone();
285-
for (k, v) in overlay {
285+
for (k, v) in overlay.iter() {
286286
#[allow(clippy::needless_borrowed_reference)]
287287
let merged = match (result.get(k), v) {
288288
(Some(&Value::Object(ref prev)), &Value::Object(ref next)) => merge_objects(prev, next),

src/builtins/azure_policy/template_functions_misc.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
use crate::ast::{Expr, Ref};
99
use crate::builtins;
1010
use crate::lexer::Span;
11+
use crate::value::Object;
1112
use crate::value::Value;
1213
use crate::Rc;
1314

14-
use alloc::collections::BTreeMap;
1515
use alloc::string::{String, ToString as _};
1616
use alloc::vec::Vec;
1717
use anyhow::Result;
@@ -84,8 +84,8 @@ fn fn_items(_span: &Span, _params: &[Ref<Expr>], args: &[Value], _strict: bool)
8484
return Ok(Value::Undefined);
8585
};
8686
let mut result = Vec::with_capacity(obj.len());
87-
for (k, v) in obj.as_ref() {
88-
let mut entry = BTreeMap::<Value, Value>::new();
87+
for (k, v) in obj.iter_sorted() {
88+
let mut entry = Object::new();
8989
entry.insert(Value::from("key"), k.clone());
9090
entry.insert(Value::from("value"), v.clone());
9191
result.push(Value::Object(Rc::new(entry)));

src/builtins/encoding.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ fn urlquery_encode_object(
308308

309309
{
310310
let mut pairs = url.query_pairs_mut();
311-
for (key, value) in obj.iter() {
311+
for (key, value) in obj.iter_sorted() {
312312
let key = ensure_string(name, &params[0], key)?;
313313
match value {
314314
Value::String(v) => {

src/builtins/graph.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@ use crate::ast::{Expr, Ref};
77
use crate::builtins;
88
use crate::builtins::utils::{enforce_limit, ensure_args_count, ensure_object};
99
use crate::lexer::Span;
10+
use crate::value::Object;
1011
use crate::value::Value;
1112
use crate::*;
1213

13-
use alloc::collections::{BTreeMap, BTreeSet};
14+
use alloc::collections::BTreeSet;
1415

1516
use anyhow::{bail, Result};
1617

@@ -80,7 +81,7 @@ fn reachable(span: &Span, params: &[Ref<Expr>], args: &[Value], strict: bool) ->
8081
}
8182

8283
fn visit(
83-
graph: &BTreeMap<Value, Value>,
84+
graph: &Object,
8485
visited: &mut BTreeSet<Value>,
8586
node: &Value,
8687
path: &mut Vec<Value>,
@@ -211,7 +212,7 @@ fn walk_visit(path: &mut Vec<Value>, value: &Value, paths: &mut Vec<Value>) -> R
211212
}
212213
}
213214
Value::Object(obj) => {
214-
for (key, value) in obj.iter() {
215+
for (key, value) in obj.iter_sorted() {
215216
path.push(key.clone());
216217
// Guard path stack growth while traversing object entries.
217218
enforce_limit()?;

src/builtins/objects.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ fn merge_filters(
205205
let vref = match f {
206206
Value::Object(obj) => {
207207
let obj = Rc::make_mut(obj);
208-
let entry = obj.entry(p.clone()).or_insert_with(Value::new_object);
208+
let entry = obj.get_or_insert_with(p.clone(), Value::new_object);
209209
// Guard filter map growth when creating nested objects.
210210
enforce_limit()?;
211211
entry

src/builtins/strings.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ fn to_string(v: &Value, unescape: bool) -> String {
207207
}
208208
Value::Object(o) => {
209209
"{".to_owned()
210-
+ &o.iter()
210+
+ &o.iter_sorted()
211211
.map(|(k, v)| to_string(k, true) + ": " + &to_string(v, true))
212212
.collect::<Vec<String>>()
213213
.join(", ")
@@ -568,7 +568,7 @@ fn replace_n(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -
568568
let mut s = ensure_string(name, &params[1], &args[1])?;
569569

570570
let span = params[0].span();
571-
for item in obj.as_ref().iter() {
571+
for item in obj.as_ref().iter_sorted() {
572572
match item {
573573
(Value::String(k), Value::String(v)) => {
574574
s = s.replace(k.as_ref(), v.as_ref()).into();

src/builtins/utils.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
use crate::ast::{Expr, Ref};
66
use crate::lexer::Span;
77
use crate::number::Number;
8+
use crate::value::Object;
89
use crate::Rc;
910
use crate::Value;
1011
use crate::*;
1112

12-
use alloc::collections::{BTreeMap, BTreeSet};
13+
use alloc::collections::BTreeSet;
1314

1415
use anyhow::{bail, Result};
1516

@@ -168,7 +169,7 @@ pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeSet<Value>>
168169
})
169170
}
170171

171-
pub fn ensure_object(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeMap<Value, Value>>> {
172+
pub fn ensure_object(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Object>> {
172173
Ok(match v {
173174
Value::Object(o) => o,
174175
_ => {

src/interpreter.rs

Lines changed: 21 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ use crate::{Expression, Extension, Location, QueryResult, QueryResults};
2828
use crate::query::traversal::traverse;
2929

3030
use crate::Rc;
31-
use alloc::collections::btree_map::Entry as BTreeMapEntry;
3231
use alloc::collections::{BTreeMap, BTreeSet};
3332
use anyhow::{anyhow, bail, Result};
3433
use core::ops::Bound::*;
@@ -1312,10 +1311,10 @@ impl Interpreter {
13121311
*obj = Value::new_object();
13131312
}
13141313

1315-
obj = obj
1316-
.as_object_mut()?
1317-
.entry(Value::String(p.to_string().into()))
1318-
.or_insert(Value::new_object());
1314+
obj = obj.as_object_mut()?.get_or_insert_with(
1315+
Value::String(p.to_string().into()),
1316+
Value::new_object,
1317+
);
13191318
}
13201319
*obj = value;
13211320
// Mark modified rules as processed.
@@ -1682,29 +1681,21 @@ impl Interpreter {
16821681
let set = obj
16831682
.as_object_mut()
16841683
.map_err(|_| anyhow!(span.error("previous value is not an object")))?
1685-
.entry(p)
1686-
.or_insert(Value::new_set())
1684+
.get_or_insert_with(p, Value::new_set)
16871685
.as_set_mut()
16881686
.map_err(|_| anyhow!(span.error("previous value is not a set")))?;
16891687
set.append(value.as_set_mut()?);
16901688
} else {
16911689
let obj = obj
16921690
.as_object_mut()
16931691
.map_err(|_| anyhow!(span.error("previous value is not an object")))?;
1694-
match obj.entry(p) {
1695-
BTreeMapEntry::Vacant(v) => {
1696-
if value != Value::Undefined {
1697-
v.insert(value);
1698-
} else {
1699-
// TODO: clean this assumption between Undefined vs Object.
1700-
v.insert(Value::new_object());
1701-
}
1702-
}
1703-
BTreeMapEntry::Occupied(o) => {
1704-
if o.get() != &value && value != Value::Undefined {
1705-
bail!(span
1706-
.error("complete rules should not produce multiple outputs"))
1707-
}
1692+
if value == Value::Undefined {
1693+
// TODO: clean this assumption between Undefined vs Object.
1694+
obj.get_or_insert_with(p, Value::new_object);
1695+
} else {
1696+
let existing = obj.get_or_insert_with(p, || value.clone());
1697+
if *existing != value {
1698+
bail!(span.error("complete rules should not produce multiple outputs"))
17081699
}
17091700
}
17101701
}
@@ -1713,8 +1704,7 @@ impl Interpreter {
17131704
obj = obj
17141705
.as_object_mut()
17151706
.map_err(|_| anyhow!(span.error("previous value is not an object")))?
1716-
.entry(p)
1717-
.or_insert(Value::new_object());
1707+
.get_or_insert_with(p, Value::new_object);
17181708
}
17191709
}
17201710
Ok(())
@@ -1822,8 +1812,7 @@ impl Interpreter {
18221812
let set = ctx_mut
18231813
.rule_value
18241814
.as_object_mut()?
1825-
.entry(Value::from_array(comps))
1826-
.or_insert(Value::new_set());
1815+
.get_or_insert_with(Value::from_array(comps), Value::new_set);
18271816
if output != Value::Undefined {
18281817
set.as_set_mut()?.insert(output);
18291818
return Ok(true);
@@ -1832,20 +1821,13 @@ impl Interpreter {
18321821
}
18331822

18341823
// Non-set rule.
1835-
match ctx_mut
1836-
.rule_value
1837-
.as_object_mut()?
1838-
.entry(Value::from_array(comps))
1839-
{
1840-
BTreeMapEntry::Vacant(v) => {
1841-
v.insert(output);
1842-
}
1843-
BTreeMapEntry::Occupied(o) if o.get() != &output => bail!(rule_ref
1824+
let key = Value::from_array(comps);
1825+
let obj_mut = ctx_mut.rule_value.as_object_mut()?;
1826+
let existing = obj_mut.get_or_insert_with(key, || output.clone());
1827+
if *existing != output {
1828+
bail!(rule_ref
18441829
.span()
1845-
.error("rules must not produce multiple outputs")),
1846-
_ => {
1847-
// Rule produced same value.
1848-
}
1830+
.error("rules must not produce multiple outputs"));
18491831
}
18501832

18511833
return Ok(true);
@@ -2471,7 +2453,7 @@ impl Interpreter {
24712453
}
24722454
Value::Object(map) => {
24732455
s.push('{');
2474-
for (idx, (k, entry_value)) in map.iter().enumerate() {
2456+
for (idx, (k, entry_value)) in map.iter_sorted().enumerate() {
24752457
if idx > 0 {
24762458
s.push_str(", ");
24772459
}

src/languages/azure_policy/aliases/denormalizer/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,10 @@ pub fn denormalize_with_aliases(
213213
// Phase 4: Attach properties to result.
214214
if !properties.is_empty() {
215215
if let Some(Value::Object(existing_rc)) = result.get_mut("properties") {
216-
// Merge directly into the BTreeMap, avoiding full ObjMap round-trip.
216+
// Merge directly into the Object, avoiding full ObjMap round-trip.
217217
let existing = Rc::make_mut(existing_rc);
218218
for (k, v) in properties {
219-
existing.entry(Value::String(k)).or_insert(v);
219+
existing.get_or_insert_with(Value::String(k), || v);
220220
}
221221
} else {
222222
obj_insert(&mut result, "properties", make_value(properties));

0 commit comments

Comments
 (0)