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

Eliminate alias map from lexical scope #5931

Merged
merged 6 commits into from
May 2, 2024
Merged

Conversation

jjcnn
Copy link
Contributor

@jjcnn jjcnn commented Apr 27, 2024

Description

Fixes #5912, #5713 .

Item imports with aliases use x::y as z have so far been represented using two maps:

  • A synonyms map mapping y to the type declaration of x::y.
  • An alias map mapping z to y.

This is confusing, since y is not actually bound by this import, and unsurprisingly has lead to a bug because of a name capture (see #5713).

This PR eliminates the alias mapping, and changes the synonyms map to map z to the type declaration of x::y.

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.

Copy link

Benchmark for 68891c7

Click to view benchmark
Test Base PR %
code_action 5.3±0.15ms 5.4±0.11ms +1.89%
code_lens 290.5±8.20ns 300.5±11.23ns +3.44%
compile 6.2±0.03s 6.2±0.02s 0.00%
completion 4.8±0.07ms 4.8±0.08ms 0.00%
did_change_with_caching 6.2±0.10s 6.1±0.05s -1.61%
document_symbol 996.3±38.03µs 942.3±26.68µs -5.42%
format 79.7±1.57ms 81.7±1.74ms +2.51%
goto_definition 371.2±26.72µs 364.0±6.90µs -1.94%
highlight 8.8±0.16ms 8.7±0.19ms -1.14%
hover 609.0±23.78µs 599.2±17.85µs -1.61%
idents_at_position 122.7±0.76µs 123.7±2.68µs +0.81%
inlay_hints 666.0±13.26µs 660.0±29.12µs -0.90%
on_enter 496.4±11.72ns 481.3±12.76ns -3.04%
parent_decl_at_position 3.6±0.02ms 3.6±0.03ms 0.00%
prepare_rename 362.4±9.72µs 361.9±10.50µs -0.14%
rename 9.3±0.17ms 9.2±0.17ms -1.08%
semantic_tokens 1008.9±16.51µs 1030.1±17.23µs +2.10%
token_at_position 359.7±2.52µs 354.4±2.38µs -1.47%
tokens_at_position 3.6±0.02ms 3.6±0.04ms 0.00%
tokens_for_file 421.4±4.45µs 421.2±2.56µs -0.05%
traverse 51.5±1.68ms 50.9±2.38ms -1.17%

Base automatically changed from jjcnn/move-name-resolution-to-root to master May 1, 2024 01:13
@jjcnn jjcnn requested review from tritao and a team May 1, 2024 11:25
@jjcnn jjcnn marked this pull request as ready for review May 1, 2024 11:25
Copy link

github-actions bot commented May 1, 2024

Benchmark for 1858aae

Click to view benchmark
Test Base PR %
code_action 5.2±0.18ms 5.3±0.16ms +1.92%
code_lens 288.9±14.15ns 320.7±17.53ns +11.01%
compile 6.0±0.05s 6.0±0.06s 0.00%
completion 4.7±0.12ms 4.8±0.20ms +2.13%
did_change_with_caching 5.9±0.07s 6.0±0.06s +1.69%
document_symbol 963.0±23.27µs 906.9±30.12µs -5.83%
format 83.0±2.01ms 83.1±2.02ms +0.12%
goto_definition 346.2±11.32µs 347.8±8.52µs +0.46%
highlight 8.5±0.21ms 8.8±0.29ms +3.53%
hover 585.9±16.60µs 590.5±25.36µs +0.79%
idents_at_position 119.9±2.71µs 120.1±2.95µs +0.17%
inlay_hints 628.6±30.36µs 626.5±18.22µs -0.33%
on_enter 462.5±11.60ns 466.9±16.80ns +0.95%
parent_decl_at_position 3.5±0.08ms 3.6±0.07ms +2.86%
prepare_rename 348.3±9.63µs 356.4±10.06µs +2.33%
rename 8.9±0.32ms 9.2±0.24ms +3.37%
semantic_tokens 993.0±24.06µs 1015.9±27.11µs +2.31%
token_at_position 343.7±7.23µs 348.5±7.79µs +1.40%
tokens_at_position 3.5±0.08ms 3.6±0.07ms +2.86%
tokens_for_file 418.5±10.00µs 416.5±9.84µs -0.48%
traverse 49.1±1.52ms 50.0±2.27ms +1.83%

@tritao
Copy link
Contributor

tritao commented May 1, 2024

I think you may need to rebase this. some commits slipped right through during the merge, like 8642ba3.

@jjcnn jjcnn force-pushed the jjcnn/eliminate-alias-map branch from f289ded to 027fb10 Compare May 1, 2024 14:55
Copy link

github-actions bot commented May 1, 2024

Benchmark for fb998c5

Click to view benchmark
Test Base PR %
code_action 5.3±0.01ms 5.5±0.02ms +3.77%
code_lens 338.5±8.08ns 288.8±9.07ns -14.68%
compile 6.3±0.07s 6.3±0.05s 0.00%
completion 4.8±0.09ms 5.0±0.02ms +4.17%
did_change_with_caching 6.2±0.07s 6.1±0.05s -1.61%
document_symbol 989.2±26.45µs 1027.2±34.73µs +3.84%
format 91.3±1.64ms 89.0±1.46ms -2.52%
goto_definition 357.5±10.31µs 382.2±5.99µs +6.91%
highlight 8.7±0.16ms 9.0±0.01ms +3.45%
hover 610.6±16.92µs 620.1±27.52µs +1.56%
idents_at_position 125.2±0.53µs 123.4±1.06µs -1.44%
inlay_hints 655.7±21.64µs 679.2±14.74µs +3.58%
on_enter 482.0±8.76ns 494.5±21.25ns +2.59%
parent_decl_at_position 3.6±0.06ms 3.7±0.04ms +2.78%
prepare_rename 365.8±7.13µs 380.2±7.43µs +3.94%
rename 9.3±0.16ms 9.5±0.02ms +2.15%
semantic_tokens 1055.0±25.33µs 1038.7±17.45µs -1.55%
token_at_position 364.5±2.65µs 363.7±2.73µs -0.22%
tokens_at_position 3.6±0.03ms 3.7±0.03ms +2.78%
tokens_for_file 435.9±3.83µs 421.3±4.30µs -3.35%
traverse 51.2±2.42ms 49.4±2.29ms -3.52%

@jjcnn jjcnn force-pushed the jjcnn/eliminate-alias-map branch from 027fb10 to 070f884 Compare May 1, 2024 15:35
Copy link

github-actions bot commented May 1, 2024

Benchmark for 28aee26

Click to view benchmark
Test Base PR %
code_action 5.4±0.08ms 5.6±0.32ms +3.70%
code_lens 337.8±6.71ns 288.4±7.08ns -14.62%
compile 6.6±0.11s 6.5±0.05s -1.52%
completion 5.0±0.16ms 5.5±0.34ms +10.00%
did_change_with_caching 6.4±0.07s 6.4±0.16s 0.00%
document_symbol 1037.1±30.82µs 987.7±39.57µs -4.76%
format 92.2±1.16ms 88.0±0.61ms -4.56%
goto_definition 368.2±10.43µs 353.4±6.62µs -4.02%
highlight 8.8±0.14ms 9.0±0.20ms +2.27%
hover 638.5±15.80µs 603.2±20.45µs -5.53%
idents_at_position 122.9±0.54µs 123.5±0.45µs +0.49%
inlay_hints 660.6±16.88µs 660.4±16.38µs -0.03%
on_enter 485.9±13.50ns 487.4±8.93ns +0.31%
parent_decl_at_position 3.6±0.05ms 3.7±0.05ms +2.78%
prepare_rename 373.2±7.86µs 359.9±5.53µs -3.56%
rename 9.5±0.50ms 9.7±0.53ms +2.11%
semantic_tokens 1061.5±10.05µs 1024.6±21.54µs -3.48%
token_at_position 358.5±7.02µs 348.3±7.08µs -2.85%
tokens_at_position 3.6±0.04ms 3.7±0.04ms +2.78%
tokens_for_file 428.5±1.04µs 419.6±3.72µs -2.08%
traverse 53.2±1.28ms 51.9±2.37ms -2.44%

@jjcnn
Copy link
Contributor Author

jjcnn commented May 1, 2024

@tritao : Fixed.

Copy link

github-actions bot commented May 2, 2024

Benchmark for bb1709d

Click to view benchmark
Test Base PR %
code_action 5.4±0.03ms 5.5±0.08ms +1.85%
code_lens 338.6±8.78ns 294.5±14.97ns -13.02%
compile 6.4±0.37s 6.5±0.09s +1.56%
completion 4.9±0.10ms 5.0±0.02ms +2.04%
did_change_with_caching 6.2±0.10s 6.2±0.07s 0.00%
document_symbol 1023.7±18.80µs 947.4±46.13µs -7.45%
format 91.0±0.82ms 88.9±0.72ms -2.31%
goto_definition 366.0±9.68µs 369.5±7.49µs +0.96%
highlight 8.7±0.15ms 9.0±0.06ms +3.45%
hover 608.2±14.51µs 623.4±47.94µs +2.50%
idents_at_position 123.4±0.48µs 123.6±1.09µs +0.16%
inlay_hints 651.5±9.20µs 674.7±21.67µs +3.56%
on_enter 481.2±17.66ns 498.8±33.15ns +3.66%
parent_decl_at_position 3.6±0.23ms 3.7±0.05ms +2.78%
prepare_rename 361.4±4.48µs 362.6±7.91µs +0.33%
rename 9.6±0.26ms 9.9±0.37ms +3.13%
semantic_tokens 1059.8±22.57µs 1045.3±9.20µs -1.37%
token_at_position 354.1±1.91µs 355.1±3.49µs +0.28%
tokens_at_position 3.6±0.03ms 3.7±0.13ms +2.78%
tokens_for_file 425.9±2.63µs 415.3±4.06µs -2.49%
traverse 50.8±1.19ms 51.3±1.44ms +0.98%

@tritao tritao merged commit 722c95c into master May 2, 2024
37 checks passed
@tritao tritao deleted the jjcnn/eliminate-alias-map branch May 2, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eliminate Items::use_aliases
3 participants