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

Adds reuse of enum variant TypeId. #5641

Closed
wants to merge 1 commit into from

Conversation

esdrubal
Copy link
Contributor

@esdrubal esdrubal commented Feb 21, 2024

Description

We already do unification of expressions with type annotation when necessary, but these were not affecting enum variants because we used separate TypeIds for the expression and for the enum variant.

With this change the enum variant and the expressions will share the same TypeId, making unification to the expression to also affect the enum variant.

Fixes #5492
Fixes #5581

Probably helpful for #5559

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.

We already do unification of expressions with type annotation when necessary,
but these were not affecting enum variants because we used separate TypeIds for
the expression and for the enum variant.

With this change the enum variant and the expressions will share the same TypeId,
making unification to the expression to also affect the enum variant.

Fixes #5492
Fixes #5581

Probably helpful for #5559
@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 Feb 21, 2024
@esdrubal esdrubal self-assigned this Feb 21, 2024
Copy link

Benchmark for d19994b

Click to view benchmark
Test Base PR %
code_action 5.1±0.12ms 5.4±0.15ms +5.88%
code_lens 287.8±3.38ns 308.5±18.08ns +7.19%
compile 3.0±0.04s 3.0±0.03s 0.00%
completion 4.9±0.68ms 4.8±0.15ms -2.04%
did_change_with_caching 2.9±0.04s 2.8±0.03s -3.45%
document_symbol 985.9±39.20µs 983.3±26.21µs -0.26%
format 75.0±3.51ms 74.6±1.66ms -0.53%
goto_definition 356.9±7.06µs 358.9±5.24µs +0.56%
highlight 8.8±0.13ms 9.1±0.12ms +3.41%
hover 534.8±7.62µs 530.8±7.06µs -0.75%
idents_at_position 121.7±0.32µs 121.6±1.16µs -0.08%
inlay_hints 654.2±46.79µs 664.6±18.68µs +1.59%
on_enter 497.5±11.24ns 483.7±12.34ns -2.77%
parent_decl_at_position 3.6±0.03ms 3.7±0.05ms +2.78%
prepare_rename 359.3±13.01µs 362.2±7.10µs +0.81%
rename 9.1±0.19ms 9.4±0.17ms +3.30%
semantic_tokens 1052.2±13.74µs 1028.2±10.33µs -2.28%
token_at_position 355.7±2.38µs 360.0±2.38µs +1.21%
tokens_at_position 3.7±0.66ms 3.7±0.03ms 0.00%
tokens_for_file 410.6±9.02µs 410.0±7.03µs -0.15%
traverse 37.6±0.69ms 38.3±1.18ms +1.86%

@esdrubal esdrubal marked this pull request as ready for review February 21, 2024 10:03
@esdrubal esdrubal requested a review from a team February 21, 2024 10:04
Copy link
Member

@ironcev ironcev left a comment

Choose a reason for hiding this comment

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

This improves just one single case but makes worst many others. E.g,

let _: Enum<u8, _> = Enum<_, bool>::A(123);

There is a PR coming that fixes the mentioned PRs in a more detailed way. Currently we are unfortunately stepping on each others toes 😄

@esdrubal esdrubal marked this pull request as draft February 21, 2024 12:54
@esdrubal
Copy link
Contributor Author

Converted this back to a Draft until the better PR that @ironcev is working on is available. Then we should be able to close this.

@ironcev
Copy link
Member

ironcev commented Feb 23, 2024

Closed in #5643.

@ironcev ironcev closed this Feb 23, 2024
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
2 participants