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.

Sign up for GitHub

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

Jump to bottom

Fix and improve Unimplemented compile errors #5847

Merged
merged 6 commits into from
Apr 15, 2024
Merged

Fix and improve Unimplemented compile errors #5847

merged 6 commits into from
Apr 15, 2024

Conversation

ironcev
Copy link
Member

@ironcev ironcev commented Apr 12, 2024 •
edited
Loading

Description

This PR:

  • closes Mutable variable is considered immutable #5803 by providing the proper error message that mut is not implemented in pattern matching. Support for ref, mut, and ref mut in pattern matching will be done later as a part of support for references in pattern matching.
  • closes Unimplemented Error not emitted when matching on aliases #5846 by checking if aliased type can be matched.
  • provides Diagnostic for the Unimplemented error and removes the UnimplementedWithHelp error.
  • differentiates between unimplemented features and features that are actually semantically not supported. Previously, Unimplemented error was used for both which emitted misleading errors in match expressions, using module paths in place of expressions, and trait implementations.

Closes #5803, #5846.

Demo

Before:
Self type of an impl block is not valid - Before

After:
Self type of an impl block is not valid - After

Before:
Module path is not an expression - Before

After:
Module path is not an expression - After

Diagnostic for Unimplemented error:
Matching on this type is currently not implemented - After

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.

@ironcev ironcev self-assigned this Apr 12, 2024
@ironcev ironcev added bug Something isn't working compiler General compiler. Should eventually become more specific as the issue is triaged compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: ui Mostly compiler messages labels Apr 12, 2024
@github-actions GitHub Actions
Copy link

Benchmark for a1919c0

Click to view benchmark
Test Base PR %
code_action 5.5±0.17ms 5.5±0.10ms 0.00%
code_lens 286.1±10.90ns 287.6±7.94ns +0.52%
compile 6.2±0.05s 6.1±0.08s -1.61%
completion 4.9±0.10ms 4.9±0.10ms 0.00%
did_change_with_caching 6.2±0.12s 6.0±0.08s -3.23%
document_symbol 948.3±19.26µs 968.2±14.37µs +2.10%
format 75.9±0.85ms 71.3±2.02ms -6.06%
goto_definition 365.0±11.52µs 365.0±8.20µs 0.00%
highlight 9.0±0.10ms 9.0±0.18ms 0.00%
hover 622.9±45.05µs 604.1±18.64µs -3.02%
idents_at_position 121.4±0.40µs 122.2±1.01µs +0.66%
inlay_hints 667.2±19.22µs 657.0±33.02µs -1.53%
on_enter 476.5±7.86ns 481.5±18.64ns +1.05%
parent_decl_at_position 3.7±0.03ms 3.7±0.18ms 0.00%
prepare_rename 365.8±17.95µs 358.9±5.67µs -1.89%
rename 9.7±0.25ms 9.5±0.17ms -2.06%
semantic_tokens 1051.6±10.98µs 1103.1±11.38µs +4.90%
token_at_position 358.4±14.34µs 359.4±3.99µs +0.28%
tokens_at_position 3.7±0.11ms 3.7±0.05ms 0.00%
tokens_for_file 415.6±3.03µs 409.1±3.49µs -1.56%
traverse 50.0±1.58ms 50.3±1.61ms +0.60%

@github-actions GitHub Actions
Copy link

Benchmark for eba608c

