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

Generate C preprocessor code to hide things not on Windows #952

Merged
merged 2 commits into from
Nov 11, 2019

Conversation

MarkMcCaskey
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey commented Nov 11, 2019

resolves #804

Review

  • Add a short description of the the change to the CHANGELOG.md file

@MarkMcCaskey MarkMcCaskey added the 📦 lib-c-api About wasmer-c-api label Nov 11, 2019
@MarkMcCaskey MarkMcCaskey force-pushed the feature/hide-trampoline-c-api-on-windows branch from 1f9de1b to e121c47 Compare November 11, 2019 22:47
lib/runtime-c-api/build.rs Show resolved Hide resolved
@@ -612,32 +635,46 @@ uint32_t wasmer_table_length(wasmer_table_t *table);
/// and `wasmer_last_error_message` to get an error message.
wasmer_result_t wasmer_table_new(wasmer_table_t **table, wasmer_limits_t limits);

#if (!defined(_WIN32) && defined(ARCH_X86_64))
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: these are clearly a group, just put a single #if around them to disable all these related trampoline functions for windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's unfortunate that it did that; if we manually edit the header file though then we have to do it after every time it's generated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put it at the top level in Rust

@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Nov 11, 2019
952: Generate C preprocessor code to hide things not on Windows r=MarkMcCaskey a=MarkMcCaskey

resolves #804 

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: nlewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Nov 11, 2019

Build succeeded

  • wasmerio.wasmer

@bors bors bot merged commit 08c66d6 into master Nov 11, 2019
@bors bors bot deleted the feature/hide-trampoline-c-api-on-windows branch November 11, 2019 23:53
Hywan referenced this pull request in wasmerio/wasmer-go Nov 14, 2019
This patch updates the runtime to 0.10.1. It includes this bug fix
wasmerio/wasmer#960.
bors bot added a commit that referenced this pull request Nov 14, 2019
960: feat(runtime-c-api) Add support for clang in `WASMER_H_MACROS` r=MarkMcCaskey a=Hywan

In #952, the `WASMER_H_MACROS` constant has been defined. The `ARCH_X86_64` constant is defined under 2 conditions: If the compiler is MSVC + `_M_AMD64` is defined, or if the compiler is GCC + `__x86_64__` is defined.

Clang is missing. And it breaks some projects (like https://github.com/wasmerio/php-ext-wasm or https://github.com/wasmerio/go-ext-wasm for instance).

This patch supports Clang.

Co-authored-by: Ivan Enderlin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 lib-c-api About wasmer-c-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C API doesn't mention platform restrictions regarding trampoline
2 participants