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

Implement i64 and i128 Types and Host Functions #45

Merged
merged 11 commits into from
Nov 19, 2024
Merged

Implement i64 and i128 Types and Host Functions #45

merged 11 commits into from
Nov 19, 2024

Conversation

bbyalcinkaya
Copy link
Member

This PR adds semantics for handling of the i64 and i128 types, and host functions to operate with these types:

  • obj_from_i64: Creates a ScVal from Wasm i64 and returns the HostVal representing this ScVal.
  • obj_from_i128_pieces: Constructs an I128 object from two i64 parts (high and low).
  • obj_to_i128_lo and obj_to_i128_hi: Extracts the high/low i64 portion of an I128 object.

@bbyalcinkaya bbyalcinkaya marked this pull request as ready for review November 12, 2024 08:42
Copy link

@ACassimiro ACassimiro left a comment

Choose a reason for hiding this comment

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

Looks good to me and I don't have any issues with the implementation. Something has been bothering me a bit, though. As we continue expanding the implementation of the host functions, it would be nice to have references to their original implementation on top of the rules. The reasoning here is that, at least for me, the hostCall parameters are not necessarily friendly with regard to user readability. Still, this is not a blocker for me.

@rv-jenkins rv-jenkins merged commit f689855 into master Nov 19, 2024
3 checks passed
@rv-jenkins rv-jenkins deleted the i128 branch November 19, 2024 12:29
@bbyalcinkaya bbyalcinkaya mentioned this pull request Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants