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

Refactor: Imports must use absolute paths #5697

Merged
merged 14 commits into from
Mar 8, 2024
Merged

Conversation

jjcnn
Copy link
Contributor

@jjcnn jjcnn commented Mar 5, 2024

Description

This PR contains the final bit of refactoring before I start fixing the import problems mentioned in #5498.

The semantics of imports has so far been implemented in the Module struct, and has assumed that the destination path of an import was relative to self. In particular, this allows imports to any submodule of the current module.

However, whenever that logic was triggered by the compiler the destination path was always absolute. The only reason this worked was that the logic was only ever triggered on the root module.

To reflect this behavior I have therefore moved the import semantics to the Root struct, and made it clear in the comments that the paths are assumed to be absolute.

This also reflects the fact that once the module structure has been populated with the imported entities we don't actually need the import logic anymore, so it shouldn't be located in the Module struct. (Name resolution should obviously stay in the Module struct, so that logic has not been moved).

I have left a couple of TODOs in the code to remind myself where I need to make changes when I start implementing pub use (see #3113 ).

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.

@jjcnn jjcnn requested a review from a team March 5, 2024 13:32
@jjcnn jjcnn requested a review from kayagokalp as a code owner March 5, 2024 13:32
Copy link

github-actions bot commented Mar 5, 2024

Benchmark for f86c780

Click to view benchmark
Test Base PR %
code_action 5.3±0.03ms 5.2±0.22ms -1.89%
code_lens 293.2±10.53ns 295.9±13.11ns +0.92%
compile 4.2±0.08s 4.5±0.21s +7.14%
completion 4.9±0.12ms 4.9±0.23ms 0.00%
did_change_with_caching 3.8±0.09s 3.9±0.10s +2.63%
document_symbol 1017.8±43.45µs 1039.9±16.93µs +2.17%
format 75.0±1.16ms 74.1±1.96ms -1.20%
goto_definition 368.1±5.86µs 366.5±7.75µs -0.43%
highlight 9.1±0.17ms 9.0±0.30ms -1.10%
hover 536.7±5.56µs 539.8±6.48µs +0.58%
idents_at_position 121.9±0.34µs 122.9±0.67µs +0.82%
inlay_hints 733.2±36.31µs 658.5±28.64µs -10.19%
on_enter 497.0±15.19ns 480.7±21.92ns -3.28%
parent_decl_at_position 3.7±0.05ms 3.6±0.02ms -2.70%
prepare_rename 408.1±5.53µs 364.1±8.49µs -10.78%
rename 9.5±0.07ms 9.3±0.36ms -2.11%
semantic_tokens 1014.0±13.22µs 1063.0±41.14µs +4.83%
token_at_position 359.3±2.55µs 354.8±3.19µs -1.25%
tokens_at_position 3.7±0.05ms 3.6±0.06ms -2.70%
tokens_for_file 408.4±2.21µs 409.8±5.52µs +0.34%
traverse 37.6±1.16ms 39.3±1.06ms +4.52%

Copy link

github-actions bot commented Mar 5, 2024

Benchmark for 595dfe2

Click to view benchmark
Test Base PR %
code_action 5.3±0.09ms 5.2±0.12ms -1.89%
code_lens 291.5±8.75ns 291.8±8.11ns +0.10%
compile 4.3±0.17s 4.4±0.13s +2.33%
completion 5.0±0.14ms 4.9±0.18ms -2.00%
did_change_with_caching 3.8±0.09s 3.7±0.10s -2.63%
document_symbol 1030.9±47.74µs 985.8±24.90µs -4.37%
format 74.0±2.19ms 74.0±0.76ms 0.00%
goto_definition 382.2±3.94µs 375.3±15.56µs -1.81%
highlight 9.1±0.24ms 9.1±0.32ms 0.00%
hover 547.2±8.73µs 552.2±13.79µs +0.91%
idents_at_position 122.5±0.47µs 124.9±1.49µs +1.96%
inlay_hints 658.5±10.74µs 659.7±26.13µs +0.18%
on_enter 486.4±7.15ns 497.4±15.06ns +2.26%
parent_decl_at_position 3.7±0.02ms 3.7±0.11ms 0.00%
prepare_rename 358.0±3.93µs 370.4±8.21µs +3.46%
rename 9.4±0.05ms 9.5±0.34ms +1.06%
semantic_tokens 1047.6±24.59µs 1077.5±33.95µs +2.85%
token_at_position 360.2±2.39µs 369.7±3.40µs +2.64%
tokens_at_position 3.7±0.02ms 4.0±0.24ms +8.11%
tokens_for_file 411.9±2.21µs 421.8±9.47µs +2.40%
traverse 38.5±0.55ms 39.0±1.20ms +1.30%

Copy link

github-actions bot commented Mar 5, 2024

Benchmark for 6c5cf66

Click to view benchmark
Test Base PR %
code_action 5.3±0.17ms 5.2±0.17ms -1.89%
code_lens 293.4±11.12ns 296.4±23.24ns +1.02%
compile 4.5±0.17s 4.5±0.10s 0.00%
completion 4.9±0.35ms 5.2±0.23ms +6.12%
did_change_with_caching 3.8±0.02s 3.8±0.04s 0.00%
document_symbol 962.3±25.69µs 1041.1±38.03µs +8.19%
format 74.4±0.67ms 74.4±0.87ms 0.00%
goto_definition 372.6±4.21µs 481.6±6.80µs +29.25%
highlight 9.1±0.13ms 9.2±0.21ms +1.10%
hover 544.9±5.97µs 648.0±9.03µs +18.92%
idents_at_position 121.6±0.72µs 122.6±1.31µs +0.82%
inlay_hints 654.4±18.48µs 658.4±29.60µs +0.61%
on_enter 482.4±10.68ns 487.6±10.72ns +1.08%
parent_decl_at_position 3.8±0.10ms 3.7±0.11ms -2.63%
prepare_rename 361.3±5.14µs 395.7±9.38µs +9.52%
rename 9.5±0.26ms 9.2±0.66ms -3.16%
semantic_tokens 1059.9±40.15µs 1046.8±17.40µs -1.24%
token_at_position 363.7±3.14µs 363.6±3.60µs -0.03%
tokens_at_position 3.7±0.06ms 3.7±0.09ms 0.00%
tokens_for_file 407.1±2.64µs 407.4±2.27µs +0.07%
traverse 38.8±1.37ms 39.7±0.86ms +2.32%

Copy link

github-actions bot commented Mar 5, 2024

Benchmark for 701f4ae

Click to view benchmark
Test Base PR %
code_action 5.6±0.14ms 5.2±0.12ms -7.14%
code_lens 290.9±10.58ns 293.2±8.46ns +0.79%
compile 4.6±0.13s 4.5±0.15s -2.17%
completion 5.3±0.15ms 4.8±0.13ms -9.43%
did_change_with_caching 3.8±0.09s 3.7±0.04s -2.63%
document_symbol 1037.7±26.54µs 1042.0±24.34µs +0.41%
format 74.5±1.16ms 74.5±1.33ms 0.00%
goto_definition 370.0±7.90µs 372.3±5.46µs +0.62%
highlight 9.7±0.24ms 8.8±0.12ms -9.28%
hover 543.8±10.31µs 546.0±8.05µs +0.40%
idents_at_position 123.2±0.74µs 122.6±1.32µs -0.49%
inlay_hints 667.0±25.29µs 660.5±12.72µs -0.97%
on_enter 487.6±18.12ns 477.3±15.34ns -2.11%
parent_decl_at_position 3.9±0.16ms 3.6±0.07ms -7.69%
prepare_rename 369.8±8.00µs 365.5±7.82µs -1.16%
rename 9.9±0.11ms 9.2±0.20ms -7.07%
semantic_tokens 1044.3±12.15µs 1067.5±12.96µs +2.22%
token_at_position 363.9±6.45µs 360.3±2.69µs -0.99%
tokens_at_position 3.7±0.03ms 3.6±0.07ms -2.70%
tokens_for_file 413.1±2.06µs 413.3±14.74µs +0.05%
traverse 40.1±0.91ms 39.0±1.05ms -2.74%

tritao
tritao previously approved these changes Mar 6, 2024
Copy link
Contributor

@tritao tritao left a comment

Choose a reason for hiding this comment

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

Nice 👍

@tritao tritao added 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 labels Mar 6, 2024
IGI-111
IGI-111 previously approved these changes Mar 7, 2024
@jjcnn jjcnn dismissed stale reviews from IGI-111 and tritao via 5576afb March 7, 2024 12:18
Copy link

github-actions bot commented Mar 7, 2024

Benchmark for 5778c2d

Click to view benchmark
Test Base PR %
code_action 5.5±0.19ms 5.3±0.14ms -3.64%
code_lens 286.9±9.57ns 286.9±13.07ns 0.00%
compile 5.9±0.09s 5.9±0.08s 0.00%
completion 5.2±0.20ms 4.9±0.17ms -5.77%
did_change_with_caching 5.4±0.05s 5.4±0.05s 0.00%
document_symbol 996.7±48.80µs 951.9±19.51µs -4.49%
format 70.1±0.75ms 70.0±1.13ms -0.14%
goto_definition 362.8±5.34µs 358.0±6.20µs -1.32%
highlight 9.1±0.09ms 8.8±0.05ms -3.30%
hover 588.3±9.55µs 585.1±11.11µs -0.54%
idents_at_position 122.7±0.41µs 121.3±0.57µs -1.14%
inlay_hints 666.6±13.62µs 661.1±12.51µs -0.83%
on_enter 501.3±16.36ns 502.2±22.41ns +0.18%
parent_decl_at_position 3.8±0.05ms 3.6±0.04ms -5.26%
prepare_rename 363.1±4.54µs 362.3±6.44µs -0.22%
rename 9.7±0.22ms 9.3±0.31ms -4.12%
semantic_tokens 1018.2±19.42µs 1055.7±26.19µs +3.68%
token_at_position 369.3±2.16µs 360.5±2.29µs -2.38%
tokens_at_position 3.8±0.03ms 3.6±0.07ms -5.26%
tokens_for_file 416.3±3.60µs 417.2±3.86µs +0.22%
traverse 44.6±1.34ms 45.1±1.26ms +1.12%

@IGI-111 IGI-111 requested a review from tritao March 8, 2024 09:48
@IGI-111 IGI-111 enabled auto-merge (squash) March 8, 2024 09:48
Copy link

github-actions bot commented Mar 8, 2024

Benchmark for 3c82d13

Click to view benchmark
Test Base PR %
code_action 5.4±0.13ms 5.3±0.03ms -1.85%
code_lens 295.1±7.05ns 291.4±7.75ns -1.25%
compile 5.8±0.08s 5.7±0.06s -1.72%
completion 4.9±0.10ms 4.9±0.02ms 0.00%
did_change_with_caching 5.2±0.03s 5.2±0.05s 0.00%
document_symbol 1013.0±42.28µs 954.7±6.63µs -5.76%
format 69.8±1.19ms 70.8±0.99ms +1.43%
goto_definition 362.1±6.34µs 358.0±6.37µs -1.13%
highlight 9.2±0.18ms 9.1±0.04ms -1.09%
hover 590.7±7.18µs 582.9±9.94µs -1.32%
idents_at_position 122.6±0.45µs 122.9±0.91µs +0.24%
inlay_hints 675.3±10.56µs 659.6±24.19µs -2.32%
on_enter 493.4±19.22ns 495.6±17.53ns +0.45%
parent_decl_at_position 3.7±0.01ms 3.7±0.11ms 0.00%
prepare_rename 357.7±2.34µs 358.1±5.43µs +0.11%
rename 9.6±0.23ms 9.5±0.17ms -1.04%
semantic_tokens 1050.2±11.57µs 1049.5±24.48µs -0.07%
token_at_position 356.6±10.17µs 351.7±2.06µs -1.37%
tokens_at_position 3.7±0.02ms 3.7±0.03ms 0.00%
tokens_for_file 412.4±2.23µs 411.6±2.03µs -0.19%
traverse 42.9±1.83ms 42.9±1.44ms 0.00%

@IGI-111 IGI-111 merged commit 13731d9 into master Mar 8, 2024
32 checks passed
@IGI-111 IGI-111 deleted the jjcnn/import-semantics branch March 8, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants