Compiled js min/max value with bigint

[remitbri]

We can't use Math.min/max for bigint values.

If the rescript code is simple enough,

let a = 5n
let b = 7n
let c = max(a,b)

the compiled js is simple too:

var c = 5n > 7n ? 5n : 7n;

var a = 5n;

var b = 7n;

But if it's trickier,

let a = (x)=> BigInt.add(x, 5n)
let b = (x)=> BigInt.add(x, 7n)

let c =  max(a(4n), b(9n))

the compiled js is

import * as Caml from "./stdlib/caml.js";

function a(x) {
  return x + 5n;
}

function b(x) {
  return x + 7n;
}

var c = Caml.bigint_max(4n + 5n, 9n + 7n);

Problem:

"Property 'bigint_max' may not exist on type 'typeof import("project_path/node_modules/rescript/lib/es6/caml")'. Did you mean 'int_max'?

[zth]

Thanks for the report!

Who's best to look at this, @mununki perhaps?

[cometkim]

We can't use Math.min/max for bigint values.

You can't use Math.min/max for "bigint" in JS, that operations are for "number" types

[cometkim]

We could add min/max as compiler primitives (and handle it in #7057 too), but that would add some runtime codes anyway. To make it "simple enough"

[mununki]

I'm thinking that adding primitives to have the compiler throw errors when using max or min with bigint might be a good idea. What do you think?

[cometkim]

Actually the compiler's %min / %max primitives already support bigints as well, and I don't think there's reason to drop it.

"Property 'bigint_max' may not exist on type 'typeof import("project_path/node_modules/rescript/lib/es6/caml")'. Did you mean 'int_max'?

What the @remitbri reported is more like not a runtime error, but a Gentype-related one.

[cometkim]

Also note the bigint folding will be a bit improved in v12 by #7029

[mununki]

Actually the compiler's %min / %max primitives already support bigints as well, and I don't think there's reason to drop it.

As far as I know, JavaScript does not support Math.max and Math.min for BigInt. Does this mean you are suggesting adding runtime support for max and min in ReScript? How do you plan to implement the runtime?

[remitbri]

When I was saying "We can't use Math.min/max for bigint values." in the issue description, it was about Javascript support, that's why I wasn't keen on having a compiler primitive. That being said, we could have a BigInt.min (and BigInt.max) that would just compile to a < b ? a : b…

[cometkim]

That's what exactly we have already. Just need to expose it to the API surface

[mununki]

I guess there is an issue with latest master branch:

let max0 = max(1n, 2n)
let min0 = min(1n, 2n)
let c0 = 1n > 2n

let a = 5n
let b = 7n
let c = max(a, b)

compiled to

let max0 = Primitive_bigint.compare(1n, 2n); // should be max?

let min0 = Primitive_bigint.compare(1n, 2n); // should be min?

let c0 = 1n > 2n;

let c = Primitive_bigint.compare(5n, 7n); // should be max?

let a = 5n;

let b = 7n;

[mununki]

Since the PR #7088, it is compiled to:

open! Js.BigInt

let e = x => x + 5n
let f = x => x + 7n

let g = max(e(4n), f(9n))

function e(x) {
  return x + 5n;
}

function f(x) {
  return x + 7n;
}

let g = Primitive_bigint.max(4n + 5n, 9n + 7n);

[mununki]

// Primitive_bigint.js
function max(x, y) {
  if (x > y) {
    return x;
  } else {
    return y;
  }
}