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

Missing define/undef of 'own' symbol for wasmtime.h #1215

Closed
rene-fonseca opened this issue Mar 3, 2020 · 11 comments
Closed

Missing define/undef of 'own' symbol for wasmtime.h #1215

rene-fonseca opened this issue Mar 3, 2020 · 11 comments
Labels
wasmtime:c-api Issues pertaining to the C API.

Comments

@rene-fonseca
Copy link

#define own
and
#undef own

are missing for new wasmtime.h.

Both wasi.h and wasm.h define own properly.

@rene-fonseca
Copy link
Author

Also, there is an extra , - that breaks build.

https://github.com/bytecodealliance/wasmtime/blob/master/crates/c-api/include/wasmtime.h#L58

bool wasmtime_wat2wasm(
wasm_engine_t *engine,
const wasm_byte_vec_t *wat,
own wasm_byte_vec_t *ret,
own wasm_byte_vec_t *error_message, // <<< REMOVE ,
);

@rene-fonseca
Copy link
Author

@alexcrichton FYI

@rene-fonseca
Copy link
Author

And then I like:
#include <wasm.h>
changed to:
#include "wasm.h"

This will allow headers to work regardless of the include search path. wasi.h already uses #include "wasm.h" as desired.

See note in #1211

@rene-fonseca
Copy link
Author

FYI This was tested with clang.

@yurydelendik yurydelendik added the wasmtime:c-api Issues pertaining to the C API. label Mar 3, 2020
@rene-fonseca
Copy link
Author

rene-fonseca commented Mar 3, 2020

Maybe error_message should use wasm_message_t instead of wasm_byte_vec_t for consistency.

@rene-fonseca
Copy link
Author

@alexcrichton
Copy link
Member

I believe this was fixed in #1286, and we're not verifying the header file is usable on CI!

@rene-fonseca
Copy link
Author

Yes, fixed now.

@rene-fonseca
Copy link
Author

Oh I still like the #include <wasm.h> changed though - see above.

@alexcrichton
Copy link
Member

Can you open a separate issue? It's hard to dig through the comments of an unrelated issue to see what needs fixing.

@rene-fonseca
Copy link
Author

Opened #1313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

No branches or pull requests

3 participants