diff --git a/crates/fj-core/src/objects/mod.rs b/crates/fj-core/src/objects/mod.rs index 27babb14ce..1b6bed7a91 100644 --- a/crates/fj-core/src/objects/mod.rs +++ b/crates/fj-core/src/objects/mod.rs @@ -33,6 +33,6 @@ pub use self::{ surface::Surface, vertex::Vertex, }, - object_set::ObjectSet, + object_set::{ObjectSet, ObjectSetIntoIter, ObjectSetIter}, stores::{Objects, Surfaces}, }; diff --git a/crates/fj-core/src/objects/object_set.rs b/crates/fj-core/src/objects/object_set.rs index 3f4a7a2118..9fb8f61d5a 100644 --- a/crates/fj-core/src/objects/object_set.rs +++ b/crates/fj-core/src/objects/object_set.rs @@ -1,8 +1,8 @@ -use std::{collections::BTreeSet, fmt::Debug, slice, vec}; +use std::{collections::BTreeSet, fmt::Debug, iter, slice, vec}; use itertools::Itertools; -use crate::storage::Handle; +use crate::storage::{Handle, HandleWrapper}; /// An ordered set of objects /// @@ -19,7 +19,7 @@ pub struct ObjectSet { // structure (since it is used in objects, and objects themselves are // immutable). We need to make sure there are no duplicates when this is // constructed (see the constructor below), but after that, we're fine. - inner: Vec>, + inner: Vec>, } impl ObjectSet { @@ -36,6 +36,8 @@ impl ObjectSet { let mut inner = Vec::new(); for handle in handles { + let handle = HandleWrapper::from(handle); + if added.contains(&handle) { panic!( "Constructing `ObjectSet` with duplicate handle: {:?}", @@ -97,7 +99,7 @@ impl ObjectSet { /// Return the n-th item pub fn nth(&self, index: usize) -> Option<&Handle> { - self.inner.get(index) + self.inner.get(index).map(|wrapper| &wrapper.0) } /// Return the n-th item, treating the index space as circular @@ -130,8 +132,8 @@ impl ObjectSet { } /// Access an iterator over the objects - pub fn iter(&self) -> slice::Iter> { - self.inner.iter() + pub fn iter(&self) -> ObjectSetIter { + self.inner.iter().map(HandleWrapper::as_handle) } /// Access an iterator over the neighboring pairs of all contained objects @@ -199,28 +201,63 @@ where } } +impl<'r, T> IntoIterator for &'r ObjectSet { + type Item = &'r Handle; + type IntoIter = ObjectSetIter<'r, T>; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + impl IntoIterator for ObjectSet { type Item = Handle; - type IntoIter = vec::IntoIter>; + type IntoIter = ObjectSetIntoIter; fn into_iter(self) -> Self::IntoIter { - self.inner.into_iter() + self.inner.into_iter().map(HandleWrapper::into_handle) } } -impl<'r, T> IntoIterator for &'r ObjectSet { - // You might wonder why we're returning references to `Handle`s here, when - // `Handle` already is a kind of reference, and easily cloned. - // - // Most of the time that doesn't make a difference, but there are use cases - // where dealing with owned `Handle`s is inconvenient, for example when - // using iterator adapters. You can't return a reference to the argument of - // an adapter's closure, if you own that argument. You can, if you just - // reference the argument. - type Item = &'r Handle; - type IntoIter = slice::Iter<'r, Handle>; +/// An borrowed iterator over an [`ObjectSet`] +/// +/// See [`ObjectSet::iter`]. +pub type ObjectSetIter<'r, T> = iter::Map< + slice::Iter<'r, HandleWrapper>, + fn(&HandleWrapper) -> &Handle, +>; - fn into_iter(self) -> Self::IntoIter { - self.iter() +/// An owned iterator over an [`ObjectSet`] +/// +/// You might wonder why we're returning references to `Handle`s here, when +/// `Handle` already is a kind of reference, and easily cloned. +/// +/// Most of the time that doesn't make a difference, but there are use cases +/// where dealing with owned `Handle`s is inconvenient, for example when using +/// iterator adapters. You can't return a reference to the argument of an +/// adapter's closure, if you own that argument. You can, if you just reference +/// the argument. +pub type ObjectSetIntoIter = iter::Map< + vec::IntoIter>, + fn(HandleWrapper) -> Handle, +>; + +#[cfg(test)] +mod tests { + use crate::{objects::Cycle, operations::insert::Insert, Core}; + + use super::ObjectSet; + + #[test] + fn equal_but_not_identical_objects() { + let mut core = Core::new(); + + let bare_cycle = Cycle::new([]); + let cycle_a = bare_cycle.clone().insert(&mut core); + let cycle_b = bare_cycle.insert(&mut core); + + let _object_set = ObjectSet::new([cycle_a, cycle_b]); + // Nothing more to test. `ObjectSet` panics on duplicate objects, and it + // shouldn't do that in this case. } } diff --git a/crates/fj-core/src/storage/handle.rs b/crates/fj-core/src/storage/handle.rs index b6d99f3b60..9c2a9420c9 100644 --- a/crates/fj-core/src/storage/handle.rs +++ b/crates/fj-core/src/storage/handle.rs @@ -235,6 +235,18 @@ impl fmt::Debug for ObjectId { /// is the purpose of `HandleWrapper`. pub struct HandleWrapper(pub Handle); +impl HandleWrapper { + /// Convert `&self` into a `&Handle` + pub fn as_handle(&self) -> &Handle { + &self.0 + } + + /// Convert `self` into a `Handle` + pub fn into_handle(self) -> Handle { + self.0 + } +} + impl Deref for HandleWrapper { type Target = Handle;