Skip to content

Conversation

awelc
Copy link
Contributor

@awelc awelc commented Sep 12, 2024

Description

This PR contains a refactoring to split a super-long get_symbols function into two smaller ones.

It not only makes code mode readable but also enables a more optimized implementation of the test suite. Previously, all symbols had to be recomputed when tests involved a cursor, even though it was only cursor that was changing, rather than symbols themselves. With this PR, we can recompute the cursor only, which brings the time to run all move-analyzer tests from minutes to (tens of) seconds.

Test plan

All existing tests must pass

@awelc awelc requested a review from cgswords September 12, 2024 16:00
@awelc awelc self-assigned this Sep 12, 2024
Copy link

vercel bot commented Sep 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2024 9:37pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 9:37pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 9:37pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 9:37pm

Copy link
Contributor

@cgswords cgswords left a comment

Choose a reason for hiding this comment

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

LGTM, nice little bit of cleanup and refactoring.

Copy link
Contributor

@cgswords cgswords left a comment

Choose a reason for hiding this comment

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

LGTM, nice little bit of cleanup and refactoring.

&compiled_pkg_info,
cursor_info,
);
cursor_context = compute_symbols_parsed_program(
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a comment that this skips typed computation for speed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Done!

@awelc awelc merged commit 2c47636 into main Sep 13, 2024
48 checks passed
@awelc awelc deleted the aw/ide-symbolication-split branch September 13, 2024 22:49
suiwombat pushed a commit that referenced this pull request Sep 16, 2024
…19338)

## Description 

This PR contains a refactoring to split a super-long `get_symbols`
function into two smaller ones.

It not only makes code mode readable but also enables a more optimized
implementation of the test suite. Previously, all symbols had to be
recomputed when tests involved a cursor, even though it was only cursor
that was changing, rather than symbols themselves. With this PR, we can
recompute the cursor only, which brings the time to run all
`move-analyzer` tests from minutes to (tens of) seconds.

## Test plan 

All existing tests must pass
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.

2 participants