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

Fix line numbers with source fragments #310

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

c42f
Copy link
Member

@c42f c42f commented Jun 16, 2023

When parsing source code fragments incrementally with

* `Meta.parse(str, index)` or
* `parsestmt(str, index)`

we must avoid scanning the rest of str for line numbers for efficiency. In this mode, the user is expected to provide first_line to "manually" specify which line number we're counting from.

Admittedly this is a bit clunky and should be integrated better with SourceFile (which should also be renamed - see issue #190) but for now seems to be the most consistent way to approach things here.

Fixes a bug found over at JuliaLang/julia#46372 (comment)

Update: Switching to using Vector{UInt8} for literal parsing also makes parsing to ParseStream and GreenNode around 10% faster

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #310 (5ca541c) into main (b0a2837) will decrease coverage by 0.03%.
The diff coverage is 97.43%.

@@            Coverage Diff             @@
##             main     #310      +/-   ##
==========================================
- Coverage   96.81%   96.78%   -0.03%     
==========================================
  Files          14       14              
  Lines        4109     4111       +2     
==========================================
+ Hits         3978     3979       +1     
- Misses        131      132       +1     
Impacted Files Coverage Δ
src/kinds.jl 80.59% <ø> (ø)
src/utils.jl 86.66% <83.33%> (+1.21%) ⬆️
src/parse_stream.jl 95.37% <92.85%> (-1.06%) ⬇️
src/literal_parsing.jl 98.35% <98.36%> (+0.39%) ⬆️
src/expr.jl 100.00% <100.00%> (ø)
src/hooks.jl 80.11% <100.00%> (+1.02%) ⬆️
src/parser_api.jl 93.75% <100.00%> (-0.10%) ⬇️
src/source_files.jl 97.36% <100.00%> (ø)
src/syntax_tree.jl 95.27% <100.00%> (+0.13%) ⬆️

When parsing source code fragments incrementally with

    * `Meta.parse(str, index)` or
    * `parsestmt(str, index)`

we must avoid scanning the rest of `str` for line numbers for
efficiency. In this mode, the user is expected to provide `first_line`
to "manually" specify which line number we're counting from.

Admittedly this is a bit clunky and should be integrated better with
SourceFile (which should also be renamed - see issue #190) but for now
seems to be the most consistent way to approach things here.
@c42f c42f force-pushed the c42f/fix-fragment-parsing-line-numbers branch from fc8b04a to 28d2696 Compare June 16, 2023 02:30
@c42f c42f merged commit b933013 into main Jun 16, 2023
@c42f c42f deleted the c42f/fix-fragment-parsing-line-numbers branch June 16, 2023 23:09
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