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

Fixes panic on unwrapping in type_check_trait_implementation. #6434

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

esdrubal
Copy link
Contributor

@esdrubal esdrubal commented Aug 19, 2024

Description

This PR fixes panic on unwrapping by only performing the required behavior with the unwrapped value when it is available.

Fixes #6336

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 19, 2024
@esdrubal esdrubal self-assigned this Aug 19, 2024
This PR fixes panic on unwrapping by only perform the required behaviour
with the unwrapped value when it is available.

Fixes #6336
Copy link

Benchmark for 76bcf6c

Click to view benchmark
Test Base PR %
code_action 5.8±0.26ms 5.6±0.24ms -3.45%
code_lens 290.9±10.97ns 298.9±8.61ns +2.75%
compile 3.1±0.07s 2.9±0.05s -6.45%
completion 5.1±0.34ms 5.1±0.32ms 0.00%
did_change_with_caching 2.9±0.06s 2.8±0.06s -3.45%
document_symbol 852.8±22.89µs 934.4±48.55µs +9.57%
format 73.9±1.23ms 73.6±1.28ms -0.41%
goto_definition 339.1±10.44µs 346.1±7.86µs +2.06%
highlight 9.4±0.26ms 9.2±0.19ms -2.13%
hover 494.0±7.29µs 501.8±10.84µs +1.58%
idents_at_position 121.6±0.47µs 122.3±1.24µs +0.58%
inlay_hints 642.5±32.16µs 647.5±8.86µs +0.78%
on_enter 2.1±0.06µs 2.0±0.05µs -4.76%
parent_decl_at_position 4.0±0.18ms 3.8±0.10ms -5.00%
prepare_rename 338.9±15.71µs 352.5±23.60µs +4.01%
rename 10.1±0.26ms 9.7±0.22ms -3.96%
semantic_tokens 1169.0±27.84µs 1269.9±21.55µs +8.63%
token_at_position 342.5±3.17µs 345.1±4.46µs +0.76%
tokens_at_position 3.9±0.14ms 3.8±0.13ms -2.56%
tokens_for_file 401.9±4.16µs 408.7±6.62µs +1.69%
traverse 38.2±1.01ms 37.4±1.20ms -2.09%

@esdrubal esdrubal marked this pull request as ready for review August 20, 2024 08:20
@esdrubal esdrubal requested a review from a team as a code owner August 20, 2024 08:21
Copy link

Benchmark for a960996

Click to view benchmark
Test Base PR %
code_action 5.1±0.06ms 5.1±0.08ms 0.00%
code_lens 301.7±9.80ns 299.6±7.17ns -0.70%
compile 2.6±0.06s 2.7±0.04s +3.85%
completion 4.5±0.03ms 4.5±0.01ms 0.00%
did_change_with_caching 2.5±0.03s 2.5±0.03s 0.00%
document_symbol 926.5±17.94µs 866.7±14.97µs -6.45%
format 67.2±0.62ms 68.1±0.99ms +1.34%
goto_definition 336.8±8.48µs 336.9±2.61µs +0.03%
highlight 8.7±0.03ms 8.8±0.02ms +1.15%
hover 491.9±4.02µs 488.2±6.46µs -0.75%
idents_at_position 118.4±0.24µs 118.3±0.62µs -0.08%
inlay_hints 624.1±22.73µs 667.8±10.63µs +7.00%
on_enter 2.1±0.08µs 2.1±0.04µs 0.00%
parent_decl_at_position 3.6±0.03ms 3.6±0.03ms 0.00%
prepare_rename 334.8±10.56µs 334.3±5.59µs -0.15%
rename 9.0±0.07ms 9.1±0.20ms +1.11%
semantic_tokens 1246.7±12.27µs 1187.7±11.41µs -4.73%
token_at_position 339.2±3.11µs 331.4±2.41µs -2.30%
tokens_at_position 3.6±0.03ms 3.6±0.02ms 0.00%
tokens_for_file 400.9±2.53µs 400.1±3.79µs -0.20%
traverse 33.3±0.96ms 33.1±0.43ms -0.60%

@JoshuaBatty JoshuaBatty enabled auto-merge (squash) August 23, 2024 03:51
@JoshuaBatty JoshuaBatty merged commit 4f08020 into master Aug 23, 2024
37 checks passed
@JoshuaBatty JoshuaBatty deleted the esdrubal/6336_2 branch August 23, 2024 04:20
Copy link

Benchmark for e8f1d05

Click to view benchmark
Test Base PR %
code_action 5.1±0.23ms 5.1±0.18ms 0.00%
code_lens 286.7±3.74ns 290.9±8.52ns +1.46%
compile 2.7±0.03s 2.7±0.05s 0.00%
completion 4.5±0.06ms 4.6±0.10ms +2.22%
did_change_with_caching 2.6±0.04s 2.6±0.03s 0.00%
document_symbol 881.6±21.03µs 850.7±16.35µs -3.50%
format 68.3±1.06ms 68.0±0.70ms -0.44%
goto_definition 343.7±5.50µs 347.0±7.39µs +0.96%
highlight 8.7±0.14ms 8.8±0.34ms +1.15%
hover 495.5±6.27µs 501.6±5.92µs +1.23%
idents_at_position 120.5±0.40µs 120.7±0.44µs +0.17%
inlay_hints 629.9±30.24µs 630.1±10.82µs +0.03%
on_enter 1890.2±39.65ns 1959.1±88.48ns +3.65%
parent_decl_at_position 3.6±0.02ms 3.6±0.04ms 0.00%
prepare_rename 348.5±24.40µs 344.4±6.44µs -1.18%
rename 9.1±0.14ms 9.1±0.03ms 0.00%
semantic_tokens 1276.8±13.90µs 1212.2±13.74µs -5.06%
token_at_position 338.2±6.50µs 335.7±2.49µs -0.74%
tokens_at_position 3.6±0.03ms 3.6±0.04ms 0.00%
tokens_for_file 405.8±2.15µs 406.1±6.93µs +0.07%
traverse 35.2±1.14ms 35.0±0.42ms -0.57%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

panic on unwrapping in type_check_trait_implementation
4 participants