Improve error handling for index based API

src/algo.rs

@@ -3,11 +3,11 @@ use alloc::{vec, vec::Vec};
 #[cfg(not(feature = "std"))]
 use libm::F32Ext;
 
+use core::any::Any;
 use core::f32;
 
 use crate::node::{Node, Storage, Stretch};
 use crate::result;
-use crate::result::Result;
 use crate::style::*;
 
 use crate::number::Number::*;
@@ -58,7 +58,7 @@ struct FlexLine {
 }
 
 impl Stretch {
-    pub(crate) fn compute(&mut self, root: Node, size: Size<Number>) -> Result<()> {
+    pub(crate) fn compute(&mut self, root: Node, size: Size<Number>) -> Result<(), Box<Any>> {
         let style = self.style[&root];
         let has_root_min_max = style.min_size.width.is_defined()
             || style.min_size.height.is_defined()
@@ -101,7 +101,7 @@ impl Stretch {
             )?
         };
 
-        *self.layout.get_mut(&root).unwrap() = result::Layout {
+        *self.layout.get_mut(root).unwrap() = result::Layout {
             order: 0,
             size: Size { width: result.size.width, height: result.size.height },
             location: Point { x: 0.0, y: 0.0 },
@@ -118,7 +118,7 @@ impl Stretch {
         abs_x: f32,
         abs_y: f32,
     ) {
-        let layout = layouts.get_mut(&root).unwrap();
+        let layout = layouts.get_mut(root).unwrap();
         let abs_x = abs_x + layout.location.x;
         let abs_y = abs_y + layout.location.y;
 
@@ -137,8 +137,8 @@ impl Stretch {
         node_size: Size<Number>,
         parent_size: Size<Number>,
         perform_layout: bool,
-    ) -> Result<ComputeResult> {
-        *self.is_dirty.get_mut(&node).unwrap() = false;
+    ) -> Result<ComputeResult, Box<Any>> {
+        *self.is_dirty.get_mut(node).unwrap() = false;
 
         // First we check if we have a result for the given input
         if let Some(cache) = &self.layout_cache[&node] {
@@ -200,7 +200,7 @@ impl Stretch {
 
             if let Some(ref measure) = self.measure[&node] {
                 let result = ComputeResult { size: measure(node_size)? };
-                *self.layout_cache.get_mut(&node).unwrap() =
+                *self.layout_cache.get_mut(node).unwrap() =
                     Some(result::Cache { node_size, parent_size, perform_layout, result: result.clone() });
                 return Ok(result);
             }
@@ -281,7 +281,7 @@ impl Stretch {
 
         // TODO - this does not follow spec. See commented out code below
         // 3. Determine the flex base size and hypothetical main size of each item:
-        flex_items.iter_mut().try_for_each(|child| -> Result<()> {
+        flex_items.iter_mut().try_for_each(|child| -> Result<(), Box<Any>> {
             let child_style = self.style[&child.node];
 
             // A. If the item has a definite used flex basis, that’s the flex base size.
@@ -368,7 +368,7 @@ impl Stretch {
         // The hypothetical main size is the item’s flex base size clamped according to its
         // used min and max main sizes (and flooring the content box size at zero).
 
-        flex_items.iter_mut().try_for_each(|child| -> Result<()> {
+        flex_items.iter_mut().try_for_each(|child| -> Result<(), Box<Any>> {
             child.inner_flex_basis = child.flex_basis - child.padding.main(dir) - child.border.main(dir);
 
             // TODO - not really spec abiding but needs to be done somewhere. probably somewhere else though.
@@ -445,7 +445,7 @@ impl Stretch {
         //
         // 9.7. Resolving Flexible Lengths
 
-        flex_lines.iter_mut().try_for_each(|line| -> Result<()> {
+        flex_lines.iter_mut().try_for_each(|line| -> Result<(), Box<Any>> {
             // 1. Determine the used flex factor. Sum the outer hypothetical main sizes of all
             //    items on the line. If the sum is less than the flex container’s inner main size,
             //    use the flex grow factor for the rest of this algorithm; otherwise, use the
@@ -462,7 +462,7 @@ impl Stretch {
             //    - If using the flex shrink factor: any item that has a flex base size
             //      smaller than its hypothetical main size
 
-            line.items.iter_mut().try_for_each(|child| -> Result<()> {
+            line.items.iter_mut().try_for_each(|child| -> Result<(), Box<Any>> {
                 // TODO - This is not found by reading the spec. Maybe this can be done in some other place
                 // instead. This was found by trail and error fixing tests to align with webkit output.
                 if node_inner_size.main(dir).is_undefined() && is_row {
@@ -618,7 +618,7 @@ impl Stretch {
                 //    item’s target main size was made smaller by this, it’s a max violation.
                 //    If the item’s target main size was made larger by this, it’s a min violation.
 
-                let total_violation = unfrozen.iter_mut().try_fold(0.0, |acc, child| -> Result<f32> {
+                let total_violation = unfrozen.iter_mut().try_fold(0.0, |acc, child| -> Result<f32, Box<Any>> {
                     // TODO - not really spec abiding but needs to be done somewhere. probably somewhere else though.
                     // The following logic was developed not from the spec but by trail and error looking into how
                     // webkit handled various scenarios. Can probably be solved better by passing in
@@ -695,7 +695,7 @@ impl Stretch {
         //    used main size and the available space, treating auto as fit-content.
 
         flex_lines.iter_mut().try_for_each(|line| {
-            line.items.iter_mut().try_for_each(|child| -> Result<()> {
+            line.items.iter_mut().try_for_each(|child| -> Result<(), Box<Any>> {
                 let child_cross =
                     child.size.cross(dir).maybe_max(child.min_size.cross(dir)).maybe_min(child.max_size.cross(dir));
 
@@ -741,7 +741,7 @@ impl Stretch {
 
         if has_baseline_child {
             flex_lines.iter_mut().try_for_each(|line| {
-                line.items.iter_mut().try_for_each(|child| -> Result<()> {
+                line.items.iter_mut().try_for_each(|child| -> Result<(), Box<Any>> {
                     let result = self.compute_internal(
                         child.node,
                         Size {
@@ -1088,7 +1088,7 @@ impl Stretch {
         // layout we are done now.
         if !perform_layout {
             let result = ComputeResult { size: container_size };
-            *self.layout_cache.get_mut(&node).unwrap() =
+            *self.layout_cache.get_mut(node).unwrap() =
                 Some(result::Cache { node_size, parent_size, perform_layout, result: result.clone() });
             return Ok(result);
         }
@@ -1152,12 +1152,12 @@ impl Stretch {
             let mut lines: Vec<Vec<result::Layout>> = vec![];
             let mut total_offset_cross = padding_border.cross_start(dir);
 
-            let layout_line = |line: &mut FlexLine| -> Result<()> {
+            let layout_line = |line: &mut FlexLine| -> Result<(), Box<Any>> {
                 let mut children: Vec<result::Layout> = vec![];
                 let mut total_offset_main = padding_border.main_start(dir);
                 let line_offset_cross = line.offset_cross;
 
-                let layout_item = |child: &mut FlexItem| -> Result<()> {
+                let layout_item = |child: &mut FlexItem| -> Result<(), Box<Any>> {
                     let result = self.compute_internal(
                         child.node,
                         child.target_size.map(|s| s.to_number()),
@@ -1176,7 +1176,7 @@ impl Stretch {
                         + child.margin.cross_start(dir)
                         + (child.position.cross_start(dir).or_else(0.0) - child.position.cross_end(dir).or_else(0.0));
 
-                    *self.layout.get_mut(&child.node).unwrap() = result::Layout {
+                    *self.layout.get_mut(child.node).unwrap() = result::Layout {
                         order: self.children[&node].iter().position(|n| *n == child.node).unwrap() as u32,
                         size: result.size,
                         location: Point {
@@ -1334,7 +1334,7 @@ impl Stretch {
                     }
                 };
 
-                *self.layout.get_mut(&child).unwrap() = result::Layout {
+                *self.layout.get_mut(child).unwrap() = result::Layout {
                     order: order as u32,
                     size: result.size,
                     location: Point {
@@ -1346,7 +1346,7 @@ impl Stretch {
         }
 
         fn hidden_layout(layout: &mut Storage<result::Layout>, children: &Storage<Vec<Node>>, node: Node, order: u32) {
-            *layout.get_mut(&node).unwrap() =
+            *layout.get_mut(node).unwrap() =
                 result::Layout { order, size: Size { width: 0.0, height: 0.0 }, location: Point { x: 0.0, y: 0.0 } };
 
             for (order, child) in children[&node].iter().enumerate() {
@@ -1361,7 +1361,7 @@ impl Stretch {
         }
 
         let result = ComputeResult { size: container_size };
-        *self.layout_cache.get_mut(&node).unwrap() =
+        *self.layout_cache.get_mut(node).unwrap() =
             Some(result::Cache { node_size, parent_size, perform_layout, result: result.clone() });
         Ok(result)
     }

src/lib.rs

@@ -16,3 +16,29 @@ mod algo;
 mod id;
 
 pub use crate::node::Stretch;
+
+use core::any::Any;
+
+#[derive(Debug)]
+pub enum Error {
+    InvalidNode(node::Node),
+    Measure(Box<Any>),
+}
+
+impl std::fmt::Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match *self {
+            Error::InvalidNode(ref node) => write!(f, "Invalid node {:?}", node),
+            Error::Measure(_) => write!(f, "Error during measurement"),
+        }
+    }
+}
+
+impl std::error::Error for Error {
+    fn description(&self) -> &str {
+        match *self {
+            Error::InvalidNode(_) => "The node is not part of the stretch instance",
+            Error::Measure(_) => "Error occurred inside a measurement function",
+        }
+    }
+}

src/node.rs

@@ -1,17 +1,20 @@
 #[cfg(not(feature = "std"))]
 use alloc::{vec, vec::Vec};
 
+use core::any::Any;
+
 use std::collections::HashMap;
 use std::ops::Drop;
 use std::sync::Mutex;
 
 use crate::geometry::Size;
 use crate::id;
 use crate::number::Number;
-use crate::result::{Cache, Layout, Result};
+use crate::result::{Cache, Layout};
 use crate::style::*;
+use crate::Error;
 
-type MeasureFunc = Box<Fn(Size<Number>) -> Result<Size<f32>>>;
+type MeasureFunc = Box<Fn(Size<Number>) -> Result<Size<f32>, Box<Any>>>;
 
 lazy_static! {
     /// Global stretch instance id allocator.
@@ -24,7 +27,39 @@ pub struct Node {
     local: id::Id,
 }
 
-pub(crate) type Storage<T> = HashMap<Node, T>;
+pub(crate) struct Storage<T>(HashMap<Node, T>);
+
+impl<T> Storage<T> {
+    pub fn new() -> Self {
+        Storage(HashMap::new())
+    }
+
+    pub fn get(&self, node: Node) -> Result<&T, Error> {
+        match self.0.get(&node) {
+            Some(v) => Ok(v),
+            None => Err(Error::InvalidNode(node)),
+        }
+    }
+
+    pub fn get_mut(&mut self, node: Node) -> Result<&mut T, Error> {
+        match self.0.get_mut(&node) {
+            Some(v) => Ok(v),
+            None => Err(Error::InvalidNode(node)),
+        }
+    }
+
+    pub fn insert(&mut self, node: Node, value: T) -> Option<T> {
+        self.0.insert(node, value)
+    }
+}
+
+impl<T> std::ops::Index<&Node> for Storage<T> {
+    type Output = T;
+
+    fn index(&self, idx: &Node) -> &T {
+        &(self.0)[idx]
+    }
+}
 
 pub struct Stretch {
     id: id::Id,
@@ -72,11 +107,11 @@ impl Stretch {
         node
     }
 
-    pub fn new_node(&mut self, style: Style, children: Vec<Node>) -> Node {
+    pub fn new_node(&mut self, style: Style, children: Vec<Node>) -> Result<Node, Error> {
         let node = self.allocate_node();
 
         for child in &children {
-            self.parents.get_mut(&child).unwrap().push(node);
+            self.parents.get_mut(*child)?.push(node);
         }
 
         self.style.insert(node, style);
@@ -87,12 +122,13 @@ impl Stretch {
         self.layout_cache.insert(node, None);
         self.is_dirty.insert(node, true);
 
-        node
+        Ok(node)
     }
 
-    pub fn set_measure(&mut self, node: Node, measure: Option<MeasureFunc>) {
-        *self.measure[&node].unwrap() = measure;
-        self.mark_dirty(node);
+    pub fn set_measure(&mut self, node: Node, measure: Option<MeasureFunc>) -> Result<(), Error> {
+        *self.measure.get_mut(node)? = measure;
+        self.mark_dirty(node)?;
+        Ok(())
     }
 
     pub fn add_child(&mut self, node: Node, child: Node) {
@@ -162,12 +198,12 @@ impl Stretch {
         unimplemented!()
     }
 
-    pub fn children(&self, node: Node) -> Vec<Node> {
-        self.children[&node].clone()
+    pub fn children(&self, node: Node) -> Result<Vec<Node>, Error> {
+        self.children.get(node).map(Clone::clone)
     }
 
-    pub fn child_count(&self, node: Node) -> usize {
-        self.children[&node].len()
+    pub fn child_count(&self, node: Node) -> Result<usize, Error> {
+        self.children.get(node).map(Vec::len)
     }
 
     pub fn set_style(&mut self, node: Node, style: Style) {
@@ -177,15 +213,15 @@ impl Stretch {
         unimplemented!()
     }
 
-    pub fn style(&self, node: Node) -> &Style {
-        &self.style[&node]
+    pub fn style(&self, node: Node) -> Result<&Style, Error> {
+        self.style.get(node)
     }
 
-    pub fn layout(&self, node: Node) -> &Layout {
-        &self.layout[&node]
+    pub fn layout(&self, node: Node) -> Result<&Layout, Error> {
+        self.layout.get(node)
     }
 
-    pub fn mark_dirty(&mut self, node: Node) {
+    pub fn mark_dirty(&mut self, node: Node) -> Result<(), Error> {
         // let mut node = self.0.borrow_mut();
         // node.layout_cache = None;
         // node.is_dirty = true;
@@ -199,12 +235,15 @@ impl Stretch {
         unimplemented!()
     }
 
-    pub fn dirty(&self, node: Node) -> bool {
-        self.is_dirty[&node]
+    pub fn dirty(&self, node: Node) -> Result<bool, Error> {
+        self.is_dirty.get(node).map(|v| *v)
     }
 
-    pub fn compute_layout(&mut self, node: Node, size: Size<Number>) -> Result<()> {
-        self.compute(node, size)
+    pub fn compute_layout(&mut self, node: Node, size: Size<Number>) -> Result<(), Error> {
+        match self.layout.get(node) {
+            Ok(_) => self.compute(node, size).map_err(|err| Error::Measure(err)),
+            _ => Err(Error::InvalidNode(node)),
+        }
     }
 }
 

src/result.rs

@@ -1,14 +1,10 @@
 #[cfg(not(feature = "std"))]
 use alloc::vec::Vec;
 
-use core::any::Any;
-
 use crate::algo::ComputeResult;
 use crate::geometry::{Point, Size};
 use crate::number::Number;
 
-pub type Result<T> = core::result::Result<T, Box<Any>>;
-
 #[derive(Copy, Debug, Clone)]
 pub struct Layout {
     pub(crate) order: u32,