Skip to content

Commit

Permalink
make types private & remove derefs
Browse files Browse the repository at this point in the history 
Summary: as requested by stepancheg

Reviewed By: stepancheg

Differential Revision: D63640110

fbshipit-source-id: 952b43fd336bc22ca96ccc0862f1eae7929107e8
  • Loading branch information
@perehonchuk @facebook-github-bot
perehonchuk authored and facebook-github-bot committed Oct 1, 2024
1 parent b9d428d commit f08ec19
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 62 deletions.
3 changes: 2 additions & 1 deletion starlark/src/values/types/set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ pub(crate) mod methods;
pub(crate) mod refs;
pub(crate) mod set;
pub(crate) mod value;
pub use crate::values::set::value::Set;
pub use crate::values::set::refs::SetMut;
pub use crate::values::set::refs::SetRef;
36 changes: 18 additions & 18 deletions starlark/src/values/types/set/methods.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::values::ValueOfUnchecked;
pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
fn clear(this: Value) -> anyhow::Result<NoneType> {
let mut this = SetMut::from_value(this)?;
this.clear();
this.aref.clear();
Ok(NoneType)
}

Expand All @@ -58,7 +58,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
) -> starlark::Result<SetData<'v>> {
let it = other.get().iterate(heap)?;
// TODO(romanp) omptimize if this is empty
let mut data = this.content.clone();
let mut data = this.aref.content.clone();
for elem in it {
let hashed = elem.get_hashed()?;
data.insert_hashed(hashed);
Expand Down Expand Up @@ -91,7 +91,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
return Ok(data);
}

