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

Do not panic when there is auto impl type check #6094

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Jun 5, 2024

Description

This PR tries to fix #6093.

The cause seems to be a problem with our error reporting. What is happening is, the source code has other errors unrelated to auto impl of AbiEncode. For example, some private fields are being accessed from other modules.

This is currently panicking because when type-checking the auto impl, some of these errors "leak" into the auto impl type_check and make it fail completely; I was expecting the type check would always finish, even if with some errors.

This was being used as a security guard against implementation bugs. But now that the auto impl is more stable we can remove it, and when a type check completely fails, we do not panic anymore, just failing that particular auto impl.

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.

@xunilrj xunilrj requested a review from a team as a code owner June 5, 2024 16:39
@xunilrj xunilrj self-assigned this Jun 5, 2024
Copy link

github-actions bot commented Jun 5, 2024

Benchmark for 75bb832

Click to view benchmark
Test Base PR %
code_action 5.4±0.03ms 5.4±0.03ms 0.00%
code_lens 355.9±11.54ns 334.6±8.83ns -5.98%
compile 4.0±0.13s 4.0±0.07s 0.00%
completion 4.8±0.09ms 4.7±0.02ms -2.08%
did_change_with_caching 3.7±0.06s 3.7±0.07s 0.00%
document_symbol 959.4±19.74µs 973.7±35.22µs +1.49%
format 74.5±0.67ms 73.8±1.44ms -0.94%
goto_definition 368.8±8.91µs 369.6±7.71µs +0.22%
highlight 9.1±0.02ms 9.0±0.02ms -1.10%
hover 541.3±9.09µs 540.0±7.89µs -0.24%
idents_at_position 123.5±0.28µs 125.3±0.73µs +1.46%
inlay_hints 670.7±9.86µs 663.7±16.92µs -1.04%
on_enter 452.3±15.38ns 457.8±15.08ns +1.22%
parent_decl_at_position 3.7±0.03ms 3.7±0.05ms 0.00%
prepare_rename 363.6±10.47µs 364.4±6.04µs +0.22%
rename 9.7±0.22ms 9.7±0.14ms 0.00%
semantic_tokens 979.4±21.04µs 994.6±24.39µs +1.55%
token_at_position 357.3±3.03µs 360.1±2.13µs +0.78%
tokens_at_position 3.7±0.01ms 3.7±0.03ms 0.00%
tokens_for_file 426.4±1.88µs 440.6±1.74µs +3.33%
traverse 39.1±1.70ms 40.4±1.47ms +3.32%

@xunilrj xunilrj merged commit e1fcf98 into master Jun 5, 2024
38 checks passed
@xunilrj xunilrj deleted the xunil/do-not-panic-when-auto-impl-fails branch June 5, 2024 19:47
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.

Semantic Analysis Panic
3 participants