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

feat: Add hexadecimal notation support for integer arguments #9989

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

VolodymyrBg
Copy link

Add support for parsing hexadecimal integers (prefixed with 0x or 0X) when passing arguments to WebAssembly functions. This enhancement allows users to specify integer arguments in both decimal and hexadecimal formats.

For example:

  • Decimal: 42
  • Hexadecimal: 0x2A or 0x2a

The change affects both i32 and i64 argument types while maintaining backward compatibility with decimal notation.

Add support for parsing hexadecimal integers (prefixed with 0x or 0X) when passing
arguments to WebAssembly functions. This enhancement allows users to specify integer
arguments in both decimal and hexadecimal formats.

For example:
- Decimal: 42
- Hexadecimal: 0x2A or 0x2a

The change affects both i32 and i64 argument types while maintaining backward
compatibility with decimal notation.
@VolodymyrBg VolodymyrBg requested a review from a team as a code owner January 12, 2025 19:35
@VolodymyrBg VolodymyrBg requested review from fitzgen and removed request for a team January 12, 2025 19:35
@alexcrichton
Copy link
Member

Thanks! Mind adding a test for this as well? (probably somewhere in tests/all/cli_tests.rs)

Defines a module with a single function sum
Takes two i32 parameters and returns their sum
Uses basic WebAssembly instructions to add the numbers
Builds a WebAssembly module from our WAT file
Invokes the sum function with two hexadecimal numbers (0x2A and 0xFF)
Verifies that the output is correct (297, which is 42 + 255)
@VolodymyrBg
Copy link
Author

Thanks! Mind adding a test for this as well? (probably somewhere in tests/all/cli_tests.rs)

Thank you for the answer. I did what you asked

@alexcrichton
Copy link
Member

(I think there's a test failure though?)

@VolodymyrBg
Copy link
Author

(I think there's a test failure though?)

Yes)) Gonna correct it soon

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Thanks!

@fitzgen
Copy link
Member

fitzgen commented Jan 13, 2025

Looks like there are still some failing CI jobs that need to be fixed before this can merge.

@VolodymyrBg
Copy link
Author

Looks like there are still some failing CI jobs that need to be fixed before this can merge.

Working on it))

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.

3 participants