Skip to content

Commit 8990347

Browse files
stepanchegfacebook-github-bot
authored andcommitted
SetFromValue utility
Summary: Refactoring for D63739933. Reviewed By: perehonchuk Differential Revision: D63739932 fbshipit-source-id: e41bc041abd57db9cfbdb94779f6f7086aee428a
1 parent 01639e2 commit 8990347

File tree

1 file changed

+52
-26
lines changed

1 file changed

+52
-26
lines changed

starlark/src/values/types/set/methods.rs

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use std::mem;
2121

2222
use starlark_derive::starlark_module;
2323
use starlark_map::small_set::SmallSet;
24+
use starlark_map::Hashed;
2425
use starlark_syntax::value_error;
2526

2627
use crate as starlark;
@@ -34,6 +35,43 @@ use crate::values::Heap;
3435
use crate::values::Value;
3536
use crate::values::ValueOfUnchecked;
3637

38+
enum SetFromValue<'v> {
39+
Set(SmallSet<Value<'v>>),
40+
}
41+
42+
impl<'v> SetFromValue<'v> {
43+
fn from_value(
44+
value: ValueOfUnchecked<'v, StarlarkIter<Value<'v>>>,
45+
heap: &'v Heap,
46+
) -> crate::Result<Self> {
47+
let mut set = SmallSet::default();
48+
for elem in value.get().iterate(heap)? {
49+
set.insert_hashed(elem.get_hashed()?);
50+
}
51+
Ok(SetFromValue::Set(set))
52+
}
53+
54+
fn get(&self) -> &SmallSet<Value<'v>> {
55+
match self {
56+
SetFromValue::Set(set) => set,
57+
}
58+
}
59+
60+
fn into_set(self) -> SmallSet<Value<'v>> {
61+
match self {
62+
SetFromValue::Set(set) => set,
63+
}
64+
}
65+
66+
fn is_empty(&self) -> bool {
67+
self.get().is_empty()
68+
}
69+
70+
fn contains_hashed(&self, value: Hashed<Value<'v>>) -> bool {
71+
self.get().contains_hashed(value.as_ref())
72+
}
73+
}
74+
3775
#[starlark_module]
3876
pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
3977
fn clear(this: Value) -> anyhow::Result<NoneType> {
@@ -81,18 +119,14 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
81119
heap: &'v Heap,
82120
) -> starlark::Result<SetData<'v>> {
83121
//TODO(romanp) check if other is set
84-
let other_it = other.get().iterate(heap)?;
85-
let mut other_set = SmallSet::default();
86-
for elem in other_it {
87-
other_set.insert_hashed(elem.get_hashed()?);
88-
}
122+
let other_set = SetFromValue::from_value(other, heap)?;
89123
let mut data = SetData::default();
90124
if other_set.is_empty() {
91125
return Ok(data);
92126
}
93127

94128
for hashed in this.aref.content.iter_hashed() {
95-
if other_set.contains_hashed(hashed) {
129+
if other_set.contains_hashed(hashed.copied()) {
96130
data.content.insert_hashed_unique_unchecked(hashed.copied());
97131
}
98132
}
@@ -113,11 +147,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
113147
heap: &'v Heap,
114148
) -> starlark::Result<SetData<'v>> {
115149
//TODO(romanp) check if other is set
116-
let other_it = other.get().iterate(heap)?;
117-
let mut other_set: SmallSet<Value<'v>> = SmallSet::default();
118-
for elem in other_it {
119-
other_set.insert_hashed(elem.get_hashed()?);
120-
}
150+
let other_set = SetFromValue::from_value(other, heap)?;
121151

122152
if other_set.is_empty() {
123153
return Ok(SetData {
@@ -127,17 +157,19 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
127157

128158
//TODO(romanp) add symmetric_difference to small set and use it here and in xor
129159
if this.aref.content.is_empty() {
130-
return Ok(SetData { content: other_set });
160+
return Ok(SetData {
161+
content: other_set.into_set(),
162+
});
131163
}
132164

133165
let mut data = SetData::default();
134166
for elem in this.aref.content.iter_hashed() {
135-
if !other_set.contains_hashed(elem) {
167+
if !other_set.contains_hashed(elem.copied()) {
136168
data.add_hashed(elem.copied());
137169
}
138170
}
139171

140-
for elem in other_set {
172+
for elem in other_set.get() {
141173
let hashed = elem.get_hashed()?;
142174
if !this.aref.content.contains_hashed(hashed.as_ref()) {
143175
data.add_hashed(hashed);
@@ -276,16 +308,13 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
276308
heap: &'v Heap,
277309
) -> starlark::Result<SetData<'v>> {
278310
//TODO(romanp) check if other is set
279-
let other_it = other.get().iterate(heap)?;
280311

281312
if this.aref.content.is_empty() {
313+
other.get().iterate(heap)?;
282314
return Ok(SetData::default());
283315
}
284316

285-
let mut other_set: SmallSet<Value<'v>> = SmallSet::default();
286-
for elem in other_it {
287-
other_set.insert_hashed(elem.get_hashed()?);
288-
}
317+
let other_set = SetFromValue::from_value(other, heap)?;
289318

290319
if other_set.is_empty() {
291320
return Ok(SetData {
@@ -295,7 +324,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
295324

296325
let mut data = SetData::default();
297326
for elem in this.aref.content.iter_hashed() {
298-
if !other_set.contains_hashed(elem) {
327+
if !other_set.contains_hashed(elem.copied()) {
299328
data.add_hashed(elem.copied());
300329
}
301330
}
@@ -339,20 +368,17 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
339368
#[starlark(require=pos)] other: ValueOfUnchecked<'v, StarlarkIter<Value<'v>>>,
340369
heap: &'v Heap,
341370
) -> starlark::Result<bool> {
342-
let other = other.get().iterate(heap)?;
343371
if this.aref.content.is_empty() {
372+
other.get().iterate(heap)?;
344373
return Ok(true);
345374
}
346375
//TODO(romanp): check if other is already a set
347-
let mut rhs: SmallSet<Value<'v>> = SmallSet::default();
348-
for elem in other {
349-
rhs.insert_hashed(elem.get_hashed()?);
350-
}
376+
let rhs = SetFromValue::from_value(other, heap)?;
351377
if rhs.is_empty() {
352378
return Ok(false);
353379
}
354380
for elem in this.aref.content.iter_hashed() {
355-
if !rhs.contains_hashed(elem) {
381+
if !rhs.contains_hashed(elem.copied()) {
356382
return Ok(false);
357383
}
358384
}

0 commit comments

Comments
 (0)