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 forc-pkg with generic manifest trait #5625

Merged
merged 82 commits into from
Feb 21, 2024

Conversation

sdankel
Copy link
Member

@sdankel sdankel commented Feb 16, 2024

Follow up to #5477

#5477 (comment)

sdankel and others added 30 commits December 13, 2023 14:03
This PR implements `-o` flags so that we can disable optimizations. This
will be useful as we are working towards debugger support and
optimizations pollute source map generated.

Provide option to the e2e test binary to specify build profile. For
tests that have their ABI tested, or rely on their compiled
hashes (for deployment), since that changes with build profile, the
tests are marked as unsupported for debug profile.
(better to test with and have expected results for release than the
debug profile).

Add testing release profile in CI since the default is now debug. Two
tests in `should_fail` that are expected to fail due to
overflows are disabled because there's a bug in our IR gen. This is
tracked by #5449

---------

Co-authored-by: Vaivaswatha Nagaraj <[email protected]>
Co-authored-by: Vaivaswatha N <[email protected]>
Co-authored-by: Sophie Dankel <[email protected]>
## Description

Don't allow configurables to be used in const expressions except as
storage and configurable initializers.

Fix #5272
Fix #5261


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: João Matos <[email protected]>
## Description
There isn't a good reason to target the local std-lib for these tests,
targeting the git tag helps reduce churn on people working on the
std-lib.
Copy link

Benchmark for 1ca0ea6

Click to view benchmark
Test Base PR %
code_action 5.2±0.05ms 5.2±0.03ms 0.00%
code_lens 313.7±10.71ns 291.3±8.41ns -7.14%
compile 2.9±0.04s 3.0±0.05s +3.45%
completion 4.8±0.11ms 4.7±0.08ms -2.08%
did_change_with_caching 2.9±0.04s 3.0±0.09s +3.45%
document_symbol 949.8±20.06µs 968.8±16.95µs +2.00%
format 89.5±1.04ms 89.2±0.99ms -0.34%
goto_definition 361.0±6.43µs 359.6±8.32µs -0.39%
highlight 8.8±0.10ms 8.8±0.14ms 0.00%
hover 544.0±8.40µs 551.5±8.90µs +1.38%
idents_at_position 123.0±0.59µs 122.1±1.04µs -0.73%
inlay_hints 662.2±21.98µs 667.9±27.18µs +0.86%
on_enter 510.5±17.31ns 486.3±11.10ns -4.74%
parent_decl_at_position 3.6±0.03ms 3.6±0.03ms 0.00%
prepare_rename 358.3±3.30µs 365.1±8.88µs +1.90%
rename 9.3±0.09ms 9.2±0.05ms -1.08%
semantic_tokens 1053.5±18.51µs 1064.4±11.52µs +1.03%
token_at_position 355.9±2.49µs 358.1±4.08µs +0.62%
tokens_at_position 3.6±0.11ms 3.6±0.03ms 0.00%
tokens_for_file 425.0±8.35µs 413.5±2.78µs -2.71%
traverse 37.6±0.85ms 37.7±0.86ms +0.27%

tritao
tritao previously approved these changes Feb 19, 2024
Base automatically changed from sophie/sourcemaps to master February 20, 2024 03:24
@sdankel sdankel dismissed tritao’s stale review February 20, 2024 03:24

The base branch was changed.

@sdankel sdankel requested a review from tritao February 20, 2024 03:48
@sdankel sdankel enabled auto-merge (squash) February 20, 2024 03:48
Copy link

Benchmark for 2a52e30

Click to view benchmark
Test Base PR %
code_action 5.2±0.11ms 5.1±0.11ms -1.92%
code_lens 285.7±8.53ns 334.3±27.64ns +17.01%
compile 2.9±0.03s 3.0±0.03s +3.45%
completion 4.7±0.01ms 4.7±0.03ms 0.00%
did_change_with_caching 2.8±0.02s 2.8±0.03s 0.00%
document_symbol 1021.8±7.23µs 955.4±19.20µs -6.50%
format 74.1±0.63ms 74.9±1.13ms +1.08%
goto_definition 357.6±6.33µs 363.2±4.03µs +1.57%
highlight 8.8±0.18ms 8.8±0.01ms 0.00%
hover 532.9±7.38µs 537.2±6.92µs +0.81%
idents_at_position 121.5±1.09µs 122.7±0.68µs +0.99%
inlay_hints 657.7±39.19µs 726.9±18.73µs +10.52%
on_enter 488.7±13.59ns 493.8±13.18ns +1.04%
parent_decl_at_position 3.6±0.05ms 3.6±0.03ms 0.00%
prepare_rename 363.1±17.94µs 358.2±5.75µs -1.35%
rename 9.1±0.18ms 9.1±0.03ms 0.00%
semantic_tokens 1036.9±21.55µs 1055.9±19.37µs +1.83%
token_at_position 355.7±3.95µs 351.0±2.10µs -1.32%
tokens_at_position 3.6±0.03ms 3.6±0.03ms 0.00%
tokens_for_file 414.6±3.87µs 412.9±1.61µs -0.41%
traverse 37.1±0.54ms 38.1±1.16ms +2.70%

@sdankel sdankel changed the title Refactor with generic manifest trait Refactor forc-pkg with generic manifest trait Feb 20, 2024
Copy link

Benchmark for 6c3ecae

Click to view benchmark
Test Base PR %
code_action 5.1±0.15ms 5.1±0.12ms 0.00%
code_lens 288.2±8.89ns 331.9±8.02ns +15.16%
compile 3.0±0.03s 3.0±0.04s 0.00%
completion 4.8±0.11ms 4.8±0.11ms 0.00%
did_change_with_caching 2.9±0.04s 2.8±0.04s -3.45%
document_symbol 981.5±19.98µs 967.7±18.02µs -1.41%
format 75.7±1.30ms 74.2±1.54ms -1.98%
goto_definition 360.0±2.90µs 379.2±5.25µs +5.33%
highlight 9.0±0.25ms 8.8±0.15ms -2.22%
hover 539.0±9.88µs 554.8±7.67µs +2.93%
idents_at_position 121.8±0.33µs 121.5±1.17µs -0.25%
inlay_hints 646.8±17.11µs 689.8±28.92µs +6.65%
on_enter 500.4±16.50ns 539.6±15.93ns +7.83%
parent_decl_at_position 3.6±0.03ms 3.6±0.04ms 0.00%
prepare_rename 364.3±6.24µs 380.2±8.80µs +4.36%
rename 9.3±0.28ms 9.3±0.21ms 0.00%
semantic_tokens 1022.6±20.52µs 1057.0±16.85µs +3.36%
token_at_position 358.5±3.30µs 373.9±3.09µs +4.30%
tokens_at_position 3.6±0.03ms 3.6±0.02ms 0.00%
tokens_for_file 416.6±25.53µs 415.8±2.76µs -0.19%
traverse 39.2±0.77ms 37.7±0.61ms -3.83%

Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

nice one

@kayagokalp kayagokalp added the forc-pkg Everything related to the `forc-pkg` crate. label Feb 21, 2024
@sdankel sdankel merged commit 68191dd into master Feb 21, 2024
35 checks passed
@sdankel sdankel deleted the sophie/generic-manifest-trait branch February 21, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forc-pkg Everything related to the `forc-pkg` crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants