Skip to content
This repository has been archived by the owner on Aug 14, 2023. It is now read-only.

Unroll ergonomics #258

Merged
merged 8 commits into from
Jul 7, 2021
Merged

Unroll ergonomics #258

merged 8 commits into from
Jul 7, 2021

Conversation

neysofu
Copy link
Contributor

@neysofu neysofu commented Jul 7, 2021

This is a PR draft to discuss loop unrolling ergonomics in the context of the SVM ABI encoder. Future changes and/or PRs might address the SVM decoder in a similar fashion if we're satisfied with these changes. The underlying tone of the refactor is:

  • Replace the overwhelming number of macro-based impl's with type safety using Rust traits. We might have to tweak trait names and implementation algebra a bit more, but I think this looks better and is much simpler to deal with.
  • Unify trait implementations for numeric primitives using the num-traits crate.
  • Unroll loops using the seq-macro crate.
  • Prune lots of code branches from the encoder logic by finding recurring patterns.

Possible TODOs and further changes to discuss:

  • Naming of layout utility generators: which crate they go in and how to integrate them within the existing codebase.
  • More comments and internal documentation.
  • Evaluate similar decisions for the SVM ABI decoder

use svm_abi_layout::layout;

#[test]
fn integer_layouts() {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can - please add a few more asserts to the other integer markers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand, what other conditions should I test?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are more markers that can be asserted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are derived automatically form the 1B ones using arithmetic. I think all are covered

crates/abi/encoder/src/types/numeric.rs Show resolved Hide resolved
@YaronWittenstein YaronWittenstein added enhancement New feature or request svm v0.3 labels Jul 7, 2021
@YaronWittenstein YaronWittenstein added this to the July 5 Sprint milestone Jul 7, 2021
@neysofu neysofu merged commit 0292b14 into master Jul 7, 2021
@neysofu neysofu deleted the unroll-ergonomics branch July 7, 2021 13:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request svm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an ergonomic way to avoid ugly code repetition to avoid loops in the SDK
2 participants