From 728fee22e93df0f281bd3542b993e8384ba5eed1 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Wed, 20 Jan 2021 12:50:00 +0100 Subject: [PATCH 1/2] =?UTF-8?q?lib:=20add=C2=A0`bound=C2=A0apply`=C2=A0var?= =?UTF-8?q?iants=20of=C2=A0varargs=C2=A0`primordials`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Antoine du Hamel PR-URL: https://github.com/nodejs/node/pull/37005 Reviewed-By: Antoine du Hamel Reviewed-By: Rich Trott --- lib/internal/per_context/primordials.js | 64 +++++++++++++++++++++---- lib/internal/util/inspect.js | 5 +- lib/repl.js | 1 + 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index ec1316d3245a41..c30451b05bdac2 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -21,10 +21,36 @@ const { // `uncurryThis` is equivalent to `func => Function.prototype.call.bind(func)`. // It is using `bind.bind(call)` to avoid using `Function.prototype.bind` // and `Function.prototype.call` after it may have been mutated by users. -const { bind, call } = Function.prototype; +const { apply, bind, call } = Function.prototype; const uncurryThis = bind.bind(call); primordials.uncurryThis = uncurryThis; +// `applyBind` is equivalent to `func => Function.prototype.apply.bind(func)`. +// It is using `bind.bind(apply)` to avoid using `Function.prototype.bind` +// and `Function.prototype.apply` after it may have been mutated by users. +const applyBind = bind.bind(apply); +primordials.applyBind = applyBind; + +// Methods that accept a variable number of arguments, and thus it's useful to +// also create `${prefix}${key}Apply`, which uses `Function.prototype.apply`, +// instead of `Function.prototype.call`, and thus doesn't require iterator +// destructuring. +const varargsMethods = [ + // 'ArrayPrototypeConcat' is omitted, because it performs the spread + // on its own for arrays and array-likes with a truthy + // @@isConcatSpreadable symbol property. + 'ArrayOf', + 'ArrayPrototypePush', + 'ArrayPrototypeUnshift', + // 'FunctionPrototypeCall' is omitted, since there's 'ReflectApply' + // and 'FunctionPrototypeApply'. + 'MathHypot', + 'MathMax', + 'MathMin', + 'StringPrototypeConcat', + 'TypedArrayOf', +]; + function getNewKey(key) { return typeof key === 'symbol' ? `Symbol${key.description[7].toUpperCase()}${key.description.slice(8)}` : @@ -51,7 +77,13 @@ function copyPropsRenamed(src, dest, prefix) { if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { - ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); + const name = `${prefix}${newKey}`; + ReflectDefineProperty(dest, name, desc); + if (varargsMethods.includes(name)) { + ReflectDefineProperty(dest, `${name}Apply`, { + value: applyBind(desc.value, src), + }); + } } } } @@ -63,10 +95,18 @@ function copyPropsRenamedBound(src, dest, prefix) { if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { - if (typeof desc.value === 'function') { - desc.value = desc.value.bind(src); + const { value } = desc; + if (typeof value === 'function') { + desc.value = value.bind(src); + } + + const name = `${prefix}${newKey}`; + ReflectDefineProperty(dest, name, desc); + if (varargsMethods.includes(name)) { + ReflectDefineProperty(dest, `${name}Apply`, { + value: applyBind(value, src), + }); } - ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); } } } @@ -78,10 +118,18 @@ function copyPrototype(src, dest, prefix) { if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { - if (typeof desc.value === 'function') { - desc.value = uncurryThis(desc.value); + const { value } = desc; + if (typeof value === 'function') { + desc.value = uncurryThis(value); + } + + const name = `${prefix}${newKey}`; + ReflectDefineProperty(dest, name, desc); + if (varargsMethods.includes(name)) { + ReflectDefineProperty(dest, `${name}Apply`, { + value: applyBind(value), + }); } - ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); } } } diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 41d95b468d14f3..88e70647626c6c 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -6,6 +6,7 @@ const { ArrayPrototypeFilter, ArrayPrototypeForEach, ArrayPrototypePush, + ArrayPrototypePushApply, ArrayPrototypeSort, ArrayPrototypeUnshift, BigIntPrototypeValueOf, @@ -665,7 +666,7 @@ function getKeys(value, showHidden) { if (showHidden) { keys = ObjectGetOwnPropertyNames(value); if (symbols.length !== 0) - ArrayPrototypePush(keys, ...symbols); + ArrayPrototypePushApply(keys, symbols); } else { // This might throw if `value` is a Module Namespace Object from an // unevaluated module, but we don't want to perform the actual type @@ -681,7 +682,7 @@ function getKeys(value, showHidden) { } if (symbols.length !== 0) { const filter = (key) => ObjectPrototypePropertyIsEnumerable(value, key); - ArrayPrototypePush(keys, ...ArrayPrototypeFilter(symbols, filter)); + ArrayPrototypePushApply(keys, ArrayPrototypeFilter(symbols, filter)); } } return keys; diff --git a/lib/repl.js b/lib/repl.js index 09c3a1fabb5204..09494c3f2fcf15 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1313,6 +1313,7 @@ function complete(line, callback) { if (!this.useGlobal) { // When the context is not `global`, builtins are not own // properties of it. + // `globalBuiltins` is a `SafeSet`, not an Array-like. ArrayPrototypePush(contextOwnNames, ...globalBuiltins); } ArrayPrototypePush(completionGroups, contextOwnNames); From 742342fa8a96ad74fbf3870bc953ec408cf7a9e8 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Fri, 22 Jan 2021 07:00:00 +0100 Subject: [PATCH 2/2] =?UTF-8?q?test:=20add=C2=A0tests=20for=C2=A0`bound?= =?UTF-8?q?=C2=A0apply`=C2=A0variants=20of=C2=A0varargs=C2=A0`primordials`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/37005 Reviewed-By: Antoine du Hamel Reviewed-By: Rich Trott --- lib/internal/per_context/primordials.js | 3 + lib/readline.js | 3 +- lib/zlib.js | 4 +- test/parallel/test-primordials-apply.js | 74 +++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-primordials-apply.js diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index c30451b05bdac2..358a3ffb1909ea 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -81,6 +81,9 @@ function copyPropsRenamed(src, dest, prefix) { ReflectDefineProperty(dest, name, desc); if (varargsMethods.includes(name)) { ReflectDefineProperty(dest, `${name}Apply`, { + // `src` is bound as the `this` so that the static `this` points + // to the object it was defined on, + // e.g.: `ArrayOfApply` gets a `this` of `Array`: value: applyBind(desc.value, src), }); } diff --git a/lib/readline.js b/lib/readline.js index a284cc8afcf0d0..bad4774a6a4162 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -43,6 +43,7 @@ const { MathCeil, MathFloor, MathMax, + MathMaxApply, NumberIsFinite, NumberIsNaN, ObjectDefineProperty, @@ -634,7 +635,7 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) { // Apply/show completions. const completionsWidth = ArrayPrototypeMap(completions, (e) => getStringWidth(e)); - const width = MathMax(...completionsWidth) + 2; // 2 space padding + const width = MathMaxApply(completionsWidth) + 2; // 2 space padding let maxColumns = MathFloor(this.columns / width) || 1; if (maxColumns === Infinity) { maxColumns = 1; diff --git a/lib/zlib.js b/lib/zlib.js index 7812f5169545d1..5fcd736f6652a1 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -28,7 +28,7 @@ const { ArrayPrototypePush, Error, FunctionPrototypeBind, - MathMax, + MathMaxApply, NumberIsFinite, NumberIsNaN, ObjectDefineProperties, @@ -788,7 +788,7 @@ function createConvenienceMethod(ctor, sync) { }; } -const kMaxBrotliParam = MathMax(...ArrayPrototypeMap( +const kMaxBrotliParam = MathMaxApply(ArrayPrototypeMap( ObjectKeys(constants), (key) => (StringPrototypeStartsWith(key, 'BROTLI_PARAM_') ? constants[key] : diff --git a/test/parallel/test-primordials-apply.js b/test/parallel/test-primordials-apply.js new file mode 100644 index 00000000000000..0901a87ba18777 --- /dev/null +++ b/test/parallel/test-primordials-apply.js @@ -0,0 +1,74 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); + +const assert = require('assert'); +const { + ArrayOfApply, + ArrayPrototypePushApply, + ArrayPrototypeUnshiftApply, + MathMaxApply, + MathMinApply, + StringPrototypeConcatApply, + TypedArrayOfApply, +} = require('internal/test/binding').primordials; + +{ + const arr1 = [1, 2, 3]; + const arr2 = ArrayOfApply(arr1); + + assert.deepStrictEqual(arr2, arr1); + assert.notStrictEqual(arr2, arr1); +} + +{ + const array = [1, 2, 3]; + const i32Array = TypedArrayOfApply(Int32Array, array); + + assert(i32Array instanceof Int32Array); + assert.strictEqual(i32Array.length, array.length); + for (let i = 0, { length } = array; i < length; i++) { + assert.strictEqual(i32Array[i], array[i], `i32Array[${i}] === array[${i}]`); + } +} + +{ + const arr1 = [1, 2, 3]; + const arr2 = [4, 5, 6]; + + const expected = [...arr1, ...arr2]; + + assert.strictEqual(ArrayPrototypePushApply(arr1, arr2), expected.length); + assert.deepStrictEqual(arr1, expected); +} + +{ + const arr1 = [1, 2, 3]; + const arr2 = [4, 5, 6]; + + const expected = [...arr2, ...arr1]; + + assert.strictEqual(ArrayPrototypeUnshiftApply(arr1, arr2), expected.length); + assert.deepStrictEqual(arr1, expected); +} + +{ + const array = [1, 2, 3]; + assert.strictEqual(MathMaxApply(array), 3); + assert.strictEqual(MathMinApply(array), 1); +} + +{ + let hint; + const obj = { [Symbol.toPrimitive](h) { + hint = h; + return '[object Object]'; + } }; + + const args = ['foo ', obj, ' bar']; + const result = StringPrototypeConcatApply('', args); + + assert.strictEqual(hint, 'string'); + assert.strictEqual(result, 'foo [object Object] bar'); +}