Skip to content

Add test for integer limits #20353

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
87 changes: 87 additions & 0 deletions test/embind/test_val_integers.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright 2023 The Emscripten Authors. All rights reserved.
// Emscripten is available under two separate licenses, the MIT license and the
// University of Illinois/NCSA Open Source License. Both these licenses can be
// found in the LICENSE file.

#include <emscripten.h>
#include <emscripten/val.h>
#include <iostream>
#include <limits>

using namespace emscripten;

bool check_num_on_js_side(std::string value_s) {
std::string code = std::string("num === ") + value_s + " || console.error(`\t\tFailed: expected " + value_s + ", got ${num}`)";
return emscripten_run_script_int(code.c_str()) == 1;
}

template <typename T>
bool check_num_on_cpp_side(T expected) {
T value = emscripten::val::global("num").as<T>();
if (value != expected) {
std::cerr << "\t\tFailed: expected " << expected << ", got " << value << std::endl;
return false;
}
return true;
}

template <typename T>
bool test_num(const char *type_name, T value) {
std::string value_s = std::to_string(value);
// 64-bit integer must be represented as a BigInt which uses `n` suffix
if (sizeof(T) > 4) {
value_s.push_back('n');
}
bool ok = true;
std::cout << "Testing (" << type_name << ") " << value_s << std::endl;
// Test C++ to JS conversion
std::cout << "\tC++ to JS via assignment" << std::endl;
emscripten::val::global().set("num", value);
ok &= check_num_on_js_side(value_s);
// Test C++ to JS conversion via function call.
// Calls use their own argument serialization, so it's best to ensure that works too.
std::cout << "\tC++ to JS via call" << std::endl;
emscripten::val::global("setNum")(value);
ok &= check_num_on_js_side(value_s);
// Test JS to C++ conversion
std::cout << "\tJS to C++" << std::endl;
ok &= check_num_on_cpp_side(value);
return ok;
}

template <typename T>
bool test_num_limits(const char *type_name) {
// note: using `&` instead of `&&` to ensure both tests are run
return test_num<T>(type_name, std::numeric_limits<T>::min()) &
test_num<T>(type_name, std::numeric_limits<T>::max());
}

#define TEST_NUM_LIMITS(T) ok &= test_num_limits<T>(#T)

int main() {
bool ok = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated to the actual failure we (I) tend to prefer just using assert(..) everywhere rather than accumulating the failures in some kind of success or ok variable.

It means test will exit on first failure but I've found that that is almost always what I want anyway (since it avoids having to page back through reams of output to find the first failure when debugging).

This also makes the tests simpler since you can basically remove ok from here and everywhere else in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It means test will exit on first failure but I've found that that is almost always what I want anyway

That's what I almost always want too, but I believe in this case an exception was warranted: there are various failure edge cases and the goal was to uncover all of them in a flat list. Otherwise we wouldn't know what other bugs are there until we fix the first one and the next one is uncovered and so on.

When all those bugs are fixed, we can revert this to simpler version if that's desirable, but for now it was more of a "show all types that are broken" exploration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(worth noting that it still doesn't fully realise that goal because at some point test crashes with JS exception after regular value mismatches, so there are probably more things broken after that line)


EM_ASM({
globalThis.setNum = num => globalThis.num = num;
});

TEST_NUM_LIMITS(char);
TEST_NUM_LIMITS(unsigned char);
TEST_NUM_LIMITS(signed char);

TEST_NUM_LIMITS(short);
TEST_NUM_LIMITS(unsigned short);

TEST_NUM_LIMITS(int);
TEST_NUM_LIMITS(unsigned int);

TEST_NUM_LIMITS(long);
TEST_NUM_LIMITS(unsigned long);

#ifdef WASM_BIGINT
TEST_NUM_LIMITS(long long);
TEST_NUM_LIMITS(unsigned long long);
#endif

return !ok;
}
7 changes: 7 additions & 0 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7768,6 +7768,13 @@ def test_embind_unsigned(self):
self.emcc_args += ['-lembind']
self.do_run_in_out_file_test('embind/test_unsigned.cpp')

@also_with_wasm_bigint
def test_embind_val_integers(self):
self.emcc_args += ['-lembind']
if self.get_setting('WASM_BIGINT'):
self.emcc_args += ['-DWASM_BIGINT']
self.do_runf(test_file('embind/test_val_integers.cpp'))

def test_embind_val(self):
self.emcc_args += ['-lembind']
self.do_run_in_out_file_test('embind/test_val.cpp')
Expand Down