turbo
3f9de96c - Refactor GraphTraversal to avoid non-determinism (#4098)

Commit
2 years ago
Refactor GraphTraversal to avoid non-determinism (#4098) ### Description The current design of the `Visit` trait makes it easy to introduce non-determinism by having `map_children` return different results depending on the order in which it's called. For instance, if `map_children` tries to de-duplicate children (as is the case in `ChunkContentVisit`): ``` Order 1: 1. (from parent Node1) map_children([NodeA, NodeB]) -> GraphTraversalControlFlow::Continue([NodeA, NodeB]) 2. (from parent Node2) map_children([NodeB, NodeC]) -> GraphTraversalControlFlow::Continue([NodeC]) Order 2: 1. (from parent Node2) map_children([NodeB, NodeC]) -> GraphTraversalControlFlow::Continue([NodeB, NodeC]) 2. (from parent Node1) map_children([NodeA, NodeB]) -> GraphTraversalControlFlow::Continue([NodeA]) ``` These two calling orders will result in different generated graphs, as the first order will forget about the edge Node2 -> NodeB, while the second will forget about the edge Node1 -> NodeB. Instead, this PR makes it so `map_children` is called *after* inserting all nodes into the graph. This ensures that a different ordering can't affect the final shape of the graph. It also refactors the `GraphTraversal::visit` method and the `Visit` trait to make it more consistent with graph terminology and (hopefully) easier to understand. ### Testing Instructions This solves an issue on the branch sokra/repro-for-alex.
Author
Parents
Loading