-
Notifications
You must be signed in to change notification settings - Fork 12.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This commit changes `syntax::fold::Folder` from a functional style (where most methods take a `T` and produce a new `T`) to a more imperative style (where most methods take and modify a `&mut T`), and renames it `syntax::mut_visit::MutVisitor`. The first benefit is speed. The functional style does not require any reallocations, due to the use of `P::map` and `MoveMap::move_{,flat_}map`. However, every field in the AST must be overwritten; even those fields that are unchanged are overwritten with the same value. This causes a lot of unnecessary memory writes. The imperative style reduces instruction counts by 1--3% across a wide range of workloads, particularly incremental workloads. The second benefit is conciseness; the imperative style is usually more concise. E.g. compare the old functional style: ``` fn fold_abc(&mut self, abc: ABC) { ABC { a: fold_a(abc.a), b: fold_b(abc.b), c: abc.c, } } ``` with the imperative style: ``` fn visit_abc(&mut self, ABC { a, b, c: _ }: &mut ABC) { visit_a(a); visit_b(b); } ``` (The reductions get larger in more complex examples.) Overall, the patch removes over 200 lines of code -- even though the new code has more comments -- and a lot of the remaining lines have fewer characters. Some notes: - The old style used methods called `fold_*`. The new style mostly uses methods called `visit_*`, but there are a few methods that map a `T` to something other than a `T`, which are called `flat_map_*` (`T` maps to multiple `T`s) or `filter_map_*` (`T` maps to 0 or 1 `T`s). - `move_map.rs`/`MoveMap`/`move_map`/`move_flat_map` are renamed `map_in_place.rs`/`MapInPlace`/`map_in_place`/`flat_map_in_place` to reflect their slightly changed signatures. - Although this commit renames the `fold` module as `mut_visit`, it keeps it in the `fold.rs` file, so as not to confuse git. The next commit will rename the file.
- Loading branch information
1 parent
970b5d1
commit 9fcb165
Showing
23 changed files
with
1,417 additions
and
1,516 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
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
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
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
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
Oops, something went wrong.