Skip to content

Commit

Permalink
Remove binding.isFunction
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Nov 22, 2023
1 parent aa8de4f commit 0c877c5
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 32 deletions.
5 changes: 0 additions & 5 deletions lib/instrument/blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ function createBlockId(state) {
* @param {boolean} [props.isSilentConst=false] - `true` if is function expression name,
* assignment to which fails silently in sloppy mode
* @param {boolean} [props.isVar=false] - `true` if defined with `var` statement or function declaration
* @param {boolean} [props.isFunction=false] - `true` if is function or class name
* (relevant as these canot be renamed)
* @param {boolean} [props.isFrozenName=false] - `true` if is var which is not renamed when used
* inside function in which it's bound
* @param {Array<string>} [props.argNames] - Linked argument names (only relevant for `arguments`)
Expand All @@ -125,8 +123,6 @@ function createBinding(block, varName, props, state) {
* @param {boolean} [props.isSilentConst=false] - `true` if is function expression name,
* assignment to which fails silently in sloppy mode
* @param {boolean} [props.isVar=false] - `true` if defined with `var` statement or function declaration
* @param {boolean} [props.isFunction=false] - `true` if is function or class name
* (relevant as these canot be renamed)
* @param {boolean} [props.isFrozenName=false] - `true` if is var which is not renamed when used
* inside function in which it's bound
* @param {Array<string>} [props.argNames] - Linked argument names (only relevant for `arguments`)
Expand All @@ -139,7 +135,6 @@ function createBindingWithoutNameCheck(block, varName, props) {
isConst: !!props.isConst,
isSilentConst: !!props.isSilentConst,
isVar: !!props.isVar,
isFunction: !!props.isFunction,
isFrozenName: !!props.isFrozenName,
argNames: props.argNames,
trails: [] // Trails of usages within same function as binding, where var not function or frozen
Expand Down
6 changes: 1 addition & 5 deletions lib/instrument/visitors/assignee.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,7 @@ function IdentifierVar(node, state) {
}

if (fn) fn.bindings.push(binding);
} else if (binding.isFunction) {
// NB: No need to check `binding.isFrozenName` because it's only set in 2nd pass
// so will always be `false` at this point.
// Exception would be if it was a shared binding with `arguments`, but this doesn't work yet.
// TODO: Add `|| binding.isFrozenName` to `if` condition when that problem is fixed.
} else if (binding.isFrozenName) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/visitors/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function ClassDeclaration(node, state, parent, key) {
assertNoCommonJsVarsClash(block, className, state);
// No need to check for internal var name clash here as will be checked when creating
// the binding for class name accessed from inside class
createBindingWithoutNameCheck(block, className, {isFunction: true});
createBindingWithoutNameCheck(block, className, {isFrozenName: true});

// Visit class
visitClass(node, parent, key, className, state);
Expand Down
28 changes: 11 additions & 17 deletions lib/instrument/visitors/function.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function FunctionDeclaration(node, state, parent, key) {
let binding = block.bindings[fnName];
if (!binding) {
const isTopLevel = block === hoistBlock;
binding = createBinding(block, fnName, {isVar: isTopLevel, isFunction: true}, state);
binding = createBinding(block, fnName, {isVar: isTopLevel, isFrozenName: true}, state);

// If is a sloppy-mode function declaration which may need to be hoisted, record that.
// Determine whether to hoist after 1st pass is complete, once all other bindings created.
Expand All @@ -99,10 +99,10 @@ function FunctionDeclaration(node, state, parent, key) {
parentFn: state.currentFunction
});
}
} else if (!binding.isFunction) {
} else if (!binding.isFrozenName) {
// Existing binding is a `var` declaration.
// Flag binding as a function so `var` declarations' identifiers don't get renamed.
binding.isFunction = true;
// Flag binding as frozen so `var` declarations' identifiers don't get renamed.
binding.isFrozenName = true;
binding.trails.length = 0;
}

Expand Down Expand Up @@ -153,7 +153,7 @@ function FunctionExpression(node, state, parent, key) {
*/
function createAndEnterFunctionOrClassNameBlock(fnName, isSilentConst, state) {
const block = createAndEnterBlock(fnName, false, state);
createBinding(block, fnName, {isConst: true, isSilentConst, isFunction: true}, state);
createBinding(block, fnName, {isConst: true, isSilentConst, isFrozenName: true}, state);
return block;
}

Expand Down Expand Up @@ -576,10 +576,10 @@ function hoistSloppyFunctionDeclaration(declaration) {
// Can be hoisted
if (!hoistBlockBinding) {
hoistBlock.bindings[varName] = {...declaration.binding, isVar: true};
} else if (!hoistBlockBinding.isFunction) {
} else if (!hoistBlockBinding.isFrozenName) {
// Existing binding is a `var` declaration.
// Flag binding as a function so `var` declarations' identifiers don't get renamed.
hoistBlockBinding.isFunction = true;
// Flag binding as frozen so `var` declarations' identifiers don't get renamed.
hoistBlockBinding.isFrozenName = true;
hoistBlockBinding.trails.length = 0;
}
}
Expand Down Expand Up @@ -726,12 +726,7 @@ function createFunctionInfoFunction(fn, state) {
// Compile internal vars and reserved var names
const internalVars = Object.create(null),
reservedVarNames = new Set();
for (const {name: varName, trails, isFunction, isFrozenName} of fn.bindings) {
// Function name bindings are not usually added to `fn.bindings`,
// but can be included if binding is initially a `var` statement e.g. `var x; function x() {}`.
// So skip them here. `parseFunction` adds them to list of reserved var names.
if (isFunction) continue;

for (const {name: varName, trails, isFrozenName} of fn.bindings) {
if (isFrozenName && varName !== 'this') {
if (!internalVars[varName]) reservedVarNames.add(varName);
} else {
Expand Down Expand Up @@ -762,13 +757,12 @@ function createFunctionInfoFunction(fn, state) {
blockId: block.id,
blockName: block.name,
vars: mapValues(vars, (varProps, varName) => {
const {binding} = varProps;
if (varName === 'arguments') argNames = binding.argNames;
if (varName === 'arguments') argNames = varProps.binding.argNames;
return {
isReadFrom: varProps.isReadFrom || undefined,
isAssignedTo: varProps.isAssignedTo || undefined,
isFrozenName: varProps.isFrozenName || undefined,
isFrozenInternalName: binding.isFrozenName || binding.isFunction || undefined,
isFrozenInternalName: varProps.binding.isFrozenName || undefined,
trails: varProps.trails
};
})
Expand Down
7 changes: 3 additions & 4 deletions lib/instrument/visitors/identifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign

// Record if internal var
if (block.id >= fn.id) {
if (!binding.isFrozenName && !binding.isFunction) binding.trails.push(trail);
if (!binding.isFrozenName) binding.trails.push(trail);
return;
}

Expand All @@ -179,15 +179,14 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign
// it will need to be added to `internalVars`.
// This is not the case if var is being read from, as an external var is created below,
// which will be converted to an internal var.
// It's also not needed if binding is a function name or frozen var, as they're not renamed
// and so aren't treated as internal vars.
// It's also not needed if binding is frozen, as they're not treated as internal vars.
// Not possible to know at this point whether binding is frozen or not (an `eval()` might be later
// in the function), so store the binding, and type will be changed to `CONST_VIOLATION_NEEDS_NO_VAR`
// later on if the binding is frozen.
fn.amendments.push({
type: binding.isSilentConst && !isStrict
? CONST_VIOLATION_SILENT
: isReadFrom || binding.isFrozenName || binding.isFunction
: isReadFrom || binding.isFrozenName
? CONST_VIOLATION_NEEDS_NO_VAR
: CONST_VIOLATION_NEEDS_VAR,
blockId: block.id,
Expand Down

0 comments on commit 0c877c5

Please sign in to comment.