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(runtime) Cast and check CLI arguments to pass to the instance #281

Closed
wants to merge 2 commits into from

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Mar 19, 2019

When the Wasm binary isn't an emscripten module, this patch does 5 things:

  1. Check that the main function exists,
  2. Check the signature (arity and types),
  3. Cast the arguments into the correct Wasm value,
  4. Pass the arguments to the instance,
  5. Display the results.

I'm not sure about the usage of eprintln!. It's probably something
to update later, like the ability to pass a description message to
ExportWrongType.

Thoughts?

Before:

$ wasmer run nbody.wasm -- 7                                                                                                         
"Resolve(Signature { expected: FuncSig { params: [I32], returns: [F64] }, found: [] })"

After:

$ wasmer run nbody.wasm -- 7                                                                                                         
result_0 = f64 : -0.16907367446575297

Test file: rust_nbody.wasm.zip

When the Wasm binary isn't an emscripten module, this patch does 5 things:

  1. Check that the `main` function exists,
  2. Check the signature (arity and types),
  3. Cast the arguments into the correct Wasm value,
  4. Pass the arguments to the instance,
  5. Display the results.

I'm not sure about the usage of `eprintln!`. It's probably something
to update later, like the ability to pass a description message to
`ExportWrongType`.

Thoughts?
if args.len() != parameter_types.len() {
Err(CallError::Resolve(ResolveError::Signature {
expected: (*signature).clone(),
found: args.iter().map(|_| Type::I32).collect(),
Copy link
Contributor Author

@Hywan Hywan Mar 19, 2019

Choose a reason for hiding this comment

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

At this step, we don't know the type of the arguments yet. For the error, let's assume it's i32.

@lachlansneff
Copy link
Contributor

Can you specify what type the arguments you pass in are?

@syrusakbary
Copy link
Member

Can you specify what type the arguments you pass in are?

I think we can just try to cast it to what's expected and fail in case it's not possible. That way the way we do wasmer run ... becomes much simpler.
Thoughts?

@Hywan
Copy link
Contributor Author

Hywan commented Mar 20, 2019

All arguments are &str because they come from the CLI. The signature of the main function is fetched, and for each parameter, we try to parse the corresponding argument to the type of the parameter. So if main: i32 x i32 x f64, then for wasmer run foo.wasm 1 2, we will try to parse 1 and 2 as i32 (and the returned value is an f64). If the parsing fails (i.e. if the “casting” fails), then an error is raised.

I think it's the best way to handle casting from &str.

@Hywan
Copy link
Contributor Author

Hywan commented Mar 22, 2019

As discussed in private, I will change this PR to be wasmer call file.wasm function_name arg1 arg2.

bors bot added a commit that referenced this pull request Dec 11, 2019
1056: Integrate Ivan's arg parsing code into --invoke r=syrusakbary a=MarkMcCaskey

Co-authored-by: Ivan Enderlin <[email protected]>

Because #281 is on a branch on a fork, I made a new branch.

Does arg parsing when using `--invoke`


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
@syrusakbary
Copy link
Member

This changes have been merged in #1056. Closing the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants