Add further comments explaining "piecewise bottom-up"

Author: JoJoDeveloping

src/borrow_tracker/tree_borrows/tree.rs

@@ -302,6 +302,12 @@ enum ContinueTraversal {
     SkipSelfAndChildren,
 }
 
+#[derive(Clone, Copy)]
+pub enum ChildrenVisitMode {
+    VisitChildrenOfAccessed,
+    SkipChildrenOfAccessed,
+}
+
 /// Stack of nodes left to explore in a tree traversal.
 struct TreeVisitorStack<NodeContinue, NodeApp, ErrHandler> {
     /// Identifier of the original access.
@@ -316,9 +322,9 @@ struct TreeVisitorStack<NodeContinue, NodeApp, ErrHandler> {
     /// Mutable state of the visit: the tags left to handle.
     /// Every tag pushed should eventually be handled,
     /// and the precise order is relevant for diagnostics.
-    /// Since the traversal is bottom-up, we need to remember
-    /// whether we're here initially (false) or after visiting all
-    /// children (true). The bool indicates this.
+    /// Since the traversal is piecewise bottom-up, we need to
+    /// remember whether we're here initially (false) or after
+    /// visiting all children (true). The bool indicates this.
     stack: Vec<(UniIndex, AccessRelatedness, bool)>,
 }
 
@@ -362,15 +368,15 @@ where
         &mut self,
         this: &mut TreeVisitor<'_>,
         accessed_node: UniIndex,
-        push_children_of_accessed: bool,
+        visit_children: ChildrenVisitMode,
     ) -> Result<(), OutErr> {
         assert!(self.stack.is_empty());
         // First, handle accessed node. A bunch of things need to
         // be handled differently here compared to the further parents
         // of `accesssed_node`.
         {
             self.propagate_at(this, accessed_node, AccessRelatedness::This)?;
-            if push_children_of_accessed {
+            if matches!(visit_children, ChildrenVisitMode::VisitChildrenOfAccessed) {
                 let accessed_node = this.nodes.get(accessed_node).unwrap();
                 for &child in accessed_node.children.iter().rev() {
                     self.stack.push((child, AccessRelatedness::AncestorAccess, false));
@@ -437,19 +443,34 @@ where
 }
 
 impl<'tree> TreeVisitor<'tree> {
-    // Applies `f_propagate` to every vertex of the tree bottom-up in the following order: first
+    // Applies `f_propagate` to every vertex of the tree in a piecewise bottom-up way: First, visit
     // all ancestors of `start` (starting with `start` itself), then children of `start`, then the rest,
-    // always going bottom-up.
+    // going bottom-up in each of these two "pieces" / sections.
     // This ensures that errors are triggered in the following order
     // - first invalid accesses with insufficient permissions, closest to the accessed node first,
     // - then protector violations, bottom-up, starting with the children of the accessed node, and then
     //   going upwards and outwards.
+    // The following graphic visualizes it, with numbers indicating visitation order and `start` being
+    // the node that is visited first ("1"):
+    //
+    //         3
+    //         |
+    //         2
+    //        /|\
+    //       / | \
+    //      7  1  8
+    //        / \
+    //       4   6
+    //           |
+    //           5
     //
     // `f_propagate` should follow the following format: for a given `Node` it updates its
     // `Permission` depending on the position relative to `start` (given by an
     // `AccessRelatedness`).
-    // `f_continue` is called before on foreign nodes, and describes whether to continue
-    // with the subtree at that node.
+    // `f_continue` is called earlier on foreign nodes, and describes whether to even start
+    // visiting the subtree at that node. If it e.g. returns `SkipSelfAndChildren` on node 6
+    // above, then nodes 5 _and_ 6 would not be visited by `f_propagate`. It is not used for
+    // notes having a child access (nodes 1, 2, 3).
     fn traverse_this_parents_children_other<InnErr, OutErr>(
         mut self,
         start: BorTag,
@@ -463,7 +484,11 @@ impl<'tree> TreeVisitor<'tree> {
         // undergoing a child access. Also pushes the children and the other
         // cousin nodes (i.e. all nodes undergoing a foreign access) to the stack
         // to be processed later.
-        stack.go_upwards_from_accessed(&mut self, start_idx, true)?;
+        stack.go_upwards_from_accessed(
+            &mut self,
+            start_idx,
+            ChildrenVisitMode::VisitChildrenOfAccessed,
+        )?;
         // Now visit all the foreign nodes we remembered earlier.
         // For this we go bottom-up, but also allow f_continue to skip entire
         // subtrees from being visited if it would be a NOP.
@@ -483,7 +508,11 @@ impl<'tree> TreeVisitor<'tree> {
         // Visits the accessed node itself, and all its parents, i.e. all nodes
         // undergoing a child access. Also pushes the other cousin nodes to the
         // stack, but not the children of the accessed node.
-        stack.go_upwards_from_accessed(&mut self, start_idx, false)?;
+        stack.go_upwards_from_accessed(
+            &mut self,
+            start_idx,
+            ChildrenVisitMode::SkipChildrenOfAccessed,
+        )?;
         // Now visit all the foreign nodes we remembered earlier.
         // For this we go bottom-up, but also allow f_continue to skip entire
         // subtrees from being visited if it would be a NOP.