Click to view benchmark
Test Base PR %
code_action 5.5±0.01ms 5.5±0.11ms 0.00%
code_lens 286.9±7.63ns 288.3±9.06ns +0.49%
compile 6.2±0.11s 6.0±0.03s -3.23%
completion 4.9±0.02ms 5.0±0.10ms +2.04%
did_change_with_caching 5.9±0.03s 6.1±0.05s +3.39%
document_symbol 956.6±20.42µs 964.8±15.49µs +0.86%
format 75.2±1.15ms 69.8±1.16ms -7.18%
goto_definition 375.1±5.59µs 359.6±6.32µs -4.13%
highlight 9.1±0.14ms 9.1±0.13ms 0.00%
hover 614.3±23.52µs 600.6±25.09µs -2.23%
idents_at_position 123.4±0.70µs 121.6±1.22µs -1.46%
inlay_hints 668.4±7.19µs 659.7±26.38µs -1.30%
on_enter 477.8±16.22ns 484.4±8.09ns +1.38%
parent_decl_at_position 3.7±0.01ms 3.7±0.03ms 0.00%
prepare_rename 362.2±3.89µs 358.0±3.44µs -1.16%
rename 9.6±0.13ms 9.5±0.15ms -1.04%
semantic_tokens 1063.5±14.46µs 1036.6±9.28µs -2.53%
token_at_position 364.3±5.56µs 352.8±2.03µs -3.16%
tokens_at_position 3.8±0.20ms 3.7±0.03ms -2.63%
tokens_for_file 412.9±2.60µs 415.0±12.62µs +0.51%
traverse 49.8±1.39ms 49.9±1.56ms +0.20%

@ironcev ironcev marked this pull request as ready for review April 12, 2024 11:22
@ironcev ironcev requested review from a team April 12, 2024 11:22
IGI-111
IGI-111 previously approved these changes Apr 12, 2024
View reviewed changes
@IGI-111 IGI-111 requested a review from a team April 12, 2024 14:11
@github-actions GitHub Actions
Copy link

Benchmark for 07cdcca

Click to view benchmark
Test Base PR %
code_action 5.5±0.02ms 5.5±0.09ms 0.00%
code_lens 297.6±8.14ns 291.5±7.88ns -2.05%
compile 6.0±0.03s 6.1±0.14s +1.67%
completion 4.9±0.09ms 4.9±0.08ms 0.00%
did_change_with_caching 6.0±0.06s 5.9±0.08s -1.67%
document_symbol 957.6±16.17µs 969.1±15.46µs +1.20%
format 76.8±1.17ms 70.8±1.12ms -7.81%
goto_definition 372.3±9.53µs 367.1±3.86µs -1.40%
highlight 9.1±0.15ms 9.0±0.14ms -1.10%
hover 617.2±21.46µs 624.9±19.62µs +1.25%
idents_at_position 124.0±0.98µs 122.4±0.78µs -1.29%
inlay_hints 678.1±19.70µs 665.6±25.85µs -1.84%
on_enter 471.7±13.25ns 491.5±8.02ns +4.20%
parent_decl_at_position 3.7±0.04ms 3.7±0.03ms 0.00%
prepare_rename 367.3±8.00µs 358.8±4.02µs -2.31%
rename 9.6±0.02ms 9.5±0.03ms -1.04%
semantic_tokens 1030.8±20.13µs 1090.1±17.43µs +5.75%
token_at_position 367.4±1.18µs 358.0±2.38µs -2.56%
tokens_at_position 3.7±0.01ms 3.7±0.04ms 0.00%
tokens_for_file 413.2±5.68µs 427.2±3.18µs +3.39%
traverse 50.4±1.68ms 49.4±2.36ms -1.98%

jjcnn
jjcnn previously approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@jjcnn jjcnn left a comment

Choose a reason for hiding this comment

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

A few nits, but otherwise LGTM.

sway-ast/src/pattern.rs Show resolved Hide resolved
sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs Outdated Show resolved Hide resolved
@ironcev ironcev enabled auto-merge (squash) April 13, 2024 06:00
@github-actions GitHub Actions
Copy link

Benchmark for dc719a8

