Skip to content

Commit

Permalink
Instrument: Prevent side-effects serializing classes with properties …
Browse files Browse the repository at this point in the history
…[fix]

Fixes #549.
  • Loading branch information
overlookmotel committed Dec 4, 2023
1 parent 0dc498d commit 9f788a4
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 20 deletions.
23 changes: 16 additions & 7 deletions lib/instrument/tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function insertBlockVarsIntoBlockStatement(block, blockNode, state) {
* @param {Object} paramsBlock - Function params block object
* @param {Object} [bodyBlock] - Function body block object
* (`undefined` if arrow function with no body block)
* @param {Object} trackerNode - AST node for tracker call
* @param {Object} [trackerNode] - AST node for tracker call (or undefined if no tracker to insert)
* @param {Object} state - State object
* @returns {undefined}
*/
Expand All @@ -78,20 +78,24 @@ function insertTrackerCodeIntoFunction(fn, fnNode, paramsBlock, bodyBlock, track
* @param {Object} paramsBlock - Function params block object
* @param {Object} [bodyBlock] - Function body block object
* (`undefined` if arrow function with no body block)
* @param {Object} trackerNode - AST node for tracker call
* @param {Object} [trackerNode] - AST node for tracker call (or undefined if no tracker to insert)
* @param {Object} state - State object
* @returns {undefined}
*/
function insertTrackerCodeIntoFunctionBody(fnNode, paramsBlock, bodyBlock, trackerNode, state) {
// Exit if no tracker, scope ID var, or temp vars to insert
const block = bodyBlock || paramsBlock;
if (!trackerNode && !block.scopeIdVarNode) return;

// If body is not a block statement, convert it to one
let bodyNode = fnNode.body;
if (!bodyBlock) bodyNode = fnNode.body = t.blockStatement([t.returnStatement(bodyNode)]);

// Insert scope ID and temp var nodes
insertBlockVarsIntoBlockStatement(bodyBlock || paramsBlock, bodyNode, state);
insertBlockVarsIntoBlockStatement(block, bodyNode, state);

// Insert tracker call
bodyNode.body.unshift(t.expressionStatement(trackerNode));
if (trackerNode) bodyNode.body.unshift(t.expressionStatement(trackerNode));
}

/**
Expand Down Expand Up @@ -143,22 +147,28 @@ function insertTrackerCodeIntoFunctionBody(fnNode, paramsBlock, bodyBlock, track
* @param {Object} paramsBlock - Function params block object
* @param {Object} [bodyBlock] - Function body block object
* (`undefined` if arrow function with no body block)
* @param {Object} trackerNode - AST node for tracker call
* @param {Object} [trackerNode] - AST node for tracker call (or undefined if no tracker to insert)
* @param {number} firstComplexParamIndex - Index of first param which is a complex param
* @param {Object} state - State object
* @returns {undefined}
*/
function insertTrackerCodeIntoFunctionParams(
fnNode, paramsBlock, bodyBlock, trackerNode, firstComplexParamIndex, state
) {
// Exit if no tracker, scope ID var, or temp vars to insert
const {scopeIdVarNode} = paramsBlock;
if (!trackerNode && !scopeIdVarNode) return;

// Create object pattern to use as rest element.
// `{ [ livepack_tracker(...) ]: [] = [] }`
// Assignments will be added to each side of `[] = []` to init vars and assign to them.
// If no tracker to insert (class constructor where class has prototype properties),
// create `livepack_tracker()` call with no arguments.
const leftNodes = [],
rightNodes = [],
propNodes = [
t.objectProperty(
trackerNode,
trackerNode || t.callExpression(state.trackerVarNode, []),
t.assignmentPattern(t.arrayPattern(leftNodes), t.arrayExpression(rightNodes)),
true
)
Expand All @@ -171,7 +181,6 @@ function insertTrackerCodeIntoFunctionParams(
}

// Insert assignments for scope ID var node and temp var nodes
const {scopeIdVarNode} = paramsBlock;
if (scopeIdVarNode) {
addAssignment(scopeIdVarNode, t.callExpression(state.getScopeIdVarNode, []));

Expand Down
41 changes: 28 additions & 13 deletions lib/instrument/visitors/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function visitClass(classNode, parent, key, className, state) {
constructorParamsBlock.varsBlock = constructorBodyBlock;

// Visit constructor and prototype properties
let protoThisBlock;
let protoThisBlock, propForTrackerNode;
if (constructorNode || protoPropertyIndexes.length !== 0) {
// Enter function
state.currentFunction = fn;
Expand Down Expand Up @@ -210,6 +210,10 @@ function visitClass(classNode, parent, key, className, state) {
for (const index of protoPropertyIndexes) {
visitKey(memberNodes, index, ClassPropertyMaybePrivate, state);
}

// If class does not extend a super class, prototype properties will be evaluated
// before class constructor, so tracker needs to go in 1st proto property
if (!classNode.superClass) propForTrackerNode = memberNodes[protoPropertyIndexes[0]];
}

// Exit function
Expand Down Expand Up @@ -286,8 +290,8 @@ function visitClass(classNode, parent, key, className, state) {
// Queue instrumentation of class in 2nd pass
state.secondPass(
instrumentClass,
classNode, fn, parent, key, constructorNode, constructorParamsBlock, constructorBodyBlock,
superBlock, state
classNode, fn, parent, key, constructorNode, propForTrackerNode,
constructorParamsBlock, constructorBodyBlock, superBlock, state
);
}

Expand Down Expand Up @@ -417,22 +421,39 @@ function serializeClassAst(classNode, constructorNode, constructorIndex) {
* @param {Object|Array} parent - Parent AST node/container
* @param {string|number} key - Node's key on parent AST node/container
* @param {Object} [constructorNode] - Class constructor AST node (or `undefined` if no constructor)
* @param {Object} [propForTrackerNode] - Property node to insert tracker in
* @param {Object} constructorParamsBlock - Constructor params block object
* @param {Object} constructorBodyBlock - Constructor body block object
* @param {Object} superBlock - `super` target block object
* @param {Object} state - State object
* @returns {undefined}
*/
function instrumentClass(
classNode, fn, parent, key, constructorNode, constructorParamsBlock, constructorBodyBlock,
superBlock, state
classNode, fn, parent, key, constructorNode, propForTrackerNode,
constructorParamsBlock, constructorBodyBlock, superBlock, state
) {
// If class has no constructor, create one for tracker code to be inserted in
if (!constructorNode) {
// Create tracker
let trackerNode = createTrackerCall(fn, state);

if (propForTrackerNode) {
// Insert tracker into prototype property
propForTrackerNode.value = propForTrackerNode.value
? t.sequenceExpression([trackerNode, propForTrackerNode.value])
: t.unaryExpression('void', trackerNode);
trackerNode = undefined;
} else if (!constructorNode) {
// Class has no constructor - create one for tracker code to be inserted in
constructorNode = createClassConstructor(fn, state);
classNode.body.body.push(constructorNode);
}

// Add tracker code + block vars to constructor
if (constructorNode) {
insertTrackerCodeIntoFunction(
fn, constructorNode, constructorParamsBlock, constructorBodyBlock, trackerNode, state
);
}

// Insert `static {}` block to define temp var for `super` target if `super` used in class
const superVarNode = getSuperVarNode(superBlock);
if (superVarNode) {
Expand All @@ -447,12 +468,6 @@ function instrumentClass(
// Restore to original place in AST
parent[key] = classNode;

// Add tracker code + block vars to constructor
const trackerNode = createTrackerCall(fn, state);
insertTrackerCodeIntoFunction(
fn, constructorNode, constructorParamsBlock, constructorBodyBlock, trackerNode, state
);

// Insert tracking comment
const commentHolderNode = classNode.id || classNode.superClass || classNode.body;
insertTrackerComment(fn.id, FN_TYPE_CLASS, commentHolderNode, 'leading', state);
Expand Down

0 comments on commit 9f788a4

Please sign in to comment.