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

Trying to pass i64 without WASM_BIGINT should warn or assert #15871

Open
mmarczell-graphisoft opened this issue Jan 3, 2022 · 3 comments
Open

Comments

@mmarczell-graphisoft
Copy link
Contributor

This code:

#include <cstdint>
#include <emscripten.h>

EM_JS(void, foo, (int64_t i), {
  console.log(i);
})

int main() {
  foo(1);
}

should issue at least a runtime warning if it was not compiled with WASM_BIGINT.

Comments and pointers appreciated for a (hopefully) upcoming PR.

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.0.0 (3fd52e107187b8a169bb04a02b9f982c8a075205)
clang version 14.0.0 (https://github.com/llvm/llvm-project 4348cd42c385e71b63e5da7e492172cff6a79d7b)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /Volumes/SSD/git/emsdk/upstream/bin
@sbc100
Copy link
Collaborator

sbc100 commented Jan 7, 2022

Good idea. We should probably also add a test to ensure it does work as expected when WASM_BIGINT is enabled.

Looking into this I'm not sure how easy it will be to issue such a warning. BTW, we also don't warn for other unsupported types (e.g. struct types) so this issue is larger than just i64.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 7, 2022

We do have access to the string of the signature, e.g. "(int64_t i)", however we don't have way to know about type names used in the C/C++ code. I'm not sure we can enumberate all the valid types or figure out if a given type lowers to i64. For example, if a user has a typedef called myint64 we don't know what type that is.

We could decide that we only accept a fixed set of names for type in EM_JS macros (for example int, long, int32_t, int8_t, int16_t, etc).. but that might break existing code that uses typedefs.

Alternatively we could look specifically for know-bad types such as long long and int64_t.

I'm not sure either of these options really solves the problem fully though.

@mmarczell-graphisoft
Copy link
Contributor Author

@sbc100 I assumed, but then it seems, wrongly, that EM_ASM and EM_JS share much more underlying infrastructure, and the assertion from #15830 could be adapted.

sbc100 added a commit that referenced this issue Jan 7, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 3, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 4, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
sbc100 added a commit that referenced this issue May 4, 2022
Ideally we should error out if WASM_BIGINT is not passed by I have
yet to find a good place to do that.

See #15871
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

No branches or pull requests

2 participants