lib: use safe methods from primordials

This changes the primordials to expose built-in prototypes with their methods already uncurried. The uncurryThis function is therefore moved to the primordials. All uses of uncurryThis on built-ins are changed to import the relevant prototypes from primordials. All uses of Function.call.bind are also changed to use primordials. PR-URL: #27096 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
nodejs · Apr 8, 2019 · 112cc7c · 112cc7c
1 parent 969bd1e
commit 112cc7c
Show file tree

Hide file tree

Showing 20 changed files with 206 additions and 189 deletions.
diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
@@ -21,6 +21,8 @@
 
 'use strict';
 
+const { ObjectPrototype } = primordials;
+
 const assert = require('internal/assert');
 const Stream = require('stream');
 const internalUtil = require('internal/util');
@@ -53,8 +55,6 @@ const { CRLF, debug } = common;
 
 const kIsCorked = Symbol('isCorked');
 
-const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);
-
 const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
 const RE_TE_CHUNKED = common.chunkExpression;
 
@@ -310,7 +310,7 @@ function _storeHeader(firstLine, headers) {
       }
     } else {
       for (const key in headers) {
-        if (hasOwnProperty(headers, key)) {
+        if (ObjectPrototype.hasOwnProperty(headers, key)) {
           processHeader(this, state, key, headers[key], true);
         }
       }

diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const { Reflect } = primordials;
+const { FunctionPrototype, Reflect } = primordials;
 
 const {
   ERR_ASYNC_TYPE,
@@ -77,8 +77,6 @@ const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve,
         kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId,
         kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants;
 
-const FunctionBind = Function.call.bind(Function.prototype.bind);
-
 // Used in AsyncHook and AsyncResource.
 const async_id_symbol = Symbol('asyncId');
 const trigger_async_id_symbol = Symbol('triggerAsyncId');
@@ -181,7 +179,7 @@ function emitHook(symbol, asyncId) {
 }
 
 function emitHookFactory(symbol, name) {
-  const fn = FunctionBind(emitHook, undefined, symbol);
+  const fn = FunctionPrototype.bind(emitHook, undefined, symbol);
 
   // Set the name property of the function as it looks good in the stack trace.
   Object.defineProperty(fn, 'name', {

diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -225,7 +225,7 @@ NativeModule.prototype.compileForPublicLoader = function(needToProxify) {
 };
 
 const getOwn = (target, property, receiver) => {
-  return Reflect.apply(ObjectPrototype.hasOwnProperty, target, [property]) ?
+  return ObjectPrototype.hasOwnProperty(target, property) ?
     Reflect.get(target, property, receiver) :
     undefined;
 };
@@ -239,8 +239,7 @@ NativeModule.prototype.proxifyExports = function() {
 
   const update = (property, value) => {
     if (this.reflect !== undefined &&
-        Reflect.apply(ObjectPrototype.hasOwnProperty,
-                      this.reflect.exports, [property]))
+        ObjectPrototype.hasOwnProperty(this.reflect.exports, property))
       this.reflect.exports[property].set(value);
   };
 
@@ -254,7 +253,7 @@ NativeModule.prototype.proxifyExports = function() {
           !Reflect.has(handler, 'get')) {
         handler.get = (target, prop, receiver) => {
           const value = Reflect.get(target, prop, receiver);
-          if (Reflect.apply(ObjectPrototype.hasOwnProperty, target, [prop]))
+          if (ObjectPrototype.hasOwnProperty(target, prop))
             update(prop, value);
           return value;
         };

diff --git a/lib/internal/bootstrap/primordials.js b/lib/internal/bootstrap/primordials.js
@@ -12,6 +12,19 @@
 // `primordials.Object` where `primordials` is a lexical variable passed
 // by the native module compiler.
 
+const ReflectApply = Reflect.apply;
+
+// This function is borrowed from the function with the same name on V8 Extras'
+// `utils` object. V8 implements Reflect.apply very efficiently in conjunction
+// with the spread syntax, such that no additional special case is needed for
+// function calls w/o arguments.
+// Refs: https://github.com/v8/v8/blob/d6ead37d265d7215cf9c5f768f279e21bd170212/src/js/prologue.js#L152-L156
+function uncurryThis(func) {
+  return (thisArg, ...args) => ReflectApply(func, thisArg, args);
+}
+
+primordials.uncurryThis = uncurryThis;
+
 function copyProps(src, dest) {
   for (const key of Reflect.ownKeys(src)) {
     if (!Reflect.getOwnPropertyDescriptor(dest, key)) {
@@ -23,6 +36,18 @@ function copyProps(src, dest) {
   }
 }
 
+function copyPrototype(src, dest) {
+  for (const key of Reflect.ownKeys(src)) {
+    if (!Reflect.getOwnPropertyDescriptor(dest, key)) {
+      const desc = Reflect.getOwnPropertyDescriptor(src, key);
+      if (typeof desc.value === 'function') {
+        desc.value = uncurryThis(desc.value);
+      }
+      Reflect.defineProperty(dest, key, desc);
+    }
+  }
+}
+
 function makeSafe(unsafe, safe) {
   copyProps(unsafe.prototype, safe.prototype);
   copyProps(unsafe, safe);
@@ -64,17 +89,23 @@ primordials.SafePromise = makeSafe(
 // Create copies of intrinsic objects
 [
   'Array',
+  'BigInt',
+  'Boolean',
   'Date',
+  'Error',
   'Function',
+  'Map',
+  'Number',
   'Object',
   'RegExp',
+  'Set',
   'String',
   'Symbol',
 ].forEach((name) => {
   const target = primordials[name] = Object.create(null);
   copyProps(global[name], target);
   const proto = primordials[name + 'Prototype'] = Object.create(null);
-  copyProps(global[name].prototype, proto);
+  copyPrototype(global[name].prototype, proto);
 });
 
 Object.setPrototypeOf(primordials, null);

diff --git a/lib/internal/cli_table.js b/lib/internal/cli_table.js
@@ -1,10 +1,9 @@
 'use strict';
 
-const { Math } = primordials;
+const { Math, ObjectPrototype } = primordials;
 
 const { Buffer } = require('buffer');
 const { removeColors } = require('internal/util');
-const HasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);
 
 // The use of Unicode characters below is the only non-comment use of non-ASCII
 // Unicode characters in Node.js built-in modules. If they are ever removed or
@@ -61,7 +60,8 @@ const table = (head, columns) => {
     for (var j = 0; j < longestColumn; j++) {
       if (rows[j] === undefined)
         rows[j] = [];
-      const value = rows[j][i] = HasOwnProperty(column, j) ? column[j] : '';
+      const value = rows[j][i] =
+        ObjectPrototype.hasOwnProperty(column, j) ? column[j] : '';
       const width = columnWidths[i] || 0;
       const counted = countSymbols(value);
       columnWidths[i] = Math.max(width, counted);

diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js
@@ -3,7 +3,7 @@
 // The Console constructor is not actually used to construct the global
 // console. It's exported for backwards compatibility.
 
-const { Reflect } = primordials;
+const { ObjectPrototype, Reflect } = primordials;
 
 const { trace } = internalBinding('trace_events');
 const {
@@ -36,7 +36,6 @@ const {
   keys: ObjectKeys,
   values: ObjectValues,
 } = Object;
-const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);
 
 const {
   isArray: ArrayIsArray,
@@ -493,7 +492,8 @@ const consoleMethods = {
         for (const key of keys) {
           if (map[key] === undefined)
             map[key] = [];
-          if ((primitive && properties) || !hasOwnProperty(item, key))
+          if ((primitive && properties) ||
+               !ObjectPrototype.hasOwnProperty(item, key))
             map[key][i] = '';
           else
             map[key][i] = _inspect(item[key]);

diff --git a/lib/internal/error-serdes.js b/lib/internal/error-serdes.js
@@ -2,27 +2,17 @@
 
 const Buffer = require('buffer').Buffer;
 const {
-  SafeSet,
+  ArrayPrototype,
+  FunctionPrototype,
   Object,
   ObjectPrototype,
-  FunctionPrototype,
-  ArrayPrototype
+  SafeSet,
 } = primordials;
 
 const kSerializedError = 0;
 const kSerializedObject = 1;
 const kInspectedError = 2;
 
-const GetPrototypeOf = Object.getPrototypeOf;
-const GetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
-const GetOwnPropertyNames = Object.getOwnPropertyNames;
-const DefineProperty = Object.defineProperty;
-const Assign = Object.assign;
-const ObjectPrototypeToString =
-    FunctionPrototype.call.bind(ObjectPrototype.toString);
-const ForEach = FunctionPrototype.call.bind(ArrayPrototype.forEach);
-const Call = FunctionPrototype.call.bind(FunctionPrototype.call);
-
 const errors = {
   Error, TypeError, RangeError, URIError, SyntaxError, ReferenceError, EvalError
 };
@@ -32,17 +22,18 @@ function TryGetAllProperties(object, target = object) {
   const all = Object.create(null);
   if (object === null)
     return all;
-  Assign(all, TryGetAllProperties(GetPrototypeOf(object), target));
-  const keys = GetOwnPropertyNames(object);
-  ForEach(keys, (key) => {
+  Object.assign(all,
+                TryGetAllProperties(Object.getPrototypeOf(object), target));
+  const keys = Object.getOwnPropertyNames(object);
+  ArrayPrototype.forEach(keys, (key) => {
     let descriptor;
     try {
-      descriptor = GetOwnPropertyDescriptor(object, key);
+      descriptor = Object.getOwnPropertyDescriptor(object, key);
     } catch { return; }
     const getter = descriptor.get;
     if (getter && key !== '__proto__') {
       try {
-        descriptor.value = Call(getter, target);
+        descriptor.value = FunctionPrototype.call(getter, target);
       } catch {}
     }
     if ('value' in descriptor && typeof descriptor.value !== 'function') {
@@ -59,10 +50,10 @@ function GetConstructors(object) {
 
   for (var current = object;
     current !== null;
-    current = GetPrototypeOf(current)) {
-    const desc = GetOwnPropertyDescriptor(current, 'constructor');
+    current = Object.getPrototypeOf(current)) {
+    const desc = Object.getOwnPropertyDescriptor(current, 'constructor');
     if (desc && desc.value) {
-      DefineProperty(constructors, constructors.length, {
+      Object.defineProperty(constructors, constructors.length, {
         value: desc.value, enumerable: true
       });
     }
@@ -72,7 +63,7 @@ function GetConstructors(object) {
 }
 
 function GetName(object) {
-  const desc = GetOwnPropertyDescriptor(object, 'name');
+  const desc = Object.getOwnPropertyDescriptor(object, 'name');
   return desc && desc.value;
 }
 
@@ -89,7 +80,7 @@ function serializeError(error) {
   if (!serialize) serialize = require('v8').serialize;
   try {
     if (typeof error === 'object' &&
-        ObjectPrototypeToString(error) === '[object Error]') {
+        ObjectPrototype.toString(error) === '[object Error]') {
       const constructors = GetConstructors(error);
       for (var i = 0; i < constructors.length; i++) {
         const name = GetName(constructors[i]);

diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js
@@ -1,16 +1,16 @@
 'use strict';
 
+const { ArrayPrototype } = primordials;
+
 const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
 const debug = require('util').debuglog('esm');
-const ArrayJoin = Function.call.bind(Array.prototype.join);
-const ArrayMap = Function.call.bind(Array.prototype.map);
 
 const createDynamicModule = (exports, url = '', evaluate) => {
   debug('creating ESM facade for %s with exports: %j', url, exports);
-  const names = ArrayMap(exports, (name) => `${name}`);
+  const names = ArrayPrototype.map(exports, (name) => `${name}`);
 
   const source = `
-${ArrayJoin(ArrayMap(names, (name) =>
+${ArrayPrototype.join(ArrayPrototype.map(names, (name) =>
     `let $${name};
 export { $${name} as ${name} };
 import.meta.exports.${name} = {

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const { FunctionPrototype } = primordials;
+
 const {
   ERR_INVALID_RETURN_PROPERTY,
   ERR_INVALID_RETURN_PROPERTY_VALUE,
@@ -18,8 +20,6 @@ const createDynamicModule = require(
 const { translators } = require('internal/modules/esm/translators');
 const { ModuleWrap } = internalBinding('module_wrap');
 
-const FunctionBind = Function.call.bind(Function.prototype.bind);
-
 const debug = require('internal/util/debuglog').debuglog('esm');
 
 const {
@@ -132,9 +132,11 @@ class Loader {
   hook({ resolve, dynamicInstantiate }) {
     // Use .bind() to avoid giving access to the Loader instance when called.
     if (resolve !== undefined)
-      this._resolve = FunctionBind(resolve, null);
-    if (dynamicInstantiate !== undefined)
-      this._dynamicInstantiate = FunctionBind(dynamicInstantiate, null);
+      this._resolve = FunctionPrototype.bind(resolve, null);
+    if (dynamicInstantiate !== undefined) {
+      this._dynamicInstantiate =
+        FunctionPrototype.bind(dynamicInstantiate, null);
+    }
   }
 
   async getModuleJob(specifier, parentURL) {

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -1,5 +1,11 @@
 'use strict';
 
+const {
+  SafeMap,
+  StringPrototype,
+  JSON
+} = primordials;
+
 const { NativeModule } = require('internal/bootstrap/loaders');
 const { ModuleWrap, callbackMap } = internalBinding('module_wrap');
 const {
@@ -11,10 +17,6 @@ const internalURLModule = require('internal/url');
 const createDynamicModule = require(
   'internal/modules/esm/create_dynamic_module');
 const fs = require('fs');
-const {
-  SafeMap,
-  JSON
-} = primordials;
 const { fileURLToPath, URL } = require('url');
 const { debuglog } = require('internal/util/debuglog');
 const { promisify } = require('internal/util');
@@ -23,7 +25,6 @@ const {
   ERR_UNKNOWN_BUILTIN_MODULE
 } = require('internal/errors').codes;
 const readFileAsync = promisify(fs.readFile);
-const StringReplace = Function.call.bind(String.prototype.replace);
 const JsonParse = JSON.parse;
 
 const debug = debuglog('esm');
@@ -67,7 +68,8 @@ translators.set('commonjs', async function commonjsStrategy(url, isMain) {
     return cached;
   }
   const module = CJSModule._cache[
-    isWindows ? StringReplace(pathname, winSepRegEx, '\\') : pathname];
+    isWindows ? StringPrototype.replace(pathname, winSepRegEx, '\\') : pathname
+  ];
   if (module && module.loaded) {
     const exports = module.exports;
     return createDynamicModule(['default'], url, (reflect) => {
@@ -110,7 +112,7 @@ translators.set('json', async function jsonStrategy(url) {
   debug(`Loading JSONModule ${url}`);
   const pathname = fileURLToPath(url);
   const modulePath = isWindows ?
-    StringReplace(pathname, winSepRegEx, '\\') : pathname;
+    StringPrototype.replace(pathname, winSepRegEx, '\\') : pathname;
   let module = CJSModule._cache[modulePath];
   if (module && module.loaded) {
     const exports = module.exports;