for hashed in this.content.iter_hashed() {
for hashed in this.aref.content.iter_hashed() {
if other_set.contains_hashed(hashed) {
data.content.insert_hashed_unique_unchecked(hashed.copied());
}
Expand Down Expand Up @@ -121,25 +121,25 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {

if other_set.is_empty() {
return Ok(SetData {
content: this.content.clone(),
content: this.aref.content.clone(),
});
}

//TODO(romanp) add symmetric_difference to small set and use it here and in xor
if this.content.is_empty() {
if this.aref.content.is_empty() {
return Ok(SetData { content: other_set });
}

let mut data = SetData::default();
for elem in this.content.iter_hashed() {
for elem in this.aref.content.iter_hashed() {
if !other_set.contains_hashed(elem) {
data.add_hashed(elem.copied());
}
}

for elem in other_set {
let hashed = elem.get_hashed()?;
if !this.content.contains_hashed(hashed.as_ref()) {
if !this.aref.content.contains_hashed(hashed.as_ref()) {
data.add_hashed(hashed);
}
}
Expand All @@ -160,7 +160,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
) -> starlark::Result<NoneType> {
let mut this = SetMut::from_value(this)?;
let hashed = value.get_hashed()?;
this.add_hashed(hashed);
this.aref.add_hashed(hashed);
Ok(NoneType)
}

Expand Down Expand Up @@ -192,7 +192,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
) -> starlark::Result<NoneType> {
let mut set = SetMut::from_value(this)?;
let hashed = value.get_hashed()?;
if set.remove_hashed(hashed.as_ref()) {
if set.aref.remove_hashed(hashed.as_ref()) {
Ok(NoneType)
} else {
mem::drop(set);
Expand Down Expand Up @@ -228,7 +228,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
) -> starlark::Result<NoneType> {
let mut set = SetMut::from_value(this)?;
let hashed = value.get_hashed()?;
set.remove_hashed(hashed.as_ref());
set.aref.remove_hashed(hashed.as_ref());
Ok(NoneType)
}

Expand All @@ -252,10 +252,10 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
/// ```
fn pop<'v>(this: Value<'v>) -> starlark::Result<Value<'v>> {
let mut set = SetMut::from_value(this)?;
let first = set.iter_hashed().next();
let first = set.aref.iter_hashed().next();
match first {
Some(x) => {
set.remove_hashed(x.as_ref());
set.aref.remove_hashed(x.as_ref());
Ok(x.into_key())
}
None => Err(value_error!("pop from an empty set")),
Expand All @@ -278,7 +278,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
//TODO(romanp) check if other is set
let other_it = other.get().iterate(heap)?;

if this.content.is_empty() {
if this.aref.content.is_empty() {
return Ok(SetData::default());
}

Expand All @@ -289,12 +289,12 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {

if other_set.is_empty() {
return Ok(SetData {
content: this.content.clone(),
content: this.aref.content.clone(),
});
}

let mut data = SetData::default();
for elem in this.content.iter_hashed() {
for elem in this.aref.content.iter_hashed() {
if !other_set.contains_hashed(elem) {
data.add_hashed(elem.copied());
}
Expand All @@ -319,7 +319,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
//TODO (romanp) skip if other is larger
for elem in other {
let hashed = elem.get_hashed()?;
if !this.contains_hashed(hashed) {
if !this.aref.contains_hashed(hashed) {
return Ok(false);
}
}
Expand All @@ -340,7 +340,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
heap: &'v Heap,
) -> starlark::Result<bool> {
let other = other.get().iterate(heap)?;
if this.content.is_empty() {
if this.aref.content.is_empty() {
return Ok(true);
}
//TODO(romanp): check if other is already a set
Expand All @@ -351,7 +351,7 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
if rhs.is_empty() {
return Ok(false);
}
for elem in this.content.iter_hashed() {
for elem in this.aref.content.iter_hashed() {
if !rhs.contains_hashed(elem) {
return Ok(false);
}
Expand Down
25 changes: 1 addition & 24 deletions starlark/src/values/types/set/refs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ use std::cell::Ref;
use std::cell::RefCell;
use std::cell::RefMut;
use std::convert::Infallible;
use std::ops::Deref;
use std::ops::DerefMut;

use dupe::Dupe;
use either::Either;
Expand All @@ -38,18 +36,11 @@ use crate::values::UnpackValue;
use crate::values::Value;
use crate::values::ValueError;

/// Define the set type.
pub struct SetRef<'v> {
pub(crate) aref: Either<Ref<'v, SetData<'v>>, &'v SetData<'v>>,
}

impl<'v> Deref for SetRef<'v> {
type Target = SetData<'v>;

fn deref(&self) -> &Self::Target {
&self.aref
}
}

impl<'v> Clone for SetRef<'v> {
fn clone(&self) -> Self {
match &self.aref {
Expand Down Expand Up @@ -99,20 +90,6 @@ impl<'v> SetMut<'v> {
}
}

impl<'v> Deref for SetMut<'v> {
type Target = SetData<'v>;

fn deref(&self) -> &Self::Target {
&self.aref
}
}

impl<'v> DerefMut for SetMut<'v> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.aref
}
}

impl<'v> StarlarkTypeRepr for SetRef<'v> {
type Canonical = <SetType<FrozenValue> as StarlarkTypeRepr>::Canonical;

Expand Down
2 changes: 1 addition & 1 deletion starlark/src/values/types/set/set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn register_set(globals: &mut GlobalsBuilder) {
) -> starlark::Result<SetData<'v>> {
let set = match arg {
Some(pos) => match SetRef::unpack_value_opt(pos.get()) {
Some(set) => (*set).clone(),
Some(set) => (set.aref).clone(),
None => {
let it = pos.get().iterate(heap)?;
let mut data = SetData::default();
Expand Down
31 changes: 14 additions & 17 deletions starlark/src/values/types/set/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ use crate::StarlarkDocs;
)]
#[starlark_docs(builtin = "standard")]
#[repr(transparent)]
pub struct SetGen<T>(pub(crate) T);
pub(crate) struct SetGen<T>(pub(crate) T);

/// Define the mutable set type.
#[derive(Default, Trace, Debug, ProvidesStaticType, Allocative, Clone)]
pub struct SetData<'v> {
pub(crate) struct SetData<'v> {
/// The data stored by the list.
pub(crate) content: SmallSet<Value<'v>>,
}
Expand Down Expand Up @@ -113,17 +113,14 @@ impl<'v> SetData<'v> {

#[derive(Clone, Default, Debug, ProvidesStaticType, Allocative)]
#[repr(transparent)]
pub struct FrozenSetData {
pub(crate) struct FrozenSetData {
/// The data stored by the set. The values must all be hashable values.
content: SmallSet<FrozenValue>,
}

/// Define the set type.
pub type Set<'v> = SetGen<SetData<'v>>;
pub(crate) type MutableSet<'v> = SetGen<RefCell<SetData<'v>>>;

pub type MutableSet<'v> = SetGen<RefCell<SetData<'v>>>;

pub type FrozenSet = SetGen<FrozenSetData>;
pub(crate) type FrozenSet = SetGen<FrozenSetData>;

impl<'v> AllocValue<'v> for SetData<'v> {
fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
Expand Down Expand Up @@ -231,7 +228,7 @@ where
fn equals(&self, other: Value<'v>) -> crate::Result<bool> {
match SetRef::unpack_value_opt(other) {
None => Ok(false),
Some(other) => Ok(equals_small_set(&self.0.content(), &other.content)),
Some(other) => Ok(equals_small_set(&self.0.content(), &other.aref.content)),
}
}

Expand Down Expand Up @@ -268,10 +265,10 @@ where
let rhs = SetRef::unpack_value_opt(rhs)
.map_or_else(|| ValueError::unsupported_with(self, "|", rhs), Ok)?;
if self.0.content().is_empty() {
return Ok(heap.alloc((*rhs).clone()));
return Ok(heap.alloc((*rhs.aref).clone()));
}
let mut items = self.0.content().clone();
for h in rhs.iter_hashed() {
for h in rhs.aref.iter_hashed() {
items.insert_hashed(h);
}
Ok(heap.alloc(SetData { content: items }))
Expand All @@ -286,7 +283,7 @@ where
let rhs = SetRef::unpack_value_opt(rhs)
.map_or_else(|| ValueError::unsupported_with(self, "&", rhs), Ok)?;
for h in rhs.iter_hashed() {
for h in rhs.aref.iter_hashed() {
if self.0.content().contains_hashed(h.as_ref()) {
items.insert_hashed_unique_unchecked(h);
}
Expand All @@ -299,19 +296,19 @@ where
fn bit_xor(&self, rhs: Value<'v>, heap: &'v Heap) -> crate::Result<Value<'v>> {
let rhs = SetRef::unpack_value_opt(rhs)
.map_or_else(|| ValueError::unsupported_with(self, "^", rhs), Ok)?;
if rhs.content.is_empty() {
if rhs.aref.content.is_empty() {
return Ok(heap.alloc(SetData {
content: self.0.content().clone(),
}));
}
let mut data = SetData::default();
for elem in self.0.content().iter_hashed() {
if !rhs.contains_hashed(elem.copied()) {
if !rhs.aref.contains_hashed(elem.copied()) {
data.add_hashed_unique_unchecked(elem.copied());
}
}

for hashed in rhs.iter_hashed() {
for hashed in rhs.aref.iter_hashed() {
if !self.0.content().contains_hashed(hashed.as_ref()) {
data.add_hashed(hashed);
}
Expand All @@ -331,7 +328,7 @@ where
}));
}

if rhs.content.is_empty() {
if rhs.aref.content.is_empty() {
return Ok(heap.alloc(SetData {
content: self.0.content().clone(),
}));
Expand All @@ -340,7 +337,7 @@ where
let mut data = SetData::default();

for elem in self.0.content().iter_hashed() {
if !rhs.contains_hashed(elem.copied()) {
if !rhs.aref.contains_hashed(elem.copied()) {
data.add_hashed(elem.copied());
}
}
Expand Down
2 changes: 1 addition & 1 deletion starlark/src/values/typing/type_compiled/matchers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub(crate) struct IsSetOf<I: TypeMatcher>(pub(crate) I);
impl<I: TypeMatcher> TypeMatcher for IsSetOf<I> {
fn matches(&self, value: Value) -> bool {
match SetRef::unpack_value_opt(value) {
Some(set) => set.iter().all(|v| self.0.matches(v)),
Some(set) => set.aref.iter().all(|v| self.0.matches(v)),
_ => false,
}
}
Expand Down

0 comments on commit f08ec19

Please sign in to comment.