Skip to content
New issue

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

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

Already on GitHub? # to your account

Fixes unreachable macro crashing compiler. #6362

Merged
merged 5 commits into from
Aug 7, 2024
Merged

Fixes unreachable macro crashing compiler. #6362

merged 5 commits into from
Aug 7, 2024

Conversation

esdrubal
Copy link
Contributor

@esdrubal esdrubal commented Aug 6, 2024

Description

In case of a bad input given to the lexer, it is possible to generate an AST with invalid ImplItem tuples.

This fix replaces the unreachable macro with an CompileError::Internal. This allows the user to see the lexer errors and fix them which will also address the new errors.

Fixes #6339.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@esdrubal esdrubal added bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Aug 6, 2024
@esdrubal esdrubal self-assigned this Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

Benchmark for c565dff

Click to view benchmark
Test Base PR %
code_action 5.2±0.04ms 5.2±0.06ms 0.00%
code_lens 306.9±15.70ns 300.9±7.06ns -1.96%
compile 2.7±0.04s 2.6±0.05s -3.70%
completion 4.7±0.08ms 4.7±0.05ms 0.00%
did_change_with_caching 2.6±0.03s 2.6±0.03s 0.00%
document_symbol 857.4±9.05µs 924.2±48.46µs +7.79%
format 73.6±1.41ms 72.4±1.05ms -1.63%
goto_definition 334.0±7.34µs 343.2±9.49µs +2.75%
highlight 9.0±0.17ms 8.9±0.02ms -1.11%
hover 487.7±10.12µs 499.5±7.24µs +2.42%
idents_at_position 119.4±0.31µs 121.3±0.71µs +1.59%
inlay_hints 636.2±35.03µs 640.8±16.28µs +0.72%
on_enter 2.1±0.06µs 2.0±0.07µs -4.76%
parent_decl_at_position 3.7±0.03ms 3.7±0.05ms 0.00%
prepare_rename 334.3±4.22µs 343.3±8.10µs +2.69%
rename 9.3±0.14ms 9.3±0.03ms 0.00%
semantic_tokens 1247.6±29.80µs 1303.7±24.22µs +4.50%
token_at_position 335.4±3.20µs 343.0±2.68µs +2.27%
tokens_at_position 3.7±0.03ms 3.7±0.04ms 0.00%
tokens_for_file 400.6±3.37µs 403.6±3.59µs +0.75%
traverse 37.6±1.00ms 38.2±1.63ms +1.60%

In case of a bad input given to the lexer it is possible to generate
an AST with invalid ImplItem tuples.

This fix replaces the unreachable macro with an CompileError::Internal.
This allows the user to see the lexer errors and fix them which will
also take care of the new errors.

Fixes #6339.
Copy link

github-actions bot commented Aug 6, 2024

Benchmark for 55537fc

Click to view benchmark
Test Base PR %
code_action 5.2±0.04ms 5.2±0.13ms 0.00%
code_lens 298.6±11.71ns 301.1±15.48ns +0.84%
compile 2.6±0.06s 2.7±0.05s +3.85%
completion 4.7±0.05ms 4.7±0.10ms 0.00%
did_change_with_caching 2.7±0.03s 2.5±0.03s -7.41%
document_symbol 889.0±21.95µs 854.8±21.04µs -3.85%
format 72.5±0.97ms 71.9±0.71ms -0.83%
goto_definition 330.4±5.15µs 340.0±5.93µs +2.91%
highlight 9.0±0.04ms 9.2±0.15ms +2.22%
hover 487.4±5.80µs 499.4±5.40µs +2.46%
idents_at_position 118.5±0.63µs 118.2±0.41µs -0.25%
inlay_hints 634.0±42.89µs 636.9±11.61µs +0.46%
on_enter 2.1±0.03µs 2.1±0.05µs 0.00%
parent_decl_at_position 3.7±0.04ms 3.7±0.04ms 0.00%
prepare_rename 334.3±7.21µs 339.8±6.77µs +1.65%
rename 9.3±0.05ms 9.4±0.17ms +1.08%
semantic_tokens 1234.3±14.32µs 1245.9±11.16µs +0.94%
token_at_position 330.6±3.01µs 338.2±3.01µs +2.30%
tokens_at_position 3.7±0.03ms 3.7±0.03ms 0.00%
tokens_for_file 403.5±4.92µs 402.8±1.77µs -0.17%
traverse 37.9±1.14ms 37.5±0.42ms -1.06%

