From ec814e4c4bbc7cf43387b9fc0657a3023c069e47 Mon Sep 17 00:00:00 2001 From: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com> Date: Sun, 9 Feb 2025 16:27:52 -0500 Subject: [PATCH 1/5] test(#309): Failing test Demonstrating that the current implementation of `Array`'s `Bind` instance causes `RangeError: Maximum call stack size exceeded` when the output of `f` in `ma >>= f` is sufficiently large. This is due to usage of `Function.prototype.apply`. From [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions): > But beware: by using apply() (or the spread syntax) with an arbitrarily long arguments list, you run the risk of exceeding the JavaScript engine's argument length limit. > The consequences of calling a function with too many arguments (that is, more than tens of thousands of arguments) is unspecified and varies across engines. (The JavaScriptCore engine has a hard-coded [argument limit of 65536](https://webkit.org/b/80797).) Node v20.18.1 seems to have a higher limit around 106,000. --- test/Test/Main.js | 8 ++++++++ test/Test/Main.purs | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/Test/Main.js b/test/Test/Main.js index b25cdaca..72ebcd92 100644 --- a/test/Test/Main.js +++ b/test/Test/Main.js @@ -40,3 +40,11 @@ export function testNumberShow(showNumber) { ]); }; } + +export function makeArray(length) { + var arr = []; + for (var i = 0; i < length; i++) { + arr.push(i); + } + return arr; +} diff --git a/test/Test/Main.purs b/test/Test/Main.purs index 13ea2cce..fd763cf0 100644 --- a/test/Test/Main.purs +++ b/test/Test/Main.purs @@ -22,6 +22,7 @@ main = do testReflectType testReifyType testSignum + testArrayBind foreign import testNumberShow :: (Number -> String) -> AlmostEff @@ -189,3 +190,14 @@ testSignum = do assert "signum positive zero" $ show (1.0/(signum 0.0)) == "Infinity" assert "Clarifies what 'signum negative zero' test is doing" $ show (1.0/(-0.0)) == "-Infinity" assert "signum negative zero" $ show (1.0/(signum (-0.0))) == "-Infinity" + +foreign import makeArray :: Int -> Array Int + +testArrayBind :: AlmostEff +testArrayBind = do + assert "Array bind does not cause RangeError" do + let + _ = do + _ <- [unit] + makeArray 106_000 + true From a7159c7e56cc67cd17168c439c01e661bc32bada Mon Sep 17 00:00:00 2001 From: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com> Date: Sun, 9 Feb 2025 22:03:56 -0500 Subject: [PATCH 2/5] fix(#309): Use `flatMap` if supported by runtime --- src/Control/Bind.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Control/Bind.js b/src/Control/Bind.js index fa0dbaeb..b6a3621d 100644 --- a/src/Control/Bind.js +++ b/src/Control/Bind.js @@ -1,4 +1,10 @@ export const arrayBind = function (arr) { + if (typeof Array.prototype.flatMap === "function") { + return function (f) { + return arr.flatMap(f); + }; + } + return function (f) { var result = []; for (var i = 0, l = arr.length; i < l; i++) { From e9ad550295918e59ba5e4e2f670ba2e5724e068f Mon Sep 17 00:00:00 2001 From: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com> Date: Sun, 9 Feb 2025 22:26:30 -0500 Subject: [PATCH 3/5] fix(#309): Use simple stack-safe fallback --- src/Control/Bind.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Control/Bind.js b/src/Control/Bind.js index b6a3621d..7c77f4a4 100644 --- a/src/Control/Bind.js +++ b/src/Control/Bind.js @@ -6,9 +6,12 @@ export const arrayBind = function (arr) { } return function (f) { - var result = []; - for (var i = 0, l = arr.length; i < l; i++) { - Array.prototype.push.apply(result, f(arr[i])); + const result = []; + for (let i = 0, l = arr.length; i < l; i++) { + const xs = f(arr[i]); + for (let j = 0, m = xs.length; j < m; j++) { + result.push(xs[j]); + } } return result; }; From bed230485c54ffa1fa0a47f33dcc865c00693cc1 Mon Sep 17 00:00:00 2001 From: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com> Date: Sun, 9 Feb 2025 23:45:24 -0500 Subject: [PATCH 4/5] chore(#309): Add to CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7e5af83..5f2f4d42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Breaking changes: New features: Bugfixes: +- Avoid `RangeError` in `arrayBind` foreign implementation (#314 by @pete-murphy) Other improvements: From fbb41ce14c2e7900d1da80cf7425db6a8a0bc82f Mon Sep 17 00:00:00 2001 From: Peter Murphy <26548438+ptrfrncsmrph@users.noreply.github.com> Date: Fri, 14 Feb 2025 19:01:24 -0500 Subject: [PATCH 5/5] feat(#309): Address feedback from code review Using static check to determine if `Array.prototype.flatMap` is available, and use `var` instead of `let` in for loop to match existing code style. --- src/Control/Bind.js | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/Control/Bind.js b/src/Control/Bind.js index 7c77f4a4..bf79719f 100644 --- a/src/Control/Bind.js +++ b/src/Control/Bind.js @@ -1,18 +1,21 @@ -export const arrayBind = function (arr) { - if (typeof Array.prototype.flatMap === "function") { - return function (f) { - return arr.flatMap(f); - }; - } - - return function (f) { - const result = []; - for (let i = 0, l = arr.length; i < l; i++) { - const xs = f(arr[i]); - for (let j = 0, m = xs.length; j < m; j++) { - result.push(xs[j]); - } +export const arrayBind = + typeof Array.prototype.flatMap === "function" + ? function (arr) { + return function (f) { + return arr.flatMap(f); + }; } - return result; - }; -}; + : function (arr) { + return function (f) { + var result = []; + var l = arr.length; + for (var i = 0; i < l; i++) { + var xs = f(arr[i]); + var k = xs.length; + for (var j = 0; j < k; j++) { + result.push(xs[j]); + } + } + return result; + }; + };