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

Workaround for Math variadic functions #82

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Next version

### API changes

- Do not return `Infinity` or `-Infinity` when passing an empty array to `Math.Int.minMany`, `Math.Int.maxMany`, `Math.minMany` and `Math.maxMany` functions. https://github.com/rescript-association/rescript-core/pull/82


## 0.5.0

### API changes
Expand Down
40 changes: 39 additions & 1 deletion src/Core__Math.mjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,50 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as Caml_splice_call from "rescript/lib/es6/caml_splice_call.js";

var Constants = {};

var Int = {};
function minMany(arr) {
if (arr.length !== 0) {
return Caml_splice_call.spliceApply(Math.min, [arr]);
} else {
return 0;
}
}

function maxMany(arr) {
if (arr.length !== 0) {
return Caml_splice_call.spliceApply(Math.max, [arr]);
} else {
return 0;
}
}

var Int = {
minMany: minMany,
maxMany: maxMany
};

function minMany$1(arr) {
if (arr.length !== 0) {
return Caml_splice_call.spliceApply(Math.min, [arr]);
} else {
return 0.0;
}
}

function maxMany$1(arr) {
if (arr.length !== 0) {
return Caml_splice_call.spliceApply(Math.max, [arr]);
} else {
return 0.0;
}
}

export {
Constants ,
Int ,
minMany$1 as minMany,
maxMany$1 as maxMany,
}
/* No side effect */
28 changes: 24 additions & 4 deletions src/Core__Math.res
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,19 @@ module Int = {
@val external clz32: int => int = "Math.clz32"
@val external imul: (int, int) => int = "Math.imul"
@val external min: (int, int) => int = "Math.min"
@variadic @val external minMany: array<int> => int = "Math.min"
@variadic @val external _minMany: array<int> => int = "Math.min"
let minMany = arr =>
switch arr {
| [] => 0
| arr => _minMany(arr)
}
@val external max: (int, int) => int = "Math.max"
@variadic @val external maxMany: array<int> => int = "Math.max"
@variadic @val external _maxMany: array<int> => int = "Math.max"
let maxMany = arr =>
switch arr {
| [] => 0
| arr => _maxMany(arr)
}
@val external pow: (int, ~exp: int) => int = "Math.pow"
@val external sign: int => int = "Math.sign"
}
Expand Down Expand Up @@ -44,9 +54,19 @@ module Int = {
@val external log10: float => float = "Math.log10"
@val external log2: float => float = "Math.log2"
@val external min: (float, float) => float = "Math.min"
@variadic @val external minMany: array<float> => float = "Math.min"
@variadic @val external _minMany: array<float> => float = "Math.min"
let minMany = arr =>
switch arr {
| [] => 0.0
| arr => _minMany(arr)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure the float versions ought to be overridden. Math.min([]) does indeed return 0, and Math.min() returns Infinity, but Math.minMany([1, 2, 3]) maps to Math.min(1, 2, 3), not Math.min([1, 2, 3]), and so it seems more consistent to have Math.minMany([]) map to Math.min(), not Math.min([]).

These are also edge cases that are very unlikely to occur in practice, since variadic calls require syntactic arrays there's little chance of it happening by accident. I'd suggest the aim of this is restricted to just making the types sound.

@val external max: (float, float) => float = "Math.max"
@variadic @val external maxMany: array<float> => float = "Math.max"
@variadic @val external _maxMany: array<float> => float = "Math.max"
let maxMany = arr =>
switch arr {
| [] => 0.0
| arr => _maxMany(arr)
}
@val external pow: (float, ~exp: float) => float = "Math.pow"
@val external random: unit => float = "Math.random"
@val external round: float => float = "Math.round"
Expand Down
24 changes: 8 additions & 16 deletions src/Core__Math.resi
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,10 @@ module Int: {
```rescript
Math.Int.minMany([1, 2]) // 1
Math.Int.minMany([-1, -2]) // -2
Math.Int.minMany([])->Float.isFinite // false
Math.Int.minMany([]) // 0
```
*/
@variadic
@val
external minMany: array<int> => int = "Math.min"
let minMany: array<int> => int

/**
`max(a, b)` returns the maximum of its two integer arguments.
Expand All @@ -240,12 +238,10 @@ module Int: {
```rescript
Math.Int.maxMany([1, 2]) // 2
Math.Int.maxMany([-1, -2]) // -1
Math.Int.maxMany([])->Float.isFinite // false
Math.Int.maxMany([]) // 0
```
*/
@variadic
@val
external maxMany: array<int> => int = "Math.max"
let maxMany: array<int> => int

/**
`pow(a, ~exp)` raises the given base `a` to the given exponent `exp`.
Expand Down Expand Up @@ -650,12 +646,10 @@ See [`Math.min`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Referen
```rescript
Math.minMany([1.0, 2.0]) // 1.0
Math.minMany([-1.0, -2.0]) // -2.0
Math.minMany([])->Float.isFinite // false
Math.minMany([]) // 0.0
```
*/
@variadic
@val
external minMany: array<float> => float = "Math.min"
let minMany: array<float> => float

/**
`max(a, b)` returns the maximum of its two float arguments.
Expand All @@ -681,12 +675,10 @@ See [`Math.max`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Referen
```rescript
Math.maxMany([1.0, 2.0]) // 2.0
Math.maxMany([-1.0, -2.0]) // -1.0
Math.maxMany([])->Float.isFinite // false
Math.maxMany([]) // 0.0
```
*/
@variadic
@val
external maxMany: array<float> => float = "Math.max"
let maxMany: array<float> => float

/**
`pow(a, ~exp)` raises the given base `a` to the given exponent `exp`.
Expand Down