From dee0b0d3022b7571199cfa6da33b8d87dec1bc88 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Wed, 30 Aug 2023 23:35:56 +0100 Subject: [PATCH] Instrument: Simplify skipping internal vars for function names [refactor] --- lib/instrument/blocks.js | 16 +++++++++++++++ lib/instrument/visitors/assignee.js | 28 ++++++--------------------- lib/instrument/visitors/function.js | 9 ++++++++- lib/instrument/visitors/identifier.js | 7 +++---- lib/instrument/visitors/super.js | 6 +++--- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/lib/instrument/blocks.js b/lib/instrument/blocks.js index 1bb662a4..5b5e8b97 100644 --- a/lib/instrument/blocks.js +++ b/lib/instrument/blocks.js @@ -17,6 +17,7 @@ module.exports = { createArgumentsBinding, createNewTargetBinding, getOrCreateExternalVar, + createInternalVar, activateBlock, activateBinding, createBlockTempVar @@ -207,6 +208,21 @@ function getOrCreateExternalVar(externalVars, block, varName, bindingOrExternalV return externalVar; } +/** + * Create an internal var in a function. + * @param {Object} fn - Function object + * @param {string} varName - Var name + * @param {Object} binding - Binding object + * @param {Array} [trail] - Trail for where binding appears (optional) + * @returns {undefined} + */ +function createInternalVar(fn, varName, binding, trail) { + const {internalVars} = fn; + let internalVar = internalVars[varName]; + if (!internalVar) internalVar = internalVars[varName] = {binding, trails: []}; + if (trail) internalVar.trails.push(trail); +} + /** * Activate block. * Called when block contains a binding which is referenced within a function. diff --git a/lib/instrument/visitors/assignee.js b/lib/instrument/visitors/assignee.js index 2c6877fa..79ee1c81 100644 --- a/lib/instrument/visitors/assignee.js +++ b/lib/instrument/visitors/assignee.js @@ -25,9 +25,8 @@ const assert = require('simple-invariant'); const Expression = require('./expression.js'), {IdentifierAssignOnly, IdentifierReadAndAssign} = require('./identifier.js'), MemberExpression = require('./memberExpression.js'), - {createBinding} = require('../blocks.js'), - {visitKey, visitKeyContainer, visitKeyContainerWithEmptyMembers} = require('../visit.js'), - {createArrayOrPush} = require('../../shared/functions.js'); + {createBinding, createInternalVar} = require('../blocks.js'), + {visitKey, visitKeyContainer, visitKeyContainerWithEmptyMembers} = require('../visit.js'); // Exports @@ -157,11 +156,11 @@ function visitConstOrLetIdentifier(node, isConst, state) { const varName = node.name, block = state.currentBlock; assertNoCommonJsVarsClash(block, varName, state); - createBinding(block, varName, {isConst}, state); + const binding = createBinding(block, varName, {isConst}, state); // Record as internal var const fn = state.currentFunction; - if (fn) createArrayOrPush(fn.internalVars, varName, [...state.trail]); + if (fn) createInternalVar(fn, varName, binding, [...state.trail]); } /** @@ -183,24 +182,9 @@ function IdentifierVar(node, state) { return; } - // Leave until later to add to `internalVars` in case binding is redeclared later - // as a function declaration. Vars defined as functions must not be - // added to `internalVars` to avoid their names being mangled. + // Record as internal var const fn = state.currentFunction; - if (fn) state.secondPass(addVarIdentifierToInternalVars, node, binding, varName, fn, [...state.trail]); -} - -/** - * Add identifier to parent function's internal vars. - * @param {Object} node - Identifier AST node (not needed but passed for debug reasons) - * @param {Object} binding - Binding object - * @param {string} varName - Variable name - * @param {Object} fn - Function object - * @param {Array} trail - Trail - * @returns {undefined} - */ -function addVarIdentifierToInternalVars(node, binding, varName, fn, trail) { - if (!binding.isFunction) createArrayOrPush(fn.internalVars, varName, trail); + if (fn) createInternalVar(fn, varName, binding, [...state.trail]); } /** diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index 67fcb644..8677aad9 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -716,6 +716,13 @@ function insertTrackerComment(fnId, fnType, commentHolderNode, commentType, stat * @returns {undefined} */ function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) { + // Remove internal vars for functions + const internalVars = Object.fromEntries( + Object.entries(fn.internalVars) + .filter(([, {binding}]) => !binding?.isFunction) + .map(([varName, {trails}]) => [varName, trails]) + ); + // Create JSON function info string const {children} = fn; let argNames; @@ -737,7 +744,7 @@ function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) { containsEval: fn.containsEval || undefined, containsImport: fn.containsImport || undefined, argNames, - internalVars: fn.internalVars, + internalVars, globalVarNames: fn.globalVarNames.size !== 0 ? [...fn.globalVarNames] : undefined, amendments: fn.amendments.length !== 0 ? fn.amendments.map(({type, blockId, trail}) => [type, blockId, ...trail]).reverse() diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index e9d4dba8..fba98dac 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -16,9 +16,8 @@ module.exports = { // Imports const visitEval = require('./eval.js'), - {getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'), + {getOrCreateExternalVar, activateBlock, activateBinding, createInternalVar} = require('../blocks.js'), {checkInternalVarNameClash} = require('../internalVars.js'), - {createArrayOrPush} = require('../../shared/functions.js'), { CONST_VIOLATION_CONST, CONST_VIOLATION_FUNCTION_THROWING, CONST_VIOLATION_FUNCTION_SILENT } = require('../../shared/constants.js'); @@ -83,7 +82,7 @@ function ThisExpression(node, state) { if (block.id < fn.id) { recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, state); } else if (fn.hasSuperClass) { - createArrayOrPush(fn.internalVars, 'this', [...state.trail]); + createInternalVar(fn, 'this', binding, [...state.trail]); } } @@ -183,7 +182,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // Record if internal var if (block.id >= fn.id) { - if (!binding.isFunction && !binding.argNames) createArrayOrPush(fn.internalVars, varName, trail); + if (!binding.isFunction && !binding.argNames) createInternalVar(fn, varName, binding, trail); return; } diff --git a/lib/instrument/visitors/super.js b/lib/instrument/visitors/super.js index 260ee71d..6f4b9b52 100644 --- a/lib/instrument/visitors/super.js +++ b/lib/instrument/visitors/super.js @@ -19,7 +19,8 @@ module.exports = { // Imports const { - createBindingWithoutNameCheck, getOrCreateExternalVar, activateBlock, createBlockTempVar + createBindingWithoutNameCheck, getOrCreateExternalVar, activateBlock, createBlockTempVar, + createInternalVar } = require('../blocks.js'), {getProp} = require('../../shared/functions.js'), {SUPER_CALL, SUPER_EXPRESSION} = require('../../shared/constants.js'); @@ -146,8 +147,7 @@ function activateSuperBinding(superBlock, state) { * @returns {undefined} */ function createInternalVarForThis(fn) { - const {internalVars} = fn; - if (!internalVars.this) internalVars.this = []; + createInternalVar(fn, 'this', null, null); } /**