Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Construct Expr directly from ParseStream #268

Merged
merged 1 commit into from
May 7, 2023
Merged

Construct Expr directly from ParseStream #268

merged 1 commit into from
May 7, 2023

Conversation

c42f
Copy link
Member

@c42f c42f commented May 6, 2023

Generalize build_tree so that we can more easily construct tree types other than GreenNode. Use this to construct Expr directly from ParseStream rather than constructing both GreenNode and SyntaxNode along the way.

Fix a bunch of type instabilities in the Expr conversion code along the way.

With these changes, parsing all of Base to Expr is sped up by about 35% overall and allocations reduced by around 50%. (Parsing to Expr is now comparable with parsing to SyntaxNode.)

@codecov
Copy link

codecov bot commented May 6, 2023

Codecov Report

Merging #268 (a894dad) into main (975cd1d) will increase coverage by 0.12%.
The diff coverage is 98.65%.

@@            Coverage Diff             @@
##             main     #268      +/-   ##
==========================================
+ Coverage   96.41%   96.54%   +0.12%     
==========================================
  Files          15       15              
  Lines        3933     3993      +60     
==========================================
+ Hits         3792     3855      +63     
+ Misses        141      138       -3     
Impacted Files Coverage Δ
src/hooks.jl 77.55% <50.00%> (-0.69%) ⬇️
src/literal_parsing.jl 97.55% <94.64%> (-0.88%) ⬇️
src/expr.jl 100.00% <100.00%> (ø)
src/green_tree.jl 87.80% <100.00%> (+7.80%) ⬆️
src/parse_stream.jl 96.35% <100.00%> (+0.03%) ⬆️
src/syntax_tree.jl 94.96% <100.00%> (+0.11%) ⬆️

... and 2 files with indirect coverage changes

@c42f c42f force-pushed the c42f/tree-building branch 2 times, most recently from 90f27ab to 9ebf92d Compare May 7, 2023 05:03
Generalize `build_tree` so that we can more easily construct tree types
other than `GreenNode`.  Use this to construct `Expr` directly from
`ParseStream` rather than constructing both GreenNode and SyntaxNode
along the way.

Fix a bunch of type instabilities in the `Expr` conversion code along the
way.

With these changes, parsing all of Base to `Expr` is sped up by about 35%
overall and allocations reduced by around 50%. (Parsing to `Expr` is
now comparable with parsing to `SyntaxNode`.)
@c42f c42f force-pushed the c42f/tree-building branch from 9ebf92d to a894dad Compare May 7, 2023 05:27
@c42f c42f merged commit 5bb321a into main May 7, 2023
@c42f c42f deleted the c42f/tree-building branch May 7, 2023 05:40
c42f added a commit that referenced this pull request May 7, 2023
Another approach to fix #239.

Elide all `K"parens"` nodes from the `SyntaxNode` tree, replacing them
with their single child.

With #268 merged, this approach works more neatly and doesn't have the
downsides of #265.
c42f added a commit that referenced this pull request May 7, 2023
Another approach to fix #239.

Elide all `K"parens"` nodes from the `SyntaxNode` tree, replacing them
with their single child.

With #268 merged, this approach works more neatly and doesn't have the
downsides of #265.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant