Skip to content

Commit 0c25af1

Browse files
anakrishCopilot
andauthored
refactor(value): migrate Value::Set to Set storage abstraction (#778)
Switch Value::Set from Rc<BTreeSet<Value>> to Rc<Set>, wiring the previously-merged Set wrapper struct (#740) into the enum and updating all call sites across interpreter, RVM, builtins, serialization, and languages. Mirrors the merged Object migration (#736). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc92437e-8820-49a4-ba5f-ec82a79f1221
1 parent 29f1cc8 commit 0c25af1

18 files changed

Lines changed: 171 additions & 303 deletions

File tree

src/builtins/json_patch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ impl EditNode {
8787
set.insert(value.render()?);
8888
enforce_limit()?;
8989
}
90-
Value::Set(crate::Rc::new(set))
90+
Value::from_set(set)
9191
}
9292
})
9393
}

src/builtins/sets.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ use crate::lexer::Span;
1010
use crate::value::Value;
1111
use crate::*;
1212

13-
use alloc::collections::BTreeSet;
14-
1513
use anyhow::{bail, Result};
1614

1715
pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) {
@@ -24,19 +22,19 @@ pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn
2422
pub fn intersection(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
2523
let s1 = ensure_set("intersection", expr1, v1)?;
2624
let s2 = ensure_set("intersection", expr2, v2)?;
27-
Ok(Value::from_set(s1.intersection(&s2).cloned().collect()))
25+
Ok(Value::from(s1.intersection(&s2)))
2826
}
2927

3028
pub fn union(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
3129
let s1 = ensure_set("union", expr1, v1)?;
3230
let s2 = ensure_set("union", expr2, v2)?;
33-
Ok(Value::from_set(s1.union(&s2).cloned().collect()))
31+
Ok(Value::from(s1.union(&s2)))
3432
}
3533

3634
pub fn difference(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
3735
let s1 = ensure_set("difference", expr1, v1)?;
3836
let s2 = ensure_set("difference", expr2, v2)?;
39-
Ok(Value::from_set(s1.difference(&s2).cloned().collect()))
37+
Ok(Value::from(s1.difference(&s2)))
4038
}
4139

4240
fn binary_set_union(
@@ -49,7 +47,7 @@ fn binary_set_union(
4947
ensure_args_count(span, name, params, args, 2)?;
5048
let left = ensure_set(name, &params[0], args[0].clone())?;
5149
let right = ensure_set(name, &params[1], args[1].clone())?;
52-
Ok(Value::from_set(left.union(&right).cloned().collect()))
50+
Ok(Value::from(left.union(&right)))
5351
}
5452

5553
fn binary_set_intersection(
@@ -62,9 +60,7 @@ fn binary_set_intersection(
6260
ensure_args_count(span, name, params, args, 2)?;
6361
let left = ensure_set(name, &params[0], args[0].clone())?;
6462
let right = ensure_set(name, &params[1], args[1].clone())?;
65-
Ok(Value::from_set(
66-
left.intersection(&right).cloned().collect(),
67-
))
63+
Ok(Value::from(left.intersection(&right)))
6864
}
6965

7066
fn intersection_of_set_of_sets(
@@ -77,7 +73,7 @@ fn intersection_of_set_of_sets(
7773
ensure_args_count(span, name, params, args, 1)?;
7874
let set = ensure_set(name, &params[0], args[0].clone())?;
7975

80-
let mut res = BTreeSet::new();
76+
let mut res = crate::value::Set::new();
8177
let mut first = true;
8278

8379
for s in set.iter() {
@@ -92,11 +88,11 @@ fn intersection_of_set_of_sets(
9288
res.clone_from(s);
9389
first = false;
9490
} else {
95-
res = res.intersection(s).cloned().collect();
91+
res = res.intersection(s);
9692
}
9793
}
9894

99-
Ok(Value::from_set(res))
95+
Ok(Value::from(res))
10096
}
10197

10298
fn union_of_set_of_sets(
@@ -109,7 +105,7 @@ fn union_of_set_of_sets(
109105
ensure_args_count(span, name, params, args, 1)?;
110106
let set = ensure_set(name, &params[0], args[0].clone())?;
111107

112-
let mut res = BTreeSet::new();
108+
let mut res = crate::value::Set::new();
113109

114110
for s in set.iter() {
115111
let s = match s {
@@ -119,8 +115,8 @@ fn union_of_set_of_sets(
119115
),
120116
};
121117

122-
res = res.union(s).cloned().collect();
118+
res = res.union(s);
123119
}
124120

125-
Ok(Value::from_set(res))
121+
Ok(Value::from(res))
126122
}

src/builtins/utils.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,11 @@
55
use crate::ast::{Expr, Ref};
66
use crate::lexer::Span;
77
use crate::number::Number;
8-
use crate::value::Object;
8+
use crate::value::{Object, Set};
99
use crate::Rc;
1010
use crate::Value;
1111
use crate::*;
1212

13-
use alloc::collections::BTreeSet;
14-
1513
use anyhow::{bail, Result};
1614

1715
#[inline]
@@ -159,7 +157,7 @@ pub fn ensure_array(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Vec<Value>>> {
159157
})
160158
}
161159

162-
pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeSet<Value>>> {
160+
pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Set>> {
163161
Ok(match v {
164162
Value::Set(s) => s,
165163
_ => {

src/languages/azure_policy/compiler/metadata.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::languages::azure_policy::ast::{
1717
Condition, EffectKind, FieldKind, JsonValue, Lhs, OperatorKind, PolicyDefinition, PolicyRule,
1818
ValueOrExpr,
1919
};
20-
use crate::{Rc, Value};
20+
use crate::Value;
2121

2222
use super::core::Compiler;
2323

@@ -237,7 +237,7 @@ impl Compiler {
237237
.iter()
238238
.map(|p| Value::String(p.name.as_str().into()))
239239
.collect();
240-
annot.insert("parameter_names".to_string(), Value::Set(Rc::new(set)));
240+
annot.insert("parameter_names".to_string(), Value::from_set(set));
241241
}
242242

243243
// Extra fields: policyType → policy_type, id → policy_id, name → policy_name.
@@ -279,6 +279,6 @@ fn insert_string_set_annotation(
279279
.iter()
280280
.map(|s| Value::String(s.as_str().into()))
281281
.collect();
282-
annot.insert(key.to_string(), Value::Set(Rc::new(set)));
282+
annot.insert(key.to_string(), Value::from_set(set));
283283
}
284284
}

src/languages/azure_rbac/builtins/lists.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
use alloc::collections::BTreeSet;
5-
6-
use crate::value::Value;
4+
use crate::value::{Set, Value};
75

86
use super::evaluator::RbacBuiltinError;
97

@@ -28,7 +26,7 @@ fn list_contains_values(list: &[Value], needle: &Value) -> bool {
2826
}
2927

3028
// For sets, treat a list/set needle as "all elements are contained".
31-
fn set_contains_values(set: &BTreeSet<Value>, needle: &Value) -> bool {
29+
fn set_contains_values(set: &Set, needle: &Value) -> bool {
3230
match *needle {
3331
// For collection needles, require all elements to be present.
3432
Value::Array(ref right_list) => right_list.iter().all(|item| set.contains(item)),

src/languages/rego/compiler/expressions/collection_literals.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ pub(in crate::languages::rego::compiler) fn try_eval_const(expr: &Expr) -> Optio
4040
.iter()
4141
.map(|i| try_eval_const(i.as_ref()))
4242
.collect::<Option<BTreeSet<_>>>()
43-
.map(|s| Value::Set(Rc::new(s))),
43+
.map(Value::from_set),
4444
Expr::Object { fields, .. } => fields
4545
.iter()
4646
.map(|(_, k, v)| Some((try_eval_const(k.as_ref())?, try_eval_const(v.as_ref())?)))
@@ -92,7 +92,7 @@ impl<'a> Compiler<'a> {
9292
items.iter().map(|i| try_eval_const(i.as_ref())).collect();
9393
if let Some(values) = all_const {
9494
let dest = self.alloc_register();
95-
let literal_idx = self.add_literal(Value::Set(Rc::new(values)));
95+
let literal_idx = self.add_literal(Value::from_set(values));
9696
self.emit_instruction(Instruction::Load { dest, literal_idx }, span);
9797
return Ok(dest);
9898
}

src/rvm/program/metadata.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ impl MetadataValue {
173173
.collect(),
174174
)
175175
} else {
176-
MetadataValue::List(set.iter().map(MetadataValue::from_value).collect())
176+
MetadataValue::List(set.iter_sorted().map(MetadataValue::from_value).collect())
177177
}
178178
}
179179
Value::Object(ref obj) => {
@@ -202,7 +202,7 @@ impl MetadataValue {
202202
for s in set {
203203
bset.insert(Value::String(s.as_str().into()));
204204
}
205-
Value::Set(Rc::new(bset))
205+
Value::from_set(bset)
206206
}
207207
MetadataValue::Bool(b) => Value::Bool(b),
208208
MetadataValue::Integer(n) => Value::from(n),
@@ -301,7 +301,7 @@ mod tests {
301301
let mut set = BTreeSet::new();
302302
set.insert(Value::String("a".into()));
303303
set.insert(Value::String("b".into()));
304-
let v = Value::Set(Rc::new(set));
304+
let v = Value::from_set(set);
305305
assert_round_trip(&v, &v);
306306
}
307307

@@ -328,11 +328,14 @@ mod tests {
328328
let mut set = BTreeSet::new();
329329
set.insert(Value::String("a".into()));
330330
set.insert(Value::from(1_i64));
331-
let v = Value::Set(Rc::new(set));
331+
let v = Value::from_set(set);
332332
let mv = MetadataValue::from_value(&v);
333-
assert!(
334-
matches!(mv, MetadataValue::List(_)),
335-
"mixed-type set should produce List, got {mv:?}"
333+
assert_eq!(
334+
mv,
335+
MetadataValue::List(alloc::vec![
336+
MetadataValue::Integer(1),
337+
MetadataValue::String("a".into()),
338+
])
336339
);
337340
}
338341

src/rvm/program/serialization/value.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use serde::ser::{SerializeSeq as _, SerializeTuple as _};
1111
use serde::{Deserialize, Serialize};
1212

1313
use crate::number::Number;
14-
use crate::value::Object;
1514
use crate::value::Value;
15+
use crate::value::{Object, Set};
1616

1717
const VARIANT_NULL: u32 = 0;
1818
const VARIANT_BOOL: u32 = 1;
@@ -118,15 +118,15 @@ impl<'a> Serialize for BinaryValueSlice<'a> {
118118
}
119119
}
120120

121-
struct BinarySetRef<'a>(&'a BTreeSet<Value>);
121+
struct BinarySetRef<'a>(&'a Set);
122122

123123
impl<'a> Serialize for BinarySetRef<'a> {
124124
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
125125
where
126126
S: serde::Serializer,
127127
{
128128
let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
129-
for value in self.0.iter() {
129+
for value in self.0.iter_sorted() {
130130
seq.serialize_element(&BinaryValueRef(value))?;
131131
}
132132
seq.end()

src/rvm/vm/arithmetic.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ impl RegoVM {
3030
match (a, b) {
3131
(&Value::Number(ref x), &Value::Number(ref y)) => Ok(Value::from(x.sub(y)?)),
3232
(&Value::Set(ref left), &Value::Set(ref right)) => {
33-
let diff: alloc::collections::BTreeSet<Value> =
34-
left.difference(right).cloned().collect();
35-
Ok(Value::from(diff))
33+
Ok(Value::from(left.difference(right)))
3634
}
3735
_ => Err(VmError::InvalidSubtraction {
3836
left: a.clone(),

0 commit comments

Comments
 (0)