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 method application argument type checking. #5632

Merged

Conversation

esdrubal
Copy link
Contributor

@esdrubal esdrubal commented Feb 19, 2024

Description

While doing the argument type checking we set the type annotation to Unknown because the method has not been resolved yet.

With this change when we fail to type check an argument expression with the annotation Unknown we try again after resolving the method name.

The reason we do the double pass on argument type checking is because we need the types from the arguments to resolve the method but sometimes we need the method resolved for resolving the argument expressions with the type parameters.

Fixes #5614

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.

While doing the argument type checking we set the type annotation to Unknown
 because the method has not been resolved yet.

With this change when we fail to type check an argument expression
with the annotation Unknown we try again after resolving the method name.

The reason we do the double pass on argument type checking is because we
need the types from the arguments to resolve the method but sometimes we
need the method resolved for resolving the argument expressions with the type parameters.
@esdrubal esdrubal added the compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen label Feb 19, 2024
@esdrubal esdrubal self-assigned this Feb 19, 2024
Copy link

Benchmark for 4eda799

Click to view benchmark
Test Base PR %
code_action 5.2±0.09ms 5.4±0.14ms +3.85%
code_lens 286.7±4.72ns 288.9±8.15ns +0.77%
compile 3.0±0.05s 3.1±0.04s +3.33%
completion 5.0±0.28ms 5.2±0.33ms +4.00%
did_change_with_caching 2.8±0.02s 3.0±0.04s +7.14%
document_symbol 964.7±17.96µs 968.1±36.73µs +0.35%
format 72.5±0.69ms 74.0±0.83ms +2.07%
goto_definition 363.4±11.15µs 371.0±3.24µs +2.09%
highlight 9.0±0.22ms 9.5±0.28ms +5.56%
hover 537.1±4.51µs 547.5±6.61µs +1.94%
idents_at_position 123.2±0.56µs 130.6±1.24µs +6.01%
inlay_hints 650.0±27.52µs 685.5±24.38µs +5.46%
on_enter 478.7±14.90ns 491.9±14.32ns +2.76%
parent_decl_at_position 3.7±0.25ms 3.8±0.09ms +2.70%
prepare_rename 360.1±9.98µs 367.7±6.05µs +2.11%
rename 9.2±0.47ms 9.7±0.29ms +5.43%
semantic_tokens 1035.1±13.21µs 1105.5±79.81µs +6.80%
token_at_position 355.8±2.19µs 369.0±2.66µs +3.71%
tokens_at_position 3.6±0.04ms 3.9±0.12ms +8.33%
tokens_for_file 409.4±2.82µs 414.8±5.10µs +1.32%
traverse 37.9±0.71ms 38.4±0.90ms +1.32%

@esdrubal esdrubal requested a review from a team February 19, 2024 18:50
@esdrubal esdrubal marked this pull request as ready for review February 19, 2024 18:50
@IGI-111 IGI-111 requested a review from a team February 19, 2024 20:23
@esdrubal esdrubal enabled auto-merge (squash) February 20, 2024 07:23
Copy link

Benchmark for f17d874

Click to view benchmark
Test Base PR %
code_action 5.2±0.06ms 5.2±0.13ms 0.00%
code_lens 288.4±4.63ns 287.7±7.88ns -0.24%
compile 3.1±0.05s 3.1±0.03s 0.00%
completion 4.8±0.21ms 5.1±0.30ms +6.25%
did_change_with_caching 2.9±0.05s 2.9±0.03s 0.00%
document_symbol 948.3±10.17µs 963.4±27.51µs +1.59%
format 76.4±1.41ms 75.3±2.38ms -1.44%
goto_definition 358.6±4.54µs 365.5±7.03µs +1.92%
highlight 8.9±0.27ms 8.9±0.12ms 0.00%
hover 542.1±9.44µs 550.3±51.08µs +1.51%
idents_at_position 122.4±0.39µs 124.7±1.21µs +1.88%
inlay_hints 649.7±17.66µs 652.5±43.75µs +0.43%
on_enter 491.7±12.43ns 488.8±14.86ns -0.59%
parent_decl_at_position 3.6±0.04ms 3.6±0.03ms 0.00%
prepare_rename 356.9±6.69µs 352.2±8.12µs -1.32%
rename 9.7±0.13ms 9.3±0.25ms -4.12%
semantic_tokens 1061.4±28.76µs 1046.9±24.27µs -1.37%
token_at_position 353.7±7.01µs 357.1±2.64µs +0.96%
tokens_at_position 3.6±0.05ms 3.6±0.06ms 0.00%
tokens_for_file 417.1±5.24µs 407.6±3.22µs -2.28%
traverse 38.5±0.98ms 39.2±1.79ms +1.82%

@esdrubal esdrubal merged commit 6b4412c into master Feb 20, 2024
31 of 32 checks passed
@esdrubal esdrubal deleted the esdrubal/5614_method_application_parameters_annotation_fix branch February 20, 2024 07:48
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Into does not work as expected
3 participants