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 dummy method while using where clause in trait impl. #5684

Merged
merged 2 commits into from Mar 4, 2024

Conversation

esdrubal
Copy link
Contributor

@esdrubal esdrubal commented Mar 4, 2024

Description

When using nested generic methods, as in the use case added, the lower method would not be properly replaced and would remain as a trait dummy method.

The fix was to do find_method_for_type in the ReplaceDecls of FunctionAplication, this allows the replacement of the function application dummy method with a proper method implementation.

Fixes #5673

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.

When using nested generic methods, as in the use case added, the lower method
would not be properly replaced and would remain as a trait dummy method.

The fix was to do find_method_for_type in the ReplaceDecls of FunctionAplication,
this allows the replacement of the function aplication dummy method with
a proper method implementation.

Fixes #5673
@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 Mar 4, 2024
@esdrubal esdrubal self-assigned this Mar 4, 2024
Copy link

github-actions bot commented Mar 4, 2024

Benchmark for c996ad1

Click to view benchmark
Test Base PR %
code_action 5.2±0.23ms 5.3±0.11ms +1.92%
code_lens 309.5±22.91ns 291.6±8.10ns -5.78%
compile 4.3±0.13s 4.4±0.11s +2.33%
completion 4.7±0.16ms 4.9±0.10ms +4.26%
did_change_with_caching 3.8±0.12s 3.7±0.06s -2.63%
document_symbol 1007.3±81.34µs 1052.9±53.61µs +4.53%
format 73.4±1.51ms 73.8±2.57ms +0.54%
goto_definition 368.1±8.26µs 371.2±18.91µs +0.84%
highlight 8.8±0.11ms 9.1±0.13ms +3.41%
hover 541.9±10.91µs 541.4±6.54µs -0.09%
idents_at_position 122.3±0.42µs 121.7±0.51µs -0.49%
inlay_hints 658.3±58.96µs 667.1±25.52µs +1.34%
on_enter 482.2±14.61ns 483.3±15.69ns +0.23%
parent_decl_at_position 3.6±0.05ms 3.7±0.05ms +2.78%
prepare_rename 364.7±11.00µs 363.9±5.56µs -0.22%
rename 9.1±0.28ms 9.5±0.18ms +4.40%
semantic_tokens 1043.2±11.12µs 1037.0±13.52µs -0.59%
token_at_position 356.5±8.53µs 360.6±2.52µs +1.15%
tokens_at_position 3.6±0.04ms 3.7±0.05ms +2.78%
tokens_for_file 410.1±6.90µs 414.4±8.82µs +1.05%
traverse 38.3±1.24ms 38.4±1.37ms +0.26%

@esdrubal esdrubal marked this pull request as ready for review March 4, 2024 12:33
@esdrubal esdrubal requested a review from a team March 4, 2024 12:33
@tritao tritao enabled auto-merge (squash) March 4, 2024 21:51
Copy link

github-actions bot commented Mar 4, 2024

Benchmark for 2c30792

Click to view benchmark
Test Base PR %
code_action 5.2±0.12ms 5.3±0.15ms +1.92%
code_lens 304.3±8.18ns 290.8±5.70ns -4.44%
compile 4.2±0.06s 4.2±0.07s 0.00%
completion 4.7±0.09ms 4.9±0.41ms +4.26%
did_change_with_caching 3.8±0.11s 3.9±0.11s +2.63%
document_symbol 1021.4±45.27µs 1006.5±42.59µs -1.46%
format 73.1±1.20ms 73.2±1.05ms +0.14%
goto_definition 368.1±7.88µs 370.4±7.72µs +0.62%
highlight 8.7±0.03ms 9.3±0.31ms +6.90%
hover 537.1±15.13µs 541.6±9.64µs +0.84%
idents_at_position 123.9±0.41µs 121.7±1.33µs -1.78%
inlay_hints 652.1±27.75µs 664.8±27.77µs +1.95%
on_enter 529.0±12.05ns 486.0±21.19ns -8.13%
parent_decl_at_position 3.6±0.04ms 3.8±0.07ms +5.56%
prepare_rename 368.0±7.15µs 368.9±6.87µs +0.24%
rename 9.1±0.19ms 9.4±0.13ms +3.30%
semantic_tokens 1032.0±22.43µs 1070.6±16.95µs +3.74%
token_at_position 362.5±2.22µs 369.3±6.22µs +1.88%
tokens_at_position 3.6±0.04ms 3.7±0.06ms +2.78%
tokens_for_file 412.0±3.09µs 417.5±6.58µs +1.33%
traverse 37.4±1.56ms 37.8±1.04ms +1.07%

@tritao tritao merged commit 6ee40a3 into master Mar 4, 2024
31 checks passed
@tritao tritao deleted the esdrubal/dummy_method_issue branch March 4, 2024 22:24
@xunilrj xunilrj mentioned this pull request Mar 18, 2024
7 tasks
xunilrj added a commit that referenced this pull request Mar 20, 2024
## Description

This PR contains three fixes:

1 - Fix to partial monomorphization. The fix was just removing a
"quick-fix" that was merged and it is not needed anymore after
#5684;
2 - To play safe, `compile_fn_call` cache will not be used for
autogenerated code;
3 - `experimental-new-encoding` was not being copied in all cases.

Two other small changes are:

1 - `Buffer` struct does not offer a generic `push` anymore, because
this function was extremely deceiving for primitive datatypes which have
memory layouts very different from the encoding memory layout (u16, u32
etc...);
2 - Changing name of one test to make the test more obvious and easier
to run just one specific test.

## 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](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.
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.

Problem replacing dummy trait methods
3 participants