get rid of arcs and weaks
authorBen Pfaff <blp@cs.stanford.edu>
Fri, 11 Apr 2025 23:11:56 +0000 (16:11 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Fri, 11 Apr 2025 23:11:56 +0000 (16:11 -0700)
rust/pspp/src/output/pivot/mod.rs
rust/pspp/src/output/pivot/output.rs
rust/pspp/src/output/pivot/test.rs

index dc8cfef2ddf214ec4c21e9b7eb121083de7a96a1..a41466af12763d7695a432773e40a5916672f436 100644 (file)
@@ -35,7 +35,7 @@ use std::{
     iter::{once, repeat, repeat_n, FusedIterator},
     ops::{Index, IndexMut, Not, Range, RangeInclusive},
     str::{from_utf8, FromStr, Utf8Error},
-    sync::{Arc, OnceLock, Weak},
+    sync::{Arc, OnceLock},
 };
 
 use binrw::Error as BinError;
@@ -307,7 +307,7 @@ pub struct Dimension {
     ///
     /// The root must always be a group, although it is allowed to have no
     /// subcategories.
-    pub root: Arc<Group>,
+    pub root: Group,
 
     /// Ordering of leaves for presentation.
     ///
@@ -318,6 +318,12 @@ pub struct Dimension {
     hide_all_labels: bool,
 }
 
+pub type GroupVec<'a> = SmallVec<[&'a Group; 4]>;
+pub struct Path<'a> {
+    groups: GroupVec<'a>,
+    leaf: &'a Leaf,
+}
+
 impl Dimension {
     pub fn is_empty(&self) -> bool {
         self.len() == 0
@@ -331,6 +337,10 @@ impl Dimension {
         self.root.nth_leaf(index)
     }
 
+    pub fn leaf_path(&self, index: usize) -> Option<Path<'_>> {
+        self.root.leaf_path(index, SmallVec::new())
+    }
+
     pub fn builder(axis: Axis3, root: GroupBuilder) -> DimensionBuilder {
         DimensionBuilder::new(axis, root)
     }
@@ -338,7 +348,6 @@ impl Dimension {
 
 #[derive(Clone, Debug)]
 pub struct Group {
-    parent: Option<Weak<Group>>,
     len: usize,
     name: Box<Value>,
 
@@ -357,10 +366,6 @@ impl Group {
         GroupBuilder::new(name)
     }
 
-    pub fn parent(&self) -> Option<Arc<Group>> {
-        self.parent.as_ref().map(|parent| parent.upgrade().unwrap())
-    }
-
     pub fn nth_leaf(&self, mut index: usize) -> Option<&Leaf> {
         for child in &self.children {
             let len = child.len();
@@ -372,6 +377,18 @@ impl Group {
         None
     }
 
+    pub fn leaf_path<'a>(&'a self, mut index: usize, mut groups: GroupVec<'a>) -> Option<Path<'a>> {
+        for child in &self.children {
+            let len = child.len();
+            if index < len {
+                groups.push(self);
+                return child.leaf_path(index, groups);
+            }
+            index -= len;
+        }
+        None
+    }
+
     pub fn len(&self) -> usize {
         self.len
     }
@@ -389,7 +406,7 @@ pub struct DimensionBuilder {
 
 impl DimensionBuilder {
     pub fn new(axis: Axis3, root: GroupBuilder) -> Self {
-        let root = root.build(None);
+        let root = root.build(true);
         Self {
             axis,
             dimension: Dimension {
@@ -449,21 +466,16 @@ impl GroupBuilder {
     fn len(&self) -> usize {
         self.children.iter().map(|category| category.len()).sum()
     }
-    fn build(self, parent: Option<Weak<Group>>) -> Arc<Group> {
-        Arc::new_cyclic(|weak| Group {
+    fn build(self, is_root: bool) -> Group {
+        Group {
             len: self.children.iter().map(|c| c.len()).sum(),
             name: self.name,
-            children: self
-                .children
-                .into_iter()
-                .map(|c| c.build(weak.clone()))
-                .collect(),
+            children: self.children.into_iter().map(|c| c.build()).collect(),
             show_label: {
                 // By default, nested group labels are shown, but not dimension root labels.
-                self.show_label.unwrap_or(parent.is_some())
+                self.show_label.unwrap_or(!is_root)
             },
-            parent,
-        })
+        }
     }
 }
 
@@ -483,15 +495,11 @@ impl CategoryBuilder {
             CategoryBuilder::Leaf { .. } => 1,
         }
     }
-    fn build(self, parent: Weak<Group>) -> Category {
+    fn build(self) -> Category {
         match self {
-            Self::Group(group) => Category::Group(group.build(Some(parent))),
+            Self::Group(group) => Category::Group(group.build(false)),
             Self::Leaf { name, class } => {
-                let leaf = Arc::new(Leaf {
-                    parent,
-                    name,
-                    class,
-                });
+                let leaf = Arc::new(Leaf { name, class });
                 Category::Leaf(leaf)
             }
         }
@@ -626,7 +634,6 @@ impl PivotTableBuilder {
 
 #[derive(Clone, Debug)]
 pub struct Leaf {
-    parent: Weak<Group>,
     name: Box<Value>,
 
     /// Default format for values in this category.
@@ -636,7 +643,6 @@ pub struct Leaf {
 impl Leaf {
     pub fn new(name: Value) -> Self {
         Self {
-            parent: Weak::new(),
             name: Box::new(name),
             class: None,
         }
@@ -647,9 +653,6 @@ impl Leaf {
             ..self
         }
     }
-    pub fn ancestors(&self) -> impl Iterator<Item = Arc<Group>> {
-        std::iter::successors(self.parent(), |group| group.parent())
-    }
 }
 
 /// Pivot result classes.
@@ -670,7 +673,7 @@ pub enum Class {
 /// A pivot_category is a leaf (a category) or a group.
 #[derive(Clone, Debug)]
 pub enum Category {
-    Group(Arc<Group>),
+    Group(Group),
     Leaf(Arc<Leaf>),
 }
 
@@ -678,7 +681,7 @@ impl Category {
     fn len(&self) -> usize {
         match self {
             Category::Group(group) => group.len,
-            Category::Leaf(leaf) => 1,
+            Category::Leaf(_) => 1,
         }
     }
 
@@ -695,6 +698,22 @@ impl Category {
         }
     }
 
+    fn leaf_path<'a>(&'a self, index: usize, groups: GroupVec<'a>) -> Option<Path<'a>> {
+        match self {
+            Category::Group(group) => group.leaf_path(index, groups),
+            Category::Leaf(leaf) => {
+                if index == 0 {
+                    Some(Path {
+                        groups,
+                        leaf: &leaf,
+                    })
+                } else {
+                    None
+                }
+            }
+        }
+    }
+
     fn show_label(&self) -> bool {
         match self {
             Category::Group(group) => group.show_label,
@@ -705,27 +724,18 @@ impl Category {
 
 trait CategoryTrait {
     fn name(&self) -> &Value;
-    fn parent(&self) -> Option<Arc<Group>>;
 }
 
 impl CategoryTrait for Group {
     fn name(&self) -> &Value {
         &self.name
     }
-
-    fn parent(&self) -> Option<Arc<Group>> {
-        self.parent.as_ref().and_then(|parent| parent.upgrade())
-    }
 }
 
 impl CategoryTrait for Leaf {
     fn name(&self) -> &Value {
         &self.name
     }
-
-    fn parent(&self) -> Option<Arc<Group>> {
-        Some(self.parent.upgrade().unwrap())
-    }
 }
 
 impl CategoryTrait for Category {
@@ -735,13 +745,6 @@ impl CategoryTrait for Category {
             Category::Leaf(leaf) => leaf.name(),
         }
     }
-
-    fn parent(&self) -> Option<Arc<Group>> {
-        match self {
-            Category::Group(group) => group.parent(),
-            Category::Leaf(leaf) => leaf.parent(),
-        }
-    }
 }
 
 /// Styling for a pivot table.
index da60afdb7abea6178f7c1e34521a5910e41f799c..e66fb8ecd91fde3fa80aaa6eb131ca0fe08b5b62 100644 (file)
@@ -2,10 +2,9 @@ use std::{ops::Range, sync::Arc};
 
 use enum_map::{enum_map, EnumMap};
 use itertools::Itertools;
-use smallvec::SmallVec;
 
 use crate::output::{
-    pivot::{Group, HeadingRegion, LabelPosition, Leaf},
+    pivot::{HeadingRegion, LabelPosition, Path},
     table::{CellInner, Table},
 };
 
@@ -324,12 +323,7 @@ pub struct OutputTables {
     pub footnotes: Option<Table>,
 }
 
-struct HeadingColumn<'a> {
-    leaf: &'a Leaf,
-    groups: SmallVec<[Arc<Group>; 4]>,
-}
-
-impl HeadingColumn<'_> {
+impl Path<'_> {
     pub fn get(&self, y: usize, height: usize) -> Option<&Value> {
         if y + 1 == height {
             Some(&self.leaf.name)
@@ -342,7 +336,7 @@ impl HeadingColumn<'_> {
 struct Heading<'a> {
     dimension: &'a Dimension,
     height: usize,
-    columns: Vec<HeadingColumn<'a>>,
+    columns: Vec<Path<'a>>,
 }
 
 impl<'a> Heading<'a> {
@@ -358,16 +352,12 @@ impl<'a> Heading<'a> {
         let mut columns = Vec::new();
         let mut height = 0;
         for indexes in column_enumeration.iter() {
-            let leaf = dimension
-                .nth_leaf(dimension.presentation_order[indexes[dim_index]])
+            let mut path = dimension
+                .leaf_path(dimension.presentation_order[indexes[dim_index]])
                 .unwrap();
-            let mut groups = leaf
-                .ancestors()
-                .filter(|group| group.show_label)
-                .collect::<SmallVec<_>>();
-            groups.reverse();
-            height = height.max(1 + groups.len());
-            columns.push(HeadingColumn { leaf, groups });
+            path.groups.retain(|group| group.show_label);
+            height = height.max(1 + path.groups.len());
+            columns.push(path);
         }
 
         Some(Self {
index acc497ddfc4da701d6d43a7194a3cac140237382..3aecd0481a5bb63acf746486d73df5fae391a377 100644 (file)
@@ -660,13 +660,13 @@ fn empty_dimensions() {
 
 #[test]
 fn empty_groups() {
-    let mut a = GroupBuilder::new(Value::new_text("a"))
+    let a = GroupBuilder::new(Value::new_text("a"))
         .with(Value::new_text("a1"))
         .with(GroupBuilder::new(Value::new_text("a2")))
         .with(Value::new_text("a3"));
     let d1 = DimensionBuilder::new(Axis3::X, a);
 
-    let mut b = GroupBuilder::new(Value::new_text("b"))
+    let b = GroupBuilder::new(Value::new_text("b"))
         .with(GroupBuilder::new(Value::new_text("b1")))
         .with(Value::new_text("b2"))
         .with(Value::new_text("b3"));