-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Merge std and core libraries #6729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Truly a Herculian effort on this one!
I've added a few comments. There shouldn't be much work in them, so there should be time to do them before the release.
The ones about using the reduced std libs can be left for a separate issue if needed.
sway-core/src/semantic_analysis/ast_node/declaration/auto_impl/abi_encoding.rs
Show resolved
Hide resolved
sway-core/src/semantic_analysis/ast_node/declaration/auto_impl/abi_encoding.rs
Show resolved
Hide resolved
test/src/e2e_vm_tests/reduced_std_libs/sway-lib-std-assert/src/prelude.sw
Show resolved
Hide resolved
test/src/e2e_vm_tests/reduced_std_libs/sway-lib-std-conversions/src/lib.sw
Show resolved
Hide resolved
...rc/e2e_vm_tests/test_programs/should_pass/test_contracts/array_of_structs_contract/Forc.toml
Show resolved
Hide resolved
test/src/e2e_vm_tests/test_programs/should_pass/test_contracts/balance_test_contract/Forc.toml
Show resolved
Hide resolved
.../e2e_vm_tests/test_programs/should_pass/test_contracts/nested_struct_args_contract/Forc.toml
Show resolved
Hide resolved
test/src/sdk-harness/test_projects/string_slice/string_slice_predicate/Forc.toml
Show resolved
Hide resolved
…abs/sway into swaystar123/merge-core-std
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing my review status to approved.
The one change that was definitely needed was comments in abi_encoding.rs, which have been added.
My remaining review comments are nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have liked to see more edge cases covered(zero and max values) in the inline tests but these can be added afterward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tooling side of the things looks good to me. We used to have that if core is not added add it by default and std was opt-out. Now that we remove core we seem to remove the auto core addition thingy as well. Just writing to confirm that this is the desired behaviour.
Description
Merges the two libraries. They were initially separate to separate the core logic and fuel vm specific functionality, but that separation is no longer maintained so having a merged library is better.
Closes #6708
Checklist
Breaking*
orNew Feature
labels where relevant.