Skip to content

[embind] Fix signature for 64 bit types. #22930

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.

Sign up for GitHub

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

Merged
brendandahl merged 5 commits into from
Nov 15, 2024
Merged

[embind] Fix signature for 64 bit types. #22930

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3644159
[embind] Fix signature for 64 bit types.
brendandahl Nov 15, 2024
16c99cf
Add test file.
brendandahl Nov 15, 2024
7777aa8
Rename test.
brendandahl Nov 15, 2024
51e6bad
Rename test again.
brendandahl Nov 15, 2024
1154f8e
nit
brendandahl Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions system/include/emscripten/bind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,13 @@ struct SignatureCode<size_t> {
}
};

template<>
struct SignatureCode<long long> {
static constexpr char get() {
return 'j';
}
};

#ifdef __wasm64__
template<>
struct SignatureCode<long> {
Expand All @@ -629,6 +636,8 @@ template<> struct SignatureTranslator<double> { using type = double; };
#ifdef __wasm64__
template<> struct SignatureTranslator<long> { using type = long; };
#endif
template<> struct SignatureTranslator<long long> { using type = long long; };
template<> struct SignatureTranslator<unsigned long long> { using type = long long; };
template<> struct SignatureTranslator<size_t> { using type = size_t; };
template<typename PtrType>
struct SignatureTranslator<PtrType*> { using type = void*; };
Expand Down
8 changes: 8 additions & 0 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3296,6 +3296,14 @@ def test_embind_return_value_policy(self):

self.do_runf('embind/test_return_value_policy.cpp')

@parameterized({
'': [[]],
'asyncify': [['-sASYNCIFY=1']]
})
def test_embind_bigint(self, args):
self.do_runf('embind/test_bigint.cpp', '1000000000000n\n-1000000000000n',
Copy link
Collaborator

@sbc100 sbc100 Nov 15, 2024

Choose a reason for hiding this comment

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

Is this new file missing from this PR?

Can you call it test_embind_bigint to match the test name?

Should it be called test_embind_int64 perhaps? Or test_embind_long_long?

emcc_args=['-lembind', '-sWASM_BIGINT'] + args)
Copy link
Collaborator

@sbc100 sbc100 Nov 15, 2024

Choose a reason for hiding this comment

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

Wow I guess we just never had anyone try this before?

Why does this require -sWASM_BIGINT to expose the issue? It looks like that lack of a long long mapping is a problem regardless of this setting?

Copy link
Collaborator Author

@brendandahl brendandahl Nov 15, 2024

Choose a reason for hiding this comment

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

It seems only ASYNCIFY exposed this because it uses the signature to create a DYNCALL with the legacy mode. Without asyncify bigints were working fine.


@requires_jspi
@parameterized({
'': [['-sJSPI_EXPORTS=async*']],
Expand Down
Loading