Click to view benchmark
Test Base PR %
code_action 5.3±0.03ms 5.4±0.10ms +1.89%
code_lens 291.3±7.28ns 333.3±11.66ns +14.42%
compile 6.1±0.05s 6.2±0.06s +1.64%
completion 4.8±0.10ms 4.9±0.02ms +2.08%
did_change_with_caching 6.1±0.08s 6.0±0.06s -1.64%
document_symbol 943.9±18.33µs 969.3±11.66µs +2.69%
format 72.0±1.02ms 74.7±1.47ms +3.75%
goto_definition 371.2±9.44µs 362.7±4.48µs -2.29%
highlight 8.7±0.05ms 9.0±0.16ms +3.45%
hover 607.1±18.18µs 598.0±21.28µs -1.50%
idents_at_position 122.9±0.41µs 121.3±0.99µs -1.30%
inlay_hints 649.1±18.95µs 666.5±34.56µs +2.68%
on_enter 481.4±12.87ns 472.8±11.95ns -1.79%
parent_decl_at_position 3.6±0.04ms 3.7±0.03ms +2.78%
prepare_rename 366.7±6.98µs 362.2±6.20µs -1.23%
rename 9.2±0.08ms 9.5±0.16ms +3.26%
semantic_tokens 1049.0±23.45µs 1046.8±24.33µs -0.21%
token_at_position 364.8±2.49µs 360.3±2.21µs -1.23%
tokens_at_position 3.6±0.02ms 3.7±0.04ms +2.78%
tokens_for_file 421.4±1.95µs 418.2±4.06µs -0.76%
traverse 51.8±3.62ms 52.3±1.71ms +0.97%

xunilrj
xunilrj previously approved these changes Apr 13, 2024
View reviewed changes
@ironcev ironcev dismissed stale reviews from xunilrj, jjcnn, and IGI-111 via b0a7021 April 13, 2024 12:37
@github-actions GitHub Actions
Copy link

Benchmark for ac1ea19

Click to view benchmark
Test Base PR %
code_action 5.4±0.17ms 5.3±0.10ms -1.85%
code_lens 292.9±11.25ns 293.5±12.62ns +0.20%
compile 6.2±0.08s 6.1±0.06s -1.61%
completion 4.8±0.15ms 4.8±0.10ms 0.00%
did_change_with_caching 6.1±0.04s 6.0±0.06s -1.64%
document_symbol 955.1±18.01µs 962.0±11.36µs +0.72%
format 72.8±0.71ms 76.6±0.73ms +5.22%
goto_definition 358.1±7.90µs 367.2±8.19µs +2.54%
highlight 8.7±0.14ms 8.7±0.21ms 0.00%
hover 605.5±8.20µs 616.8±11.13µs +1.87%
idents_at_position 122.6±0.35µs 122.0±0.87µs -0.49%
inlay_hints 652.2±23.91µs 662.4±35.44µs +1.56%
on_enter 481.3±12.69ns 481.3±15.81ns 0.00%
parent_decl_at_position 3.6±0.02ms 3.6±0.04ms 0.00%
prepare_rename 357.0±11.49µs 369.9±4.79µs +3.61%
rename 9.2±0.24ms 9.2±0.01ms 0.00%
semantic_tokens 1033.1±24.86µs 1050.1±14.46µs +1.65%
token_at_position 352.5±4.04µs 362.3±2.14µs +2.78%
tokens_at_position 3.7±0.04ms 3.6±0.03ms -2.70%
tokens_for_file 426.9±3.47µs 409.5±5.11µs -4.08%
traverse 50.1±2.82ms 49.5±1.26ms -1.20%

@ironcev ironcev merged commit c6948e2 into master Apr 15, 2024
36 checks passed
@ironcev ironcev deleted the ironcev/5803-mut-in-match-expressions branch April 15, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xunilrj xunilrj xunilrj approved these changes

@IGI-111 IGI-111 IGI-111 approved these changes

@jjcnn jjcnn Awaiting requested review from jjcnn

Assignees

@ironcev ironcev

Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: ui Mostly compiler messages compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unimplemented Error not emitted when matching on aliases Mutable variable is considered immutable
4 participants
@ironcev @xunilrj @IGI-111 @jjcnn