fix(napi): set lossless parameter in napi_get_value_bigint_{int64,uint64}, and trim leading zeroes in napi_create_bigint_words (#15804)

src/bun.js/bindings/napi.cpp

@@ -2519,6 +2519,78 @@ extern "C" napi_status napi_typeof(napi_env env, napi_value val,
     return napi_generic_failure;
 }
 
+static_assert(std::is_same_v<JSBigInt::Digit, uint64_t>, "All NAPI bigint functions assume that bigint words are 64 bits");
+#if USE(BIGINT32)
+#error All NAPI bigint functions assume that BIGINT32 is disabled
+#endif
+
+extern "C" napi_status napi_get_value_bigint_int64(napi_env env, napi_value value, int64_t* result, bool* lossless)
+{
+    NAPI_PREMABLE
+    JSValue jsValue = toJS(value);
+    if (!env || jsValue.isEmpty() || !result || !lossless) {
+        return napi_invalid_arg;
+    }
+    if (!jsValue.isHeapBigInt()) {
+        return napi_bigint_expected;
+    }
+
+    auto* globalObject = toJS(env);
+    auto& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    *result = jsValue.toBigInt64(toJS(env));
+    // toBigInt64 can throw if the value is not a bigint. we have already checked, so we shouldn't
+    // hit an exception here, but we should check just in case
+    scope.assertNoException();
+
+    JSBigInt* bigint = jsValue.asHeapBigInt();
+    uint64_t digit = bigint->length() > 0 ? bigint->digit(0) : 0;
+
+    if (bigint->length() > 1) {
+        *lossless = false;
+    } else if (bigint->sign()) {
+        // negative
+        // lossless if numeric value is >= -2^63,
+        // for which digit will be <= 2^63
+        *lossless = (digit <= (1ull << 63));
+    } else {
+        // positive
+        // lossless if numeric value is <= 2^63 - 1
+        *lossless = (digit <= static_cast<uint64_t>(INT64_MAX));
+    }
+
+    return napi_ok;
+}
+
+extern "C" napi_status napi_get_value_bigint_uint64(napi_env env, napi_value value, uint64_t* result, bool* lossless)
+{
+    NAPI_PREMABLE
+    JSValue jsValue = toJS(value);
+    if (!env || jsValue.isEmpty() || !result || !lossless) {
+        return napi_invalid_arg;
+    }
+    if (!jsValue.isHeapBigInt()) {
+        return napi_bigint_expected;
+    }
+
+    auto* globalObject = toJS(env);
+    auto& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    *result = jsValue.toBigUInt64(toJS(env));
+    // toBigUInt64 can throw if the value is not a bigint. we have already checked, so we shouldn't
+    // hit an exception here, but we should check just in case
+    scope.assertNoException();
+
+    // bigint to uint64 conversion is lossless if and only if there aren't multiple digits and the
+    // value is positive
+    JSBigInt* bigint = jsValue.asHeapBigInt();
+    *lossless = (bigint->length() <= 1 && bigint->sign() == false);
+
+    return napi_ok;
+}
+
 extern "C" napi_status napi_get_value_bigint_words(napi_env env,
     napi_value value,
     int* sign_bit,
@@ -2662,32 +2734,45 @@ extern "C" napi_status napi_create_bigint_words(napi_env env,
     const uint64_t* words,
     napi_value* result)
 {
-    NAPI_PREMABLE
-
-    if (UNLIKELY(!result)) {
+    NAPI_PREMABLE;
+    // JSBigInt::createWithLength's size argument is unsigned int
+    if (!env || !result || !words || word_count > UINT_MAX) {
         return napi_invalid_arg;
     }
 
     Zig::GlobalObject* globalObject = toJS(env);
-    JSC::VM& vm = globalObject->vm();
-    auto* bigint = JSC::JSBigInt::tryCreateWithLength(vm, word_count);
-    if (UNLIKELY(!bigint)) {
-        return napi_generic_failure;
+    auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
+    RETURN_IF_EXCEPTION(scope, napi_pending_exception);
+
+    if (word_count == 0) {
+        auto* bigint = JSBigInt::createZero(globalObject);
+        scope.assertNoException();
+        *result = toNapi(bigint, globalObject);
+        return napi_ok;
     }
 
-    // TODO: verify sign bit is consistent
-    bigint->setSign(sign_bit);
+    // JSBigInt requires there are no leading zeroes in the words array, but native modules may have
+    // passed an array containing leading zeroes. so we have to cut those off.
+    while (word_count > 0 && words[word_count - 1] == 0) {
+        word_count--;
+    }
 
-    if (words != nullptr) {
-        const uint64_t* word = words;
-        // TODO: add fast path that uses memcpy here instead of setDigit
-        // we need to add this to JSC. V8 has this optimization
-        for (size_t i = 0; i < word_count; i++) {
-            bigint->setDigit(i, *word++);
-        }
+    // throws RangeError if size is larger than JSC's limit
+    auto* bigint = JSBigInt::createWithLength(globalObject, word_count);
+    RETURN_IF_EXCEPTION(scope, napi_pending_exception);
+    ASSERT(bigint);
+
+    bigint->setSign(sign_bit != 0);
+
+    const uint64_t* current_word = words;
+    // TODO: add fast path that uses memcpy here instead of setDigit
+    // we need to add this to JSC. V8 has this optimization
+    for (size_t i = 0; i < word_count; i++) {
+        bigint->setDigit(i, *current_word++);
     }
 
     *result = toNapi(bigint, globalObject);
+    scope.assertNoException();
     return napi_ok;
 }
 

src/napi/napi.zig

@@ -1032,26 +1032,8 @@ pub export fn napi_create_bigint_uint64(env: napi_env, value: u64, result_: ?*na
     return .ok;
 }
 pub extern fn napi_create_bigint_words(env: napi_env, sign_bit: c_int, word_count: usize, words: [*c]const u64, result: *napi_value) napi_status;
-// TODO: lossless
-pub export fn napi_get_value_bigint_int64(_: napi_env, value_: napi_value, result_: ?*i64, _: *bool) napi_status {
-    log("napi_get_value_bigint_int64", .{});
-    const result = result_ orelse {
-        return invalidArg();
-    };
-    const value = value_.get();
-    result.* = value.toInt64();
-    return .ok;
-}
-// TODO: lossless
-pub export fn napi_get_value_bigint_uint64(_: napi_env, value_: napi_value, result_: ?*u64, _: *bool) napi_status {
-    log("napi_get_value_bigint_uint64", .{});
-    const result = result_ orelse {
-        return invalidArg();
-    };
-    const value = value_.get();
-    result.* = value.toUInt64NoTruncate();
-    return .ok;
-}
+pub extern fn napi_get_value_bigint_int64(env: napi_env, value: napi_value, result: ?*i64, lossless: ?*bool) napi_status;
+pub extern fn napi_get_value_bigint_uint64(env: napi_env, value: napi_value, result: ?*u64, lossless: ?*bool) napi_status;
 
 pub extern fn napi_get_value_bigint_words(env: napi_env, value: napi_value, sign_bit: [*c]c_int, word_count: [*c]usize, words: [*c]u64) napi_status;
 pub extern fn napi_get_all_property_names(env: napi_env, object: napi_value, key_mode: napi_key_collection_mode, key_filter: napi_key_filter, key_conversion: napi_key_conversion, result: *napi_value) napi_status;

test/napi/napi-app/main.cpp

@@ -4,6 +4,7 @@
 
 #include <array>
 #include <cassert>
+#include <cinttypes>
 #include <cmath>
 #include <cstdarg>
 #include <cstdint>
@@ -969,6 +970,112 @@ static napi_value try_add_tag(const Napi::CallbackInfo &info) {
   return Napi::Boolean::New(env, status == napi_ok);
 }
 
+static napi_value bigint_to_i64(const Napi::CallbackInfo &info) {
+  napi_env env = info.Env();
+  // start at 1 is intentional, since argument 0 is the callback to run GC
+  // passed to every function
+  // perform test on all arguments
+  for (size_t i = 1; i < info.Length(); i++) {
+    napi_value bigint = info[i];
+
+    napi_valuetype type;
+    NODE_API_CALL(env, napi_typeof(env, bigint, &type));
+
+    int64_t result = 0;
+    bool lossless = false;
+
+    if (type != napi_bigint) {
+      printf("napi_get_value_bigint_int64 return for non-bigint: %d\n",
+             napi_get_value_bigint_int64(env, bigint, &result, &lossless));
+    } else {
+      NODE_API_CALL(
+          env, napi_get_value_bigint_int64(env, bigint, &result, &lossless));
+      printf("napi_get_value_bigint_int64 result: %" PRId64 "\n", result);
+      printf("lossless: %s\n", lossless ? "true" : "false");
+    }
+  }
+
+  return ok(env);
+}
+
+static napi_value bigint_to_u64(const Napi::CallbackInfo &info) {
+  napi_env env = info.Env();
+  // start at 1 is intentional, since argument 0 is the callback to run GC
+  // passed to every function
+  // perform test on all arguments
+  for (size_t i = 1; i < info.Length(); i++) {
+    napi_value bigint = info[i];
+
+    napi_valuetype type;
+    NODE_API_CALL(env, napi_typeof(env, bigint, &type));
+
+    uint64_t result;
+    bool lossless;
+
+    if (type != napi_bigint) {
+      printf("napi_get_value_bigint_uint64 return for non-bigint: %d\n",
+             napi_get_value_bigint_uint64(env, bigint, &result, &lossless));
+    } else {
+      NODE_API_CALL(
+          env, napi_get_value_bigint_uint64(env, bigint, &result, &lossless));
+      printf("napi_get_value_bigint_uint64 result: %" PRIu64 "\n", result);
+      printf("lossless: %s\n", lossless ? "true" : "false");
+    }
+  }
+
+  return ok(env);
+}
+
+static napi_value bigint_to_64_null(const Napi::CallbackInfo &info) {
+  napi_env env = info.Env();
+
+  napi_value bigint;
+  NODE_API_CALL(env, napi_create_bigint_int64(env, 5, &bigint));
+
+  int64_t result_signed;
+  uint64_t result_unsigned;
+  bool lossless;
+
+  printf("status (int64, null result) = %d\n",
+         napi_get_value_bigint_int64(env, bigint, nullptr, &lossless));
+  printf("status (int64, null lossless) = %d\n",
+         napi_get_value_bigint_int64(env, bigint, &result_signed, nullptr));
+  printf("status (uint64, null result) = %d\n",
+         napi_get_value_bigint_uint64(env, bigint, nullptr, &lossless));
+  printf("status (uint64, null lossless) = %d\n",
+         napi_get_value_bigint_uint64(env, bigint, &result_unsigned, nullptr));
+
+  return ok(env);
+}
+
+static napi_value create_weird_bigints(const Napi::CallbackInfo &info) {
+  // create bigints by passing weird parameters to napi_create_bigint_words
+  napi_env env = info.Env();
+
+  std::array<napi_value, 5> bigints;
+  std::array<uint64_t, 4> words{{123, 0, 0, 0}};
+
+  NODE_API_CALL(env, napi_create_bigint_int64(env, 0, &bigints[0]));
+  NODE_API_CALL(env, napi_create_bigint_uint64(env, 0, &bigints[1]));
+  // sign is not 0 or 1 (should be interpreted as negative)
+  NODE_API_CALL(env,
+                napi_create_bigint_words(env, 2, 1, words.data(), &bigints[2]));
+  // leading zeroes in word representation
+  NODE_API_CALL(env,
+                napi_create_bigint_words(env, 0, 4, words.data(), &bigints[3]));
+  // zero
+  NODE_API_CALL(env,
+                napi_create_bigint_words(env, 1, 0, words.data(), &bigints[4]));
+
+  napi_value array;
+  NODE_API_CALL(env,
+                napi_create_array_with_length(env, bigints.size(), &array));
+  for (size_t i = 0; i < bigints.size(); i++) {
+    NODE_API_CALL(env, napi_set_element(env, array, (uint32_t)i, bigints[i]));
+  }
+  return array;
+}
+
 Napi::Value RunCallback(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
   // this function is invoked without the GC callback
@@ -1035,6 +1142,11 @@ Napi::Object InitAll(Napi::Env env, Napi::Object exports1) {
   exports.Set("add_tag", Napi::Function::New(env, add_tag));
   exports.Set("try_add_tag", Napi::Function::New(env, try_add_tag));
   exports.Set("check_tag", Napi::Function::New(env, check_tag));
+  exports.Set("bigint_to_i64", Napi::Function::New(env, bigint_to_i64));
+  exports.Set("bigint_to_u64", Napi::Function::New(env, bigint_to_u64));
+  exports.Set("bigint_to_64_null", Napi::Function::New(env, bigint_to_64_null));
+  exports.Set("create_weird_bigints",
+              Napi::Function::New(env, create_weird_bigints));
 
   napitests::register_wrap_tests(env, exports);
 

test/napi/napi-app/module.js

@@ -490,4 +490,8 @@ nativeTests.test_remove_wrap_lifetime_with_strong_ref = async () => {
   await gcUntil(() => nativeTests.get_object_from_ref() === undefined);
 };
 
+nativeTests.test_create_bigint_words = () => {
+  console.log(nativeTests.create_weird_bigints());
+};
+
 module.exports = nativeTests;

test/napi/napi.test.ts

@@ -349,6 +349,34 @@ describe("napi", () => {
       checkSameOutput("test_remove_wrap_lifetime_with_strong_ref", []);
     });
   });
+
+  describe("bigint conversion to int64/uint64", () => {
+    it("works", () => {
+      const tests = [-1n, 0n, 1n];
+      for (const power of [63, 64, 65]) {
+        for (const sign of [-1, 1]) {
+          const boundary = BigInt(sign) * 2n ** BigInt(power);
+          tests.push(boundary, boundary - 1n, boundary + 1n);
+        }
+      }
+
+      const testsString = "[" + tests.map(bigint => bigint.toString() + "n").join(",") + "]";
+      checkSameOutput("bigint_to_i64", testsString);
+      checkSameOutput("bigint_to_u64", testsString);
+    });
+    it("returns the right error code", () => {
+      const badTypes = '[null, undefined, 5, "123", "abc"]';
+      checkSameOutput("bigint_to_i64", badTypes);
+      checkSameOutput("bigint_to_u64", badTypes);
+      checkSameOutput("bigint_to_64_null", []);
+    });
+  });
+
+  describe("create_bigint_words", () => {
+    it("works", () => {
+      checkSameOutput("test_create_bigint_words", []);
+    });
+  });
 });
 
 function checkSameOutput(test: string, args: any[] | string) {

test/package-json-lint.test.ts

@@ -22,20 +22,25 @@ describe("package.json dependencies must be exact versions", async () => {
         optionalDependencies = {},
       } = await Bun.file(join(dir, "./package.json")).json();
 
+      // Hyphen is necessary to accept prerelease versions like "1.1.3-alpha.7"
+      // This regex still forbids semver ranges like "1.0.0 - 1.2.0", as those must have spaces
+      // around the hyphen.
+      const okRegex = /^([a-zA-Z0-9\.\-])+$/;
+
       for (const [name, dep] of Object.entries(dependencies)) {
-        expect(dep).toMatch(/^([a-zA-Z0-9\.])+$/);
+        expect(dep, `dependency ${name} specifies non-exact version "${dep}"`).toMatch(okRegex);
       }
 
       for (const [name, dep] of Object.entries(devDependencies)) {
-        expect(dep).toMatch(/^([a-zA-Z0-9\.])+$/);
+        expect(dep, `dev dependency ${name} specifies non-exact version "${dep}"`).toMatch(okRegex);
       }
 
       for (const [name, dep] of Object.entries(peerDependencies)) {
-        expect(dep).toMatch(/^([a-zA-Z0-9\.])+$/);
+        expect(dep, `peer dependency ${name} specifies non-exact version "${dep}"`).toMatch(okRegex);
       }
 
       for (const [name, dep] of Object.entries(optionalDependencies)) {
-        expect(dep).toMatch(/^([a-zA-Z0-9\.])+$/);
+        expect(dep, `optional dependency ${name} specifies non-exact version "${dep}"`).toMatch(okRegex);
       }
     });
   }

test/package.json

@@ -10,6 +10,7 @@
   },
   "dependencies": {
     "@azure/service-bus": "7.9.4",
+    "@duckdb/node-api": "1.1.3-alpha.7",
     "@grpc/grpc-js": "1.12.0",
     "@grpc/proto-loader": "0.7.10",
     "@napi-rs/canvas": "0.1.65",
@@ -33,6 +34,7 @@
     "iconv-lite": "0.6.3",
     "isbot": "5.1.13",
     "jest-extended": "4.0.0",
+    "jimp": "1.6.0",
     "jsonwebtoken": "9.0.2",
     "jws": "4.0.0",
     "lodash": "4.17.21",
@@ -69,8 +71,7 @@
     "webpack": "5.88.0",
     "webpack-cli": "4.7.2",
     "xml2js": "0.6.2",
-    "yargs": "17.7.2",
-    "jimp": "1.6.0"
+    "yargs": "17.7.2"
   },
   "private": true,
   "scripts": {