-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This change highlights a serious disadvantage to an intrusive tree representation: it is cumbersome and fragile to separate the non-topological data (i.e., adjacent nodes) and domain data in a branch. To implement `fold_map`, which consumes and moves its data, bespoke intermediate branch token representations and de/compositions are needed. This is fairly gross. There are a few options for a cleaner implementation: 1. Use an unintrusive tree implementation. 2. Always copy the tree (implement `fold_map` over `&self`). 3. Drain and fill branches instead of extracting tokens. Ultimately, [1] is probably the way to go. In the meantime, [2] may look a bit odd, betray the term "map", and perhaps compile to less optimal code (not too sure about that last one though). [3] eliminates the need for abstracting over branch composition (including strongly coupled types like `BranchFold`), but requires making `Repetition` drainable (`token` becomes `Option<Token<_>>`) and presents a somewhat unusual API where `FoldMap::fold` receives a _drained_ `BranchKind` (using normal looking APIs like `BranchKind::tokens` becomes an error in this context). Making the APIs in [3] robust likely requires at least as much code as is used in this change.
- Loading branch information
1 parent
5d737b9
commit c140681
Showing
1 changed file
with
257 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters