Skip to content

Commit bef798c

Browse files
fix(javascript): keep the arguments.length test before reading an index
An index at or past `arguments.length` reads `Object.prototype`, where a getter may sit, so dropping Babel's `arguments.length > n` test before `arguments[n]` could run it. oxc and terser both keep the test.
1 parent c2c86ad commit bef798c

2 files changed

Lines changed: 2 additions & 134 deletions

File tree

‎lib/javascript/syntax-printer.js‎

Lines changed: 0 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -22885,133 +22885,6 @@ const installImprove = (modules) => {
2288522885
}
2288622886
);
2288722887

22888-
/** @type {WeakMap<Node, boolean>} */
22889-
const argumentsReadOnly = new WeakMap();
22890-
22891-
/**
22892-
* Whether a function only reads its `arguments` object's `length` and
22893-
* indices, so an index at or past `length` reads `undefined`.
22894-
* @param {Node} lambda a function
22895-
* @returns {boolean} true when nothing writes or passes the object on
22896-
*/
22897-
const readsArgumentsOnly = (lambda) => {
22898-
const known = argumentsReadOnly.get(lambda);
22899-
if (known !== undefined) return known;
22900-
let readOnly = true;
22901-
const walker = A.createWalker((/** @type {Node} */ node) => {
22902-
if (!readOnly) return true;
22903-
if (!A.isSymbolRefNode(node) || node.name !== "arguments") return;
22904-
const access = walker.parent();
22905-
const outer = walker.parent(1);
22906-
if (
22907-
!(
22908-
(A.isDotNode(access) && access.property.name === "length") ||
22909-
(A.isSubNode(access) && A.isNumberNode(access.property))
22910-
) ||
22911-
access.object !== node ||
22912-
(A.isAssignNode(outer) && outer.left === access) ||
22913-
(A.isUnaryNode(outer) &&
22914-
(outer.operator === "++" ||
22915-
outer.operator === "--" ||
22916-
outer.operator === "delete"))
22917-
) {
22918-
readOnly = false;
22919-
}
22920-
});
22921-
walkNode(lambda, walker);
22922-
argumentsReadOnly.set(lambda, readOnly);
22923-
return readOnly;
22924-
};
22925-
22926-
/**
22927-
* The index `arguments.length > n` tests for, where the function's own
22928-
* `arguments` object is only ever read.
22929-
* @param {Node} node a test
22930-
* @returns {number | undefined} the index n, or undefined
22931-
*/
22932-
const testedArgumentIndex = (node) => {
22933-
if (!A.isBinaryNode(node) || A.isAssignNode(node)) return;
22934-
let length;
22935-
let count;
22936-
if (node.operator === ">") [length, count] = [node.left, node.right];
22937-
else if (node.operator === "<") [count, length] = [node.left, node.right];
22938-
else return;
22939-
if (
22940-
!A.isDotNode(length) ||
22941-
length.property.name !== "length" ||
22942-
!A.isArgumentsObject(length.object) ||
22943-
!A.isNumberNode(count) ||
22944-
!readsArgumentsOnly(length.object.definition.scope.node)
22945-
) {
22946-
return;
22947-
}
22948-
return count.value;
22949-
};
22950-
22951-
/**
22952-
* @param {Node} node an expression
22953-
* @returns {boolean} whether it is `undefined` or `void` of a constant
22954-
*/
22955-
const isUndefinedValue = (node) =>
22956-
A.isUndefinedNode(node) ||
22957-
(A.isUnaryPrefixNode(node) &&
22958-
node.operator === "void" &&
22959-
A.isConstantNode(node.argument));
22960-
22961-
/**
22962-
* Whether a node reads `arguments[n]` off the same object a test read.
22963-
* @param {Node} node an expression
22964-
* @param {number} index the index n
22965-
* @returns {boolean} true for that read
22966-
*/
22967-
const readsArgument = (node, index) =>
22968-
A.isSubNode(node) &&
22969-
A.isArgumentsObject(node.object) &&
22970-
A.isNumberNode(node.property) &&
22971-
node.property.value === index;
22972-
22973-
// Babel's default parameters test `arguments.length > n` before reading
22974-
// `arguments[n]`, which reads `undefined` past the length anyway.
22975-
A.wrapOptimizer("Binary", (optimizeBinary) => (self, compressor) => {
22976-
const optimized = optimizeBinary(self, compressor);
22977-
if (
22978-
!improvements.enabled ||
22979-
!A.isBinaryNode(optimized) ||
22980-
optimized.operator !== "&&"
22981-
) {
22982-
return optimized;
22983-
}
22984-
const index = testedArgumentIndex(optimized.left);
22985-
const { right } = optimized;
22986-
if (
22987-
index === undefined ||
22988-
!A.isBinaryNode(right) ||
22989-
right.operator !== "!==" ||
22990-
!(
22991-
(isUndefinedValue(right.left) && readsArgument(right.right, index)) ||
22992-
(isUndefinedValue(right.right) && readsArgument(right.left, index))
22993-
)
22994-
) {
22995-
return optimized;
22996-
}
22997-
return right;
22998-
});
22999-
A.wrapOptimizer(
23000-
"Conditional",
23001-
(optimizeConditional) => (self, compressor) => {
23002-
const optimized = optimizeConditional(self, compressor);
23003-
if (!improvements.enabled || !A.isConditionalNode(optimized)) {
23004-
return optimized;
23005-
}
23006-
const index = testedArgumentIndex(optimized.test);
23007-
return index !== undefined &&
23008-
readsArgument(optimized.consequent, index) &&
23009-
isUndefinedValue(optimized.alternate)
23010-
? optimized.consequent
23011-
: optimized;
23012-
}
23013-
);
23014-
2301522888
/**
2301622889
* Whether the value of the node visited goes on as it is, where no test or
2301722890
* operator around it could fold a constant written in its place.

‎test/unitCases/syntax-printer.unittest.js‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,6 @@ const IMPROVED_CASES = [
329329
{ compress: { ecma: 2020 }, ecma: 2020, mangle: false },
330330
"function f(y,z){return[+y,y>>>z]}console.log(f(\"3\",1));"
331331
],
332-
[
333-
"Babel's default parameters, testing `arguments.length` before reading the index",
334-
"function f() { var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 1; var b = arguments.length > 1 ? arguments[1] : void 0; return [a, b]; } console.log(f(), f(5), f(5, 6));",
335-
{ compress: {}, mangle: false },
336-
"function f(){return[void 0!==arguments[0]?arguments[0]:1,arguments[1]]}console.log(f(),f(5),f(5,6));"
337-
],
338332
[
339333
"unread calls of an async function with no body and of a generator",
340334
"var log = []; function f() { (async () => {})(); (function* () { log.push(1); })(); log.push(2); } f(); console.log(log.join());",
@@ -8503,9 +8497,10 @@ describe("syntax-printer", () => {
85038497
}
85048498
});
85058499

8506-
it("should keep `arguments.length` tested where something else uses the object", async () => {
8500+
it("should keep `arguments.length` tested, as an index past it may read a getter `Object.prototype` holds", async () => {
85078501
const { minify } = await load();
85088502
for (const input of [
8503+
"export function f(){var a=arguments.length>0&&arguments[0]!==undefined?arguments[0]:1;var b=arguments.length>1?arguments[1]:void 0;return[a,b]}",
85098504
"export function f(){arguments[1]=5;return arguments.length>1?arguments[1]:void 0}",
85108505
"export function f(){g(arguments);return arguments.length>1&&void 0!==arguments[1]?arguments[1]:2}",
85118506
"export function f(){return arguments.length>2&&void 0!==arguments[1]?arguments[1]:2}"

0 commit comments

Comments
 (0)