Instrumentation check for internal var name clashes in 1st pass [refa…

…ctor]

lib/init/eval.js

@@ -16,9 +16,8 @@ const {parseImpl} = require('../instrument/instrument.js'),
 	{
 		createBlockWithId, createThisBinding, createNewTargetBinding, createBindingWithoutNameCheck
 	} = require('../instrument/blocks.js'),
-	{
-		INTERNAL_VAR_NAMES_PREFIX, TRACKER_VAR_NAME_BODY, GET_SCOPE_ID_VAR_NAME_BODY
-	} = require('../shared/constants.js'),
+	{createInternalVarNodeFromPrefixNum} = require('../instrument/internalVars.js'),
+	{TRACKER_VAR_NAME_BODY, GET_SCOPE_ID_VAR_NAME_BODY} = require('../shared/constants.js'),
 	specialFunctions = require('../shared/internal.js').functions,
 	getScopeId = require('./getScopeId.js'),
 	assertBug = require('../shared/assertBug.js');
@@ -257,8 +256,8 @@ function compile(code, tracker, filename, isIndirectEval, state, isStrict) {
 function wrapInFunction(ast, internalPrefixNum) {
 	return t.arrowFunctionExpression(
 		[
-			internalVarNode(TRACKER_VAR_NAME_BODY, internalPrefixNum),
-			internalVarNode(GET_SCOPE_ID_VAR_NAME_BODY, internalPrefixNum)
+			createInternalVarNodeFromPrefixNum(TRACKER_VAR_NAME_BODY, internalPrefixNum),
+			createInternalVarNodeFromPrefixNum(GET_SCOPE_ID_VAR_NAME_BODY, internalPrefixNum)
 		],
 		// eslint-disable-next-line no-use-before-define
 		t.callExpression(t.identifier('eval'), [generateCodeStringNode(ast)])
@@ -269,16 +268,12 @@ function wrapInFunctionCall(fnNode, externalPrefixNum) {
 	return t.callExpression(
 		fnNode,
 		[
-			internalVarNode(TRACKER_VAR_NAME_BODY, externalPrefixNum),
-			internalVarNode(GET_SCOPE_ID_VAR_NAME_BODY, externalPrefixNum)
+			createInternalVarNodeFromPrefixNum(TRACKER_VAR_NAME_BODY, externalPrefixNum),
+			createInternalVarNodeFromPrefixNum(GET_SCOPE_ID_VAR_NAME_BODY, externalPrefixNum)
 		]
 	);
 }
 
-function internalVarNode(name, prefixNum) {
-	return t.identifier(`${INTERNAL_VAR_NAMES_PREFIX}${prefixNum || ''}_${name}`);
-}
-
 /**
  * Generate code from AST.
  * If debugging enabled, generates in a more readable form.

lib/instrument/blocks.js

@@ -210,6 +210,8 @@ function getOrCreateExternalVar(externalVars, block, varName, bindingOrExternalV
 /**
  * Activate block.
  * Called when block contains a binding which is referenced within a function.
+ * Must only be called during 2nd compilation pass, as creates a `livepack_scopeId` internal var
+ * and the prefix num for internal vars is only known once 1st pass has completed.
  * @param {Object} block - Block object
  * @param {Object} state - State object
  * @returns {undefined}
@@ -232,6 +234,8 @@ function activateBinding(binding, varName) {
 
 /**
  * Create temp var in block.
+ * Must only be called during 2nd compilation pass, as creates a `livepack_temp` internal var
+ * and the prefix num for internal vars is only known once 1st pass has completed.
  * @param {Object} block - Block object
  * @param {Object} state - State object
  * @returns {undefined}

lib/instrument/internalVars.js

@@ -15,13 +15,6 @@ const {createBlockId} = require('./blocks.js'),
 		SCOPE_ID_VAR_NAME_BODY, TEMP_VAR_NAME_BODY, FN_INFO_VAR_NAME_BODY
 	} = require('../shared/constants.js');
 
-// Constants
-const TRACKER_VAR_NAME = `${INTERNAL_VAR_NAMES_PREFIX}_${TRACKER_VAR_NAME_BODY}`,
-	GET_SCOPE_ID_VAR_NAME = `${INTERNAL_VAR_NAMES_PREFIX}_${GET_SCOPE_ID_VAR_NAME_BODY}`,
-	SCOPE_ID_VAR_NAME = `${INTERNAL_VAR_NAMES_PREFIX}_${SCOPE_ID_VAR_NAME_BODY}_`,
-	TEMP_VAR_NAME = `${INTERNAL_VAR_NAMES_PREFIX}_${TEMP_VAR_NAME_BODY}_`,
-	FN_INFO_VAR_NAME = `${INTERNAL_VAR_NAMES_PREFIX}_${FN_INFO_VAR_NAME_BODY}_`;
-
 // Exports
 
 module.exports = {
@@ -30,40 +23,39 @@ module.exports = {
 	createScopeIdVarNode,
 	createTempVarNode,
 	createFnInfoVarNode,
-	checkInternalVarNameClash,
-	renameInternalVars
+	createInternalVarNodeFromPrefixNum,
+	checkInternalVarNameClash
 };
 
 /*
  * Functions to create internal vars nodes
  */
 function createTrackerVarNode(state) {
-	return createInternalVarNode(TRACKER_VAR_NAME, state);
+	return createInternalVarNode(TRACKER_VAR_NAME_BODY, state);
 }
 
 function createGetScopeIdVarNode(state) {
-	return createInternalVarNode(GET_SCOPE_ID_VAR_NAME, state);
+	return createInternalVarNode(GET_SCOPE_ID_VAR_NAME_BODY, state);
 }
 
 function createScopeIdVarNode(blockId, state) {
-	return createInternalVarNode(`${SCOPE_ID_VAR_NAME}${blockId}`, state);
+	return createInternalVarNode(`${SCOPE_ID_VAR_NAME_BODY}_${blockId}`, state);
 }
 
 function createTempVarNode(state) {
-	return createInternalVarNode(`${TEMP_VAR_NAME}${createBlockId(state)}`, state);
+	return createInternalVarNode(`${TEMP_VAR_NAME_BODY}_${createBlockId(state)}`, state);
 }
 
 function createFnInfoVarNode(id, state) {
-	return createInternalVarNode(`${FN_INFO_VAR_NAME}${id}`, state);
+	return createInternalVarNode(`${FN_INFO_VAR_NAME_BODY}_${id}`, state);
 }
 
 function createInternalVarNode(name, state) {
-	return addToInternalVars(t.identifier(name), state);
+	return createInternalVarNodeFromPrefixNum(name, state.internalVarsPrefixNum);
 }
 
-function addToInternalVars(node, state) {
-	state.internalVarNodes.push(node);
-	return node;
+function createInternalVarNodeFromPrefixNum(name, prefixNum) {
+	return t.identifier(`${INTERNAL_VAR_NAMES_PREFIX}${prefixNum || ''}_${name}`);
 }
 
 /**
@@ -83,19 +75,3 @@ function checkInternalVarNameClash(varName, state) {
 }
 
 const INTERNAL_VAR_NAME_REGEX = new RegExp(`^${INTERNAL_VAR_NAMES_PREFIX}([1-9]\\d*)?_`);
-
-/**
- * Rename all internal vars to ensure names don't clash with other vars, using prefix counter.
- * @param {Object} state - State object
- * @returns {undefined}
- */
-function renameInternalVars(state) {
-	const prefixNum = state.internalVarsPrefixNum;
-	if (prefixNum === 0) return;
-
-	const prefix = `${INTERNAL_VAR_NAMES_PREFIX}${prefixNum}_`,
-		oldPrefixLen = INTERNAL_VAR_NAMES_PREFIX.length + 1;
-	for (const varNode of state.internalVarNodes) {
-		varNode.name = `${prefix}${varNode.name.slice(oldPrefixLen)}`;
-	}
-}

lib/instrument/modify.js

@@ -18,9 +18,7 @@ const Program = require('./visitors/program.js'),
 		createNewTargetBinding
 	} = require('./blocks.js'),
 	{insertBlockVarsIntoBlockStatement} = require('./tracking.js'),
-	{
-		createTrackerVarNode, createGetScopeIdVarNode, createFnInfoVarNode, renameInternalVars
-	} = require('./internalVars.js'),
+	{createTrackerVarNode, createGetScopeIdVarNode, createFnInfoVarNode} = require('./internalVars.js'),
 	{visitKey} = require('./visit.js'),
 	{hasUseStrictDirective, stringLiteralWithSingleQuotes} = require('./utils.js'),
 	{getProp} = require('../shared/functions.js');
@@ -43,6 +41,7 @@ module.exports = modifyAst;
  *   - A queue is created of actions to take in 2nd pass.
  *     Actions are added to queue when exiting nodes (rather than when entering them),
  *     so that in 2nd pass deeper nodes are processed first.
+ *   - Identify any variables in code which would clash with Livepack's internal vars
  * 2nd pass:
  *   - Bindings are created for sloppy-mode function declarations which are hoisted
  *     (where they are bound can only be determined once all other bindings are known).
@@ -78,7 +77,6 @@ function modifyAst(ast, filename, isCommonJs, isStrict, sources, evalState) {
 		fileNode: ast,
 		trail: [],
 		sloppyFunctionDeclarations: [],
-		internalVarNodes: [],
 		internalVarsPrefixNum: 0,
 		trackerVarNode: undefined,
 		getScopeIdVarNode: undefined,
@@ -196,8 +194,6 @@ function modifySecondPass(ast, secondPassQueue, isEvalCode, sources, state) {
 	}
 
 	if (!isEvalCode) insertImportStatement(programNode, state);
-
-	renameInternalVars(state);
 }
 
 /**

lib/instrument/visitors/class.js

@@ -272,10 +272,6 @@ function visitClass(classNode, parent, key, className, state) {
 		innerNameBlock.bindings[className] = nameBlock.bindings[className];
 	}
 
-	// If class has super class but no existing constructor, activate super block.
-	// Constructor will be added in 2nd pass, and will contain `super()`.
-	if (hasSuperClass && !constructorNode) recordSuper(superBlock, fn, state);
-
 	// Exit class body
 	state.currentBlock = parentBlock;
 	externalTrail.length -= 2;
@@ -434,7 +430,7 @@ function instrumentClass(
 ) {
 	// If class has no constructor, create one for tracker code to be inserted in
 	if (!constructorNode) {
-		constructorNode = createClassConstructor(fn, state);
+		constructorNode = createClassConstructor(fn, superBlock, state);
 		classNode.body.body.push(constructorNode);
 	}
 
@@ -461,14 +457,18 @@ function instrumentClass(
  * Create empty class constructor node.
  * It will be instrumented by `instrumentFunctionOrClassConstructor()`, same as a real constructor.
  * @param {Object} fn - Function object for class
+ * @param {Object} superBlock - `super` block object
  * @param {Object} state - State object
  * @returns {Object} - Class constructor AST node
  */
-function createClassConstructor(fn, state) {
+function createClassConstructor(fn, superBlock, state) {
 	// If extends a super class: `constructor(...livepack_temp_4) { super(...livepack_temp_4); }`
 	// Otherwise: `constructor() {}`
 	let paramNodes, bodyNode;
 	if (fn.hasSuperClass) {
+		// Activate super block because created constructor contains `super()`
+		recordSuper(superBlock, fn, state);
+
 		const argsVarNode = createTempVarNode(state);
 		paramNodes = [t.restElement(argsVarNode)];
 		bodyNode = t.blockStatement([

lib/instrument/visitors/eval.js

@@ -35,14 +35,12 @@ function visitEval(node, parent, key, state) {
 	// Flag that `eval()` used in this function and all functions above
 	let isEvalCall = key === 'callee' && parent.type === 'CallExpression';
 	const fn = state.currentFunction;
-	let canUseSuper = false;
 	if (isEvalCall && parent.arguments.length > 0) {
 		// If called with spread element as first arg (`eval(...x)`), acts as indirect `eval`
 		if (parent.arguments[0].type === 'SpreadElement') {
 			isEvalCall = false;
 		} else {
 			flagAllAncestorFunctions(fn, 'containsEval');
-			canUseSuper = activateSuperIfIsUsable(fn, state);
 		}
 	}
 
@@ -51,8 +49,9 @@ function visitEval(node, parent, key, state) {
 
 	// Queue instrumentation on 2nd pass
 	state.secondPass(
-		instrumentEval, node, isEvalCall, parent, key, state.currentBlock, fn,
-		state.isStrict, canUseSuper, state
+		instrumentEval, node, isEvalCall, parent, key,
+		state.currentBlock, state.currentSuperBlock, state.currentThisBlock,
+		fn, state.isStrict, state.currentSuperIsProto, state
 	);
 }
 
@@ -63,19 +62,23 @@ function visitEval(node, parent, key, state) {
  * @param {Object|Array<Object>} parent - Parent AST node/container
  * @param {string|number} key - Node's key on parent AST node/container
  * @param {Object} block - Block object for block `eval` is in
+ * @param {Object} [superBlock] - `super` block object (if `super` block exists)
+ * @param {Object} [thisBlock] - `this` block object (if `this` block exists)
  * @param {Object} [fn] - Function object for function `eval` is in (`undefined` if not in a function)
  * @param {boolean} isStrict - `true` if is strict mode
- * @param {boolean} canUseSuper - `true` if `super()` can be used in `eval()`
+ * @param {boolean} superIsProto - `true` if `super` is in a prototype context (class proto method)
  * @param {Object} state - State object
  * @returns {undefined}
  */
-function instrumentEval(node, isEvalCall, parent, key, block, fn, isStrict, canUseSuper, state) {
+function instrumentEval(
+	node, isEvalCall, parent, key, block, superBlock, thisBlock, fn, isStrict, superIsProto, state
+) {
 	// Check `eval` is global
 	if (!isGlobalEval(block)) return;
 
 	// Handle either `eval()` call or `eval` identifier
 	if (isEvalCall) {
-		instrumentEvalCall(parent, block, fn, isStrict, canUseSuper, state);
+		instrumentEvalCall(parent, block, superBlock, thisBlock, fn, isStrict, superIsProto, state);
 	} else {
 		instrumentEvalIdentifier(node, parent, key, state);
 	}
@@ -90,17 +93,22 @@ function instrumentEval(node, isEvalCall, parent, key, block, fn, isStrict, canU
  *
  * @param {Object} callNode - Call expression AST node
  * @param {Object} block - Block object for block `eval` is in
+ * @param {Object} [superBlock] - `super` block object (if `super` block exists)
+ * @param {Object} [thisBlock] - `this` block object (if `this` block exists)
  * @param {Object} [fn] - Function object for function `eval` is in (`undefined` if not in a function)
  * @param {boolean} isStrict - `true` if is strict mode
- * @param {boolean} canUseSuper - `true` if `super()` can be used in `eval()`
+ * @param {boolean} superIsProto - `true` if `super` is in a prototype context (class proto method)
  * @param {Object} state - State object
  * @returns {undefined}
  */
-function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) {
+function instrumentEvalCall(callNode, block, superBlock, thisBlock, fn, isStrict, superIsProto, state) {
 	// If no arguments, leave as is
 	const argNodes = callNode.arguments;
 	if (argNodes.length === 0) return;
 
+	// If `super` is accessible to `eval`, activate `super` block
+	const canUseSuper = activateSuperIfIsUsable(fn, superBlock, thisBlock, superIsProto, state);
+
 	// Capture all vars in scopes above
 	const scopeNodes = [],
 		functionId = fn ? fn.id : undefined,
@@ -223,11 +231,13 @@ function isGlobalEval(block) {
  * If `super` is usable in this `eval()`, activate `super` block,
  * create an internal / external var for `this`, and set `superIsProto` on function.
  * @param {Object} [fn] - Function object
+ * @param {Object} [superBlock] - `super` block object (if `super` block exists)
+ * @param {Object} [thisBlock] - `this` block object (if `this` block exists)
+ * @param {boolean} superIsProto - `true` if `super` is in a prototype context (class proto method)
  * @param {Object} state - State object
  * @returns {boolean} - `true` if `super` is usable
  */
-function activateSuperIfIsUsable(fn, state) {
-	const superBlock = state.currentSuperBlock;
+function activateSuperIfIsUsable(fn, superBlock, thisBlock, superIsProto, state) {
 	if (!superBlock) return false;
 
 	// If no function, this code must be within `eval()` and `super` is outside `eval()`, so it is usable
@@ -240,7 +250,7 @@ function activateSuperIfIsUsable(fn, state) {
 				const fullFnType = fullFunction.node.type;
 				if (fullFnType === 'FunctionDeclaration' || fullFnType === 'FunctionExpression') return false;
 			}
-			createExternalVarForThis(fn, state);
+			createExternalVarForThis(fn, thisBlock, state);
 		} else if (fnType === 'ClassDeclaration' || fnType === 'ClassExpression') {
 			const {trail} = state;
 			if (trail.length > 6) {
@@ -252,7 +262,7 @@ function activateSuperIfIsUsable(fn, state) {
 			}
 		}
 
-		setSuperIsProtoOnFunctions(superBlock, fn, state);
+		if (superIsProto) setSuperIsProtoOnFunctions(superBlock, fn);
 	}
 
 	// TODO Also need to pass `superIsProto` and `superVarNode` to `evalDirect()`