Skip to content

Commit

Permalink
BREAKING: replace im:Vector with std::VecDeque and remove im
Browse files Browse the repository at this point in the history
Replace uses of `im::Vector` with `std::collections::VecDeque`, and
remove `im` as a dependency.

The reasons for this change are:

* `std` collections are much more common, and are used in `conjure_oxide`.

  Currently I find myself using `.into_iter().collect()` after every
  Uniplate call to turn things back into `std` types. Using `std` types
  is much more common than `im`, so making this use-case easier should
  be a priority.

  If needed down the line, implementations of Uniplate using non-std
  data structures will be added as separate traits in a sub-module (e.g.
  `uniplate::im::Uniplate`)

* `im` seems to not be actively maintained

* It would be better for `uniplate` not to unnecessarily rely on types
  from external APIs which could change / become abandoned.

* For the derive-macro, we currently re-export `im::Vector`. This makes
  compile errors for Uniplate messy, as they often reference the
  undocumented module `uniplate::_dependencies::im`.

I don't think using persistent data-structures (why I originally used
im) improved performance that much. The main performance bottlenecks
seems to instead be clones and heap allocations with `Box`.
  • Loading branch information
niklasdewally committed Jan 12, 2025
1 parent ddd5f4e commit 7b8f19f
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 137 deletions.
58 changes: 1 addition & 57 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion uniplate-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ fn _derive_children(state: &mut ParserState, fields: &[ast::Field]) -> TokenStre
0 => quote! {let children = ::uniplate::Tree::Zero;},
_ => {
let subtrees = subtrees.iter();
quote! {let children = ::uniplate::Tree::Many(::uniplate::_dependencies::im::vector![#(#subtrees),*]);}
quote! {let children = ::uniplate::Tree::Many(::std::collections::VecDeque::from([#(#subtrees),*]));}
}
}
}
Expand Down
1 change: 0 additions & 1 deletion uniplate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ categories = ["rust-patterns", "data-structures", "algorithms"]
[lib]

[dependencies]
im = { version = "15.1.0", features = ["proptest"] }
proptest = "1.6.0"
proptest-derive = "0.5.1"
thiserror = "2.0.9"
Expand Down
2 changes: 0 additions & 2 deletions uniplate/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// this and/or devirtualise the Box<dyn Fn()> when necessary to make this fast.
// https://users.rust-lang.org/t/why-box-dyn-fn-is-the-same-fast-as-normal-fn/96392

use im::Vector;
use std::collections::VecDeque;

use crate::derive_iter;
Expand All @@ -35,4 +34,3 @@ derive_unplateable!(String);

derive_iter!(Vec);
derive_iter!(VecDeque);
derive_iter!(Vector);
7 changes: 1 addition & 6 deletions uniplate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ pub mod derive {
pub use uniplate_derive::Uniplate;
}

#[doc(hidden)]
pub mod _dependencies {
pub use im;
}

/// Generates [`Biplate`] and [`Uniplate`] instances for an unplateable type.
///
/// An unplateable type is one that you don't want Uniplate to traverse inside of.
Expand Down Expand Up @@ -188,7 +183,7 @@ macro_rules! derive_iter {
}

// T != F: return all type T's contained in the type F's in the vector
let mut child_trees: im::Vector<::uniplate::Tree<T>> = im::Vector::new();
let mut child_trees: VecDeque<::uniplate::Tree<T>> = VecDeque::new();
let mut child_ctxs: Vec<Box<dyn Fn(::uniplate::Tree<T>) -> F>> = Vec::new();
for item in self {
let (tree, plate) = <F as ::uniplate::Biplate<T>>::biplate(item);
Expand Down
4 changes: 3 additions & 1 deletion uniplate/src/test_common/paper.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::derive::Uniplate;

#[cfg(test)]
use crate::Uniplate;
use proptest::prelude::*;
Expand Down Expand Up @@ -76,12 +77,13 @@ proptest! {

#[test]
fn uniplate_children(ast in proptest_stmts(), new_children in proptest::collection::vec(proptest_stmts(),1..=10)) {
use std::collections::VecDeque;
let original_children = ast.children();
prop_assume!(original_children.len() == new_children.len());

let mut ast = ast.with_children(new_children.clone().into());

prop_assert_eq!(im::Vector::<Stmt>::from(new_children),ast.children());
prop_assert_eq!(VecDeque::<Stmt>::from(new_children),ast.children());

ast = ast.with_children(original_children.clone());
prop_assert_eq!(original_children,ast.children());
Expand Down
42 changes: 20 additions & 22 deletions uniplate/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

#![allow(clippy::type_complexity)]

use std::sync::Arc;
use std::{collections::VecDeque, sync::Arc};

pub use super::Tree;
use im::vector;

/// `Biplate<U>` for type `T` operates over all values of type `U` within `T`.
pub trait Biplate<To>
Expand All @@ -24,7 +23,7 @@ where
///
/// If there are a different number of children given as there were originally returned by
/// children().
fn with_children_bi(&self, children: im::Vector<To>) -> Self {
fn with_children_bi(&self, children: VecDeque<To>) -> Self {
// 1. Turn old tree into list.
// 2. Check lists are same size.
// 3. Use the reconstruction function given by old_children.list() to
Expand All @@ -45,7 +44,6 @@ where
/// is highly unlikely that this function should be used in the recursive case. A common
/// pattern is to first match the types using descendBi, then continue the recursion with
/// descend.
fn descend_bi(&self, op: Arc<dyn Fn(To) -> To>) -> Self {
let (children, ctx) = self.biplate();
ctx(children.map(op))
Expand All @@ -58,15 +56,15 @@ where
//
// https://github.com/ndmitchell/uniplate/blob/66a2c55a7de0f5d8b0e437479719469244e00fa4/Data/Generics/Uniplate/Internal/OperationsInc.hs#L189

fn universe_bi(&self) -> im::Vector<To> {
fn universe_bi(&self) -> VecDeque<To> {
self.children_bi()
.into_iter()
.flat_map(|child| child.universe())
.collect()
}

/// Returns the children of a type. If to == from then it returns the original element (in contrast to children).
fn children_bi(&self) -> im::Vector<To> {
fn children_bi(&self) -> VecDeque<To> {
self.biplate().0.list().0
}

Expand Down Expand Up @@ -115,16 +113,16 @@ where
}

/// Gets all children of a node, including itself and all children.
fn universe(&self) -> im::Vector<Self> {
let mut results = vector![self.clone()];
fn universe(&self) -> VecDeque<Self> {
let mut results = VecDeque::from([self.clone()]);
for child in self.children() {
results.append(child.universe());
results.append(&mut child.universe());
}
results
}

/// Gets the direct children (maximal substructures) of a node.
fn children(&self) -> im::Vector<Self> {
fn children(&self) -> VecDeque<Self> {
let (children, _) = self.uniplate();
children.list().0.clone()
}
Expand All @@ -135,7 +133,7 @@ where
///
/// If there are a different number of children given as there were originally returned by
/// children().
fn with_children(&self, children: im::Vector<Self>) -> Self {
fn with_children(&self, children: VecDeque<Self>) -> Self {
// 1. Turn old tree into list.
// 2. Check lists are same size.
// 3. Use the reconstruction function given by old_children.list() to
Expand Down Expand Up @@ -183,7 +181,7 @@ where
/// The meaning of the callback function is the following:
///
/// f(element_to_fold, folded_children) -> folded_element
fn cata<T>(&self, op: Arc<dyn Fn(Self, Vec<T>) -> T>) -> T {
fn cata<T>(&self, op: Arc<dyn Fn(Self, VecDeque<T>) -> T>) -> T {
let children = self.children();
(*op)(
self.clone(),
Expand Down Expand Up @@ -224,8 +222,8 @@ where
}

struct HolesIterator<T: Uniplate> {
children_iter: std::iter::Enumerate<im::vector::ConsumingIter<T>>,
children: im::vector::Vector<T>,
children_iter: std::iter::Enumerate<std::collections::vec_deque::IntoIter<T>>,
children: VecDeque<T>,
parent: T,
}

Expand Down Expand Up @@ -261,8 +259,8 @@ impl<T: Uniplate> HolesIterator<T> {
}

struct HolesBiIterator<T: Uniplate, F: Biplate<T>> {
children_iter: std::iter::Enumerate<im::vector::ConsumingIter<T>>,
children: im::vector::Vector<T>,
children_iter: std::iter::Enumerate<std::collections::vec_deque::IntoIter<T>>,
children: VecDeque<T>,
parent: F,
}

Expand Down Expand Up @@ -307,24 +305,24 @@ mod tests {
proptest! {
#[test]
fn test_context_same_as_universe(ast in proptest_stmts()) {
prop_assert_eq!(ast.universe(),ast.contexts().map(|(elem,_)| elem).collect());
prop_assert_eq!(ast.universe(),ast.contexts().map(|(elem,_)| elem).collect::<VecDeque<_>>());
}

#[test]
fn test_holes_same_as_children(ast in proptest_stmts()) {
prop_assert_eq!(ast.children(),ast.holes().map(|(elem,_)| elem).collect());
prop_assert_eq!(ast.children(),ast.holes().map(|(elem,_)| elem).collect::<VecDeque<_>>());
}

#[test]
fn test_context_bi_same_as_universe_bi(ast in proptest_stmts()) {
prop_assert_eq!(Biplate::<Expr>::universe_bi(&ast),Biplate::<Expr>::contexts_bi(&ast).map(|(elem,_)| elem).collect());
prop_assert_eq!(Biplate::<Stmt>::universe_bi(&ast),Biplate::<Stmt>::contexts_bi(&ast).map(|(elem,_)| elem).collect());
prop_assert_eq!(Biplate::<Expr>::universe_bi(&ast),Biplate::<Expr>::contexts_bi(&ast).map(|(elem,_)| elem).collect::<VecDeque<_>>());
prop_assert_eq!(Biplate::<Stmt>::universe_bi(&ast),Biplate::<Stmt>::contexts_bi(&ast).map(|(elem,_)| elem).collect::<VecDeque<_>>());
}

#[test]
fn test_holes_bi_same_as_children_bi(ast in proptest_stmts()) {
prop_assert_eq!(Biplate::<Expr>::children_bi(&ast),Biplate::<Expr>::holes_bi(&ast).map(|(elem,_)| elem).collect());
prop_assert_eq!(Biplate::<Stmt>::children_bi(&ast),Biplate::<Stmt>::holes_bi(&ast).map(|(elem,_)| elem).collect());
prop_assert_eq!(Biplate::<Expr>::children_bi(&ast),Biplate::<Expr>::holes_bi(&ast).map(|(elem,_)| elem).collect::<VecDeque<_>>());
prop_assert_eq!(Biplate::<Stmt>::children_bi(&ast),Biplate::<Stmt>::holes_bi(&ast).map(|(elem,_)| elem).collect::<VecDeque<_>>());
}
}
}
Loading

0 comments on commit 7b8f19f

Please sign in to comment.