@tritao tritao changed the title Fixes ureachable macro crashing compiler. Fixes unreachable macro crashing compiler. Aug 6, 2024
@esdrubal esdrubal marked this pull request as ready for review August 7, 2024 12:38
@esdrubal esdrubal requested a review from a team as a code owner August 7, 2024 12:38
Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 54b8fe3

Click to view benchmark
Test Base PR %
code_action 5.6±0.17ms 5.6±0.23ms 0.00%
code_lens 285.4±7.26ns 284.2±8.40ns -0.42%
compile 2.9±0.05s 2.8±0.03s -3.45%
completion 4.9±0.12ms 5.1±0.20ms +4.08%
did_change_with_caching 2.8±0.04s 2.8±0.03s 0.00%
document_symbol 921.0±41.56µs 926.0±47.74µs +0.54%
format 74.0±1.19ms 73.6±1.38ms -0.54%
goto_definition 342.2±3.31µs 347.6±7.27µs +1.58%
highlight 9.4±0.26ms 9.3±0.31ms -1.06%
hover 496.6±6.29µs 506.5±11.76µs +1.99%
idents_at_position 118.5±0.49µs 120.0±1.60µs +1.27%
inlay_hints 638.8±10.72µs 640.4±29.98µs +0.25%
on_enter 2.4±0.03µs 2.1±0.10µs -12.50%
parent_decl_at_position 3.8±0.15ms 3.9±0.30ms +2.63%
prepare_rename 339.1±4.07µs 348.3±5.17µs +2.71%
rename 9.8±0.19ms 9.7±0.11ms -1.02%
semantic_tokens 1228.2±19.83µs 1267.9±25.90µs +3.23%
token_at_position 333.2±2.48µs 343.5±3.48µs +3.09%
tokens_at_position 3.8±0.13ms 3.8±0.09ms 0.00%
tokens_for_file 400.9±4.02µs 445.8±10.20µs +11.20%
traverse 38.8±0.89ms 39.5±0.84ms +1.80%

Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 976ebaa

Click to view benchmark
Test Base PR %
code_action 5.3±0.13ms 5.1±0.12ms -3.77%
code_lens 286.5±11.53ns 287.0±11.56ns +0.17%
compile 2.9±0.06s 2.7±0.05s -6.90%
completion 4.7±0.09ms 4.6±0.12ms -2.13%
did_change_with_caching 2.8±0.04s 2.7±0.04s -3.57%
document_symbol 946.3±26.03µs 852.2±45.71µs -9.94%
format 73.7±0.58ms 73.6±2.26ms -0.14%
goto_definition 346.9±22.91µs 346.0±5.23µs -0.26%
highlight 9.2±0.77ms 8.7±0.10ms -5.43%
hover 496.4±4.07µs 497.7±10.51µs +0.26%
idents_at_position 116.8±0.37µs 118.9±0.75µs +1.80%
inlay_hints 652.8±13.45µs 636.4±29.73µs -2.51%
on_enter 2.0±0.05µs 2.0±0.05µs 0.00%
parent_decl_at_position 3.7±0.03ms 3.6±0.02ms -2.70%
prepare_rename 343.0±7.83µs 344.8±6.33µs +0.52%
rename 9.5±0.17ms 9.0±0.22ms -5.26%
semantic_tokens 1263.3±13.30µs 1191.4±12.37µs -5.69%
token_at_position 335.7±2.52µs 337.5±2.09µs +0.54%
tokens_at_position 3.7±0.06ms 3.6±0.06ms -2.70%
tokens_for_file 401.1±2.06µs 402.5±6.15µs +0.35%
traverse 38.8±0.79ms 38.4±0.99ms -1.03%

@tritao tritao enabled auto-merge (squash) August 7, 2024 19:21
Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 204da92

Click to view benchmark
Test Base PR %
code_action 5.5±0.16ms 5.1±0.12ms -7.27%
code_lens 292.4±4.69ns 287.0±12.17ns -1.85%
compile 2.7±0.04s 2.7±0.10s 0.00%
completion 4.6±0.13ms 4.5±0.11ms -2.17%
did_change_with_caching 2.7±0.05s 2.7±0.05s 0.00%
document_symbol 885.1±34.51µs 867.8±45.01µs -1.95%
format 74.1±1.94ms 73.9±0.98ms -0.27%
goto_definition 336.2±8.07µs 339.7±6.82µs +1.04%
highlight 8.7±0.12ms 9.0±0.14ms +3.45%
hover 493.2±21.95µs 514.9±4.78µs +4.40%
idents_at_position 121.2±0.73µs 118.2±3.27µs -2.48%
inlay_hints 677.7±18.66µs 627.5±32.62µs -7.41%
on_enter 1916.8±56.30ns 2.0±0.04µs +4.34%
parent_decl_at_position 3.6±0.08ms 3.5±0.03ms -2.78%
prepare_rename 333.4±5.78µs 337.7±4.27µs +1.29%
rename 9.2±0.28ms 9.2±0.25ms 0.00%
semantic_tokens 1175.0±10.28µs 1261.3±12.66µs +7.34%
token_at_position 347.6±2.36µs 334.6±2.56µs -3.74%
tokens_at_position 3.6±0.11ms 3.6±0.08ms 0.00%
tokens_for_file 398.9±2.30µs 400.1±2.02µs +0.30%
traverse 39.2±0.79ms 38.7±0.78ms -1.28%

@tritao tritao merged commit 2332025 into master Aug 7, 2024
36 checks passed
@tritao tritao deleted the esdrubal/6339 branch August 7, 2024 23:23
Copy link

github-actions bot commented Aug 7, 2024

Benchmark for 1d2e0f2

Click to view benchmark
Test Base PR %
code_action 5.0±0.01ms 5.0±0.12ms 0.00%
code_lens 290.7±7.96ns 288.6±5.93ns -0.72%
compile 2.6±0.03s 2.6±0.06s 0.00%
completion 4.5±0.06ms 4.5±0.03ms 0.00%
did_change_with_caching 2.6±0.02s 2.5±0.03s -3.85%
document_symbol 848.3±26.43µs 923.4±23.56µs +8.85%
format 71.7±1.01ms 72.5±1.57ms +1.12%
goto_definition 340.5±3.79µs 337.8±2.68µs -0.79%
highlight 8.7±0.06ms 8.7±0.13ms 0.00%
hover 492.0±5.25µs 493.2±4.50µs +0.24%
idents_at_position 120.2±0.47µs 119.6±0.64µs -0.50%
inlay_hints 626.8±26.88µs 629.5±18.47µs +0.43%
on_enter 1874.9±39.75ns 2.0±0.06µs +6.67%
parent_decl_at_position 3.6±0.02ms 3.6±0.04ms 0.00%
prepare_rename 334.9±4.95µs 333.7±6.50µs -0.36%
rename 9.0±0.17ms 9.0±0.09ms 0.00%
semantic_tokens 1242.6±13.60µs 1253.7±16.72µs +0.89%
token_at_position 335.5±2.77µs 332.1±2.51µs -1.01%
tokens_at_position 3.6±0.03ms 3.5±0.06ms -2.78%
tokens_for_file 396.8±2.03µs 396.6±2.85µs -0.05%
traverse 36.5±0.83ms 37.0±1.22ms +1.37%

esdrubal added a commit that referenced this pull request Aug 13, 2024
## Description

In case of a bad input given to the lexer, it is possible to generate an
AST with invalid ImplItem tuples.

This fix replaces the unreachable macro with an CompileError::Internal.
This allows the user to see the lexer errors and fix them which will
also address the new errors.

Fixes #6339.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: IGI-111 <igi-111@protonmail.com>
Co-authored-by: João Matos <joao@tritao.eu>
Copy link
Member

@sdankel sdankel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sway compiler gets unreachable crash when compiling trait with unknown impl type
4 participants