feat(c-api) Update `wasm-c-api` repository #1699

Hywan · 2020-10-09T12:38:44Z

This change is important because wasm.h contains new functions, like
wasm_name_new_from_string_nt, which are useful for the Go
implementation.

This change is important because `wasm.h` contains new functions, like `wasm_name_new_from_string_nt`, which are useful for the Go implementation.

MarkMcCaskey

Looks good to me...

we may need to update some of our own tests as well if any behavior has changed

MarkMcCaskey · 2020-10-09T18:26:29Z

lib/c-api/tests/wasm_c_api/wasm-c-api/README.md

@@ -94,6 +94,7 @@ Currently, known implementations of this API are included in
 * V8 natively (both C and C++)
 * Wabt (only C?)
 * Wasmtime (only C?)
+* [Wasmer](https://github.com/wasmerio/wasmer/tree/master/lib/c-api) (only C, C++ coming soon)


Unrelated to this PR but I want to put this idea out into the universe before I forget:

When doing the release we have c-api/docs/deprecated and we currently package that as README.md in the C API that we distribute...

We should probably make a note that it's the deprecated API and file an issue to start building docs for Wasmer with the Wasm C API!

Completely!

The `CArrayIter` type is no longer needed. This patch removes it.

… so that it matches the definition of `wasm_instance_new`, which is much simpler and consistent.

Hywan

In addition to the updates required by the changes in wasm.h, I've updated wasi/mod.rs (the wasm_get_imports function) to use a wasm_extern_vec_t, so that it matches the definition of wasm_instance_new, which is simpler and more consistent.

Hywan · 2020-10-12T09:26:05Z

lib/c-api/tests/wasm_c_api/wasm-c-api/include/wasm.h

 typedef own wasm_trap_t* (*wasm_func_callback_t)(
-  const wasm_val_t args[], wasm_val_t results[]);
+  const wasm_val_vec_t* args, own wasm_val_vec_t* results);
 typedef own wasm_trap_t* (*wasm_func_callback_with_env_t)(
-  void* env, const wasm_val_t args[], wasm_val_t results[]);
+  void* env, const wasm_val_vec_t* args, wasm_val_vec_t* results);


Rust implementation updated ✅

Hywan · 2020-10-12T09:26:13Z

lib/c-api/tests/wasm_c_api/wasm-c-api/include/wasm.h

 WASM_API_EXTERN own wasm_trap_t* wasm_func_call(
-  const wasm_func_t*, const wasm_val_t args[], wasm_val_t results[]);
+  const wasm_func_t*, const wasm_val_vec_t* args, wasm_val_vec_t* results);


Rust implementation updated ✅

Hywan · 2020-10-12T09:46:20Z

lib/c-api/tests/wasm_c_api/wasm-c-api/include/wasm.h

 WASM_API_EXTERN own wasm_instance_t* wasm_instance_new(
-  wasm_store_t*, const wasm_module_t*, const wasm_extern_t* const imports[],
+  wasm_store_t*, const wasm_module_t*, const wasm_extern_vec_t* imports,
  own wasm_trap_t**
 );


Rust implementation updated ✅

Hywan · 2020-10-12T10:17:44Z

@MarkMcCaskey We must be very careful with the last update of wasm.h. The following code is now a common pattern:

wasm_val_vec_t args = WASM_EMPTY_VEC;

where WASM_EMPTY_VEC is a new macro such as:

#define WASM_EMPTY_VEC {0, NULL}

So basically, it creates a wasm_val_vec_t { .size = 0, .data = NULL }.

Why is it dangerous for us? Because our into_slice implementation for wasm_$name_vec_t returns None if data is null. Yup. So an empty array passed to into_slice will return None.

I don't know if we should change the behavior of into_slice. Maybe we should. At least to return an empty static slice ([])? Thoughts?

Hywan · 2020-10-12T10:19:16Z

All wasm-c-api examples/tests are green. Let's migrate our own tests.

`wasm_$name_vec_t.into_slice` returns `None` if the vec is empty. So an empty vec of `wasm_extern_t` given to `wasm_func_call` was raising an error. This patch fixes this.

We must pass an empty array.

Hywan · 2020-10-12T11:36:05Z

All good. All wasmer_c_api tests are green. Let's see how it impacts all the project.

Hywan · 2020-10-12T11:36:09Z

bors try

bors · 2020-10-12T12:07:51Z

try

Build failed:

Test on windows

MarkMcCaskey · 2020-10-12T17:30:12Z

lib/c-api/src/wasm_c_api/externals/function.rs


-            let _traps = callback(*env, processed_args.as_ptr(), results.as_mut_ptr());
+            let _traps = callback(*env, &processed_args, &mut results);


This might not be the same, we should be careful of the difference between fat pointers and thin pointers.

as_ptr and as_mut_ptr generally pass word sized pointers, references to slices are double words and references to vecs are different data entirely

processed_args and results are now of kind wasm_val_vec_t. I believe it is safe to use & in this case.

lib/c-api/src/wasm_c_api/externals/function.rs

lib/c-api/src/wasm_c_api/instance.rs

Hywan · 2020-10-16T09:01:38Z

@MarkMcCaskey Please, can you review the following changes:

Thanks!

Hywan · 2020-10-16T13:29:50Z

bors try

bors · 2020-10-16T14:02:41Z

try

Build failed:

Test on macos

Hywan · 2020-10-16T18:45:20Z

bors try

bors · 2020-10-16T19:16:39Z

try

Build failed:

Test on macos

Hywan · 2020-10-16T19:24:50Z

bors try

Windows will raise a warning because `clang++` is used to handle C code, so I previously add `-x c` to tell `clang++` to handle code as C code. Bbut if code is treated as C, it will fail because `static_assert` is not available for C on Windows. So let's live with the warning.

Hywan · 2020-10-16T19:34:04Z

bors try-

Hywan · 2020-10-16T20:08:05Z

bors try

bors · 2020-10-16T20:57:57Z

try

Build failed:

Test on linux-aarch64

MarkMcCaskey · 2020-10-17T00:18:49Z

bors try

bors · 2020-10-17T01:10:21Z

try

Build succeeded:

Hywan · 2020-10-19T06:01:12Z

bors r+

bors · 2020-10-19T06:47:11Z

Build succeeded:

feat(c-api) Update wasm-c-api repository.

1872970

This change is important because `wasm.h` contains new functions, like `wasm_name_new_from_string_nt`, which are useful for the Go implementation.

Hywan requested a review from MarkMcCaskey October 9, 2020 12:38

Hywan requested a review from syrusakbary as a code owner October 9, 2020 12:38

MarkMcCaskey approved these changes Oct 9, 2020

View reviewed changes

Hywan added 7 commits October 12, 2020 11:22

feat(c-api) Implement wasm_val_vec_t.

8b38dd5

feat(c-api) Update the definitions of wasm_func_callback*_t.

d8dcb41

feat(c-api) Update wasm_func_new.

5dffd97

faet(c-api) Update wasm_func_new_with_env.

7f09838

feat(c-api) Update wasm_func_call.

e9cd710

feat(c-api) Update wasm_instance_new.

fa7fe83

The `CArrayIter` type is no longer needed. This patch removes it.

feat(c-api) Update wasi_get_imports to use a wasm_extern_vec_t…

fea156d

… so that it matches the definition of `wasm_instance_new`, which is much simpler and consistent.

Hywan commented Oct 12, 2020

View reviewed changes

Hywan added 7 commits October 12, 2020 12:20

fix(c-api) Fix wasm_func_call when params are empty.

202ffe7

`wasm_$name_vec_t.into_slice` returns `None` if the vec is empty. So an empty vec of `wasm_extern_t` given to `wasm_func_call` was raising an error. This patch fixes this.

Merge branch 'master' into feat-c-api-update-wasm-h

35f8e17

fix(c-api) Remove rustc warnings.

4584a53

test(c-api) Fix the early-exit test.

5bf9055

test(c-api) Use wasm_extern_vec_t for wasm_get_imports.

4186d1c

test(c-api) Don't pass NULL for zero args/results to wasm_func_call!

0b4ef7a

We must pass an empty array.

feat(c-api) Update wasmer_wasm.h.

437426d

bors bot added a commit that referenced this pull request Oct 12, 2020

Try #1699:

8cd73fe

Hywan added 4 commits October 12, 2020 13:36

Merge branch 'master' into feat-c-api-update-wasm-h

122ba83

doc(changelog) Add wasmerio#1699.

2023016

Merge branch 'master' into feat-c-api-update-wasm-h

bf389f9

doc(changelog) Add wasmerio#1685.

4463294

MarkMcCaskey reviewed Oct 12, 2020

View reviewed changes

Hywan added 7 commits October 16, 2020 10:17

feat(c-api) Simplify code with unwrap_or_default.

e29ea25

Merge branch 'master' into feat-c-api-update-wasm-h

d7d40e5

chore(changelog) Fix merge.

ddf2459

feat(cli) Update the crate_exe_main.c file to the latest Wasm C API.

4f9932c

test(integration) Add -x c to force clang++ to treat code as C.

5601ecb

test(integration) Update to use the latest Wasm C API.

cb5afa1

chore(test) Format code.

2fc1aaa

bors bot added a commit that referenced this pull request Oct 16, 2020

Try #1699:

cb5bf52

fix(c-api) Fix last edit of the wasmer_create_exe_main.c file.

3b05c26

bors bot added a commit that referenced this pull request Oct 16, 2020

Try #1699:

2b4da3c

fix(c-api) Fix another typo. Damn.

1cdb5b5

bors bot added a commit that referenced this pull request Oct 16, 2020

Try #1699:

9766fdc

bors bot added a commit that referenced this pull request Oct 16, 2020

Try #1699:

ca90f4f

bors bot added a commit that referenced this pull request Oct 17, 2020

Try #1699:

c9f054f

Merge branch 'master' into feat-c-api-update-wasm-h

a73e457

bors bot merged commit aeea0d6 into wasmerio:master Oct 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(c-api) Update `wasm-c-api` repository #1699

feat(c-api) Update `wasm-c-api` repository #1699

Hywan commented Oct 9, 2020

MarkMcCaskey left a comment

MarkMcCaskey Oct 9, 2020

syrusakbary Oct 9, 2020

Hywan left a comment

Hywan Oct 12, 2020

Hywan Oct 12, 2020

Hywan Oct 12, 2020

Hywan commented Oct 12, 2020 •

edited

Loading

Hywan commented Oct 12, 2020

Hywan commented Oct 12, 2020

Hywan commented Oct 12, 2020

bors bot commented Oct 12, 2020

MarkMcCaskey Oct 12, 2020

Hywan Oct 16, 2020

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

MarkMcCaskey commented Oct 17, 2020

bors bot commented Oct 17, 2020

Hywan commented Oct 19, 2020

bors bot commented Oct 19, 2020


		let _traps = callback(*env, processed_args.as_ptr(), results.as_mut_ptr());
		let _traps = callback(*env, &processed_args, &mut results);

feat(c-api) Update wasm-c-api repository #1699

feat(c-api) Update wasm-c-api repository #1699

Conversation

Hywan commented Oct 9, 2020

MarkMcCaskey left a comment

Choose a reason for hiding this comment

MarkMcCaskey Oct 9, 2020

Choose a reason for hiding this comment

syrusakbary Oct 9, 2020

Choose a reason for hiding this comment

Hywan left a comment

Choose a reason for hiding this comment

Hywan Oct 12, 2020

Choose a reason for hiding this comment

Hywan Oct 12, 2020

Choose a reason for hiding this comment

Hywan Oct 12, 2020

Choose a reason for hiding this comment

Hywan commented Oct 12, 2020 • edited Loading

Hywan commented Oct 12, 2020

Hywan commented Oct 12, 2020

Hywan commented Oct 12, 2020

bors bot commented Oct 12, 2020

try

MarkMcCaskey Oct 12, 2020

Choose a reason for hiding this comment

Hywan Oct 16, 2020

Choose a reason for hiding this comment

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

try

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

try

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

Hywan commented Oct 16, 2020

bors bot commented Oct 16, 2020

try

MarkMcCaskey commented Oct 17, 2020

bors bot commented Oct 17, 2020

try

Hywan commented Oct 19, 2020

bors bot commented Oct 19, 2020

feat(c-api) Update `wasm-c-api` repository #1699

feat(c-api) Update `wasm-c-api` repository #1699

Hywan commented Oct 12, 2020 •

edited

Loading