Skip to content

Commit

Permalink
Do not include unused return expressions (#4573)
Browse files Browse the repository at this point in the history
* Add test

* Do not include unused return expressions

* Improve coverage
  • Loading branch information
lukastaegert committed Jul 15, 2022
1 parent f019fae commit 01e2461
Show file tree
Hide file tree
Showing 15 changed files with 233 additions and 95 deletions.
4 changes: 0 additions & 4 deletions src/ast/nodes/CallExpression.ts
Expand Up @@ -99,10 +99,6 @@ export default class CallExpression extends CallExpressionBase implements Deopti
this.callee.include(context, false);
}
this.callee.includeCallArguments(context, this.arguments);
const returnExpression = this.getReturnExpression();
if (!returnExpression.included) {
returnExpression.include(context, false);
}
}

render(
Expand Down
13 changes: 1 addition & 12 deletions src/ast/nodes/shared/MultiExpression.ts
@@ -1,9 +1,8 @@
import type { DeoptimizableEntity } from '../../DeoptimizableEntity';
import type { HasEffectsContext, InclusionContext } from '../../ExecutionContext';
import type { HasEffectsContext } from '../../ExecutionContext';
import { NodeInteraction, NodeInteractionCalled } from '../../NodeInteractions';
import type { ObjectPath, PathTracker } from '../../utils/PathTracker';
import { ExpressionEntity } from './Expression';
import type { IncludeChildren } from './Node';

export class MultiExpression extends ExpressionEntity {
included = false;
Expand Down Expand Up @@ -41,14 +40,4 @@ export class MultiExpression extends ExpressionEntity {
}
return false;
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
// This is only relevant to include values that do not have an AST representation,
// such as UnknownArrayExpression. Thus we only need to include them once.
for (const expression of this.expressions) {
if (!expression.included) {
expression.include(context, includeChildrenRecursively);
}
}
}
}
24 changes: 17 additions & 7 deletions test/form/samples/builtin-prototypes/literal/_expected.js
@@ -1,15 +1,25 @@
// retained
true.valueOf().unknown.unknown();
true.valueOf()();
(1).valueOf().unknown.unknown();
(1).valueOf().unknown();
(1).valueOf()[globalThis.unknown]();
true.valueOf()[globalThis.unknown]();
true.valueOf().unknown();
true.valueOf().unknown.unknown();
true.valueOf().unknown.unknown().unknown;

(1).valueOf()();
'ab'.charAt(1).unknown.unknown();
(1).valueOf()[globalThis.unknown]();
(1).valueOf().unknown();
(1).valueOf().unknown.unknown();
(1).valueOf().unknown.unknown().unknown;

'ab'.charAt(1)();
'ab'.charAt(1)[globalThis.unknown]();
'ab'.charAt(1).unknown();
'ab'.charAt(1).unknown.unknown();
'ab'.charAt(1).unknown.unknown().unknown;

null.unknown;
'ab'.replace( 'a', () => console.log( 1 ) || 'b' );
'ab'.replaceAll( 'a', () => console.log( 1 ) || 'b' );
'ab'.replace('a', () => console.log(1) || 'b');
'ab'.replaceAll('a', () => console.log(1) || 'b');

// deep property access is forbidden
true.x.y;
Expand Down
136 changes: 79 additions & 57 deletions test/form/samples/builtin-prototypes/literal/main.js
@@ -1,67 +1,90 @@
const boolean = true;
const valueOf1 = boolean.valueOf();
const valueOf2 = true.valueOf();
const valueOf3 = true.valueOf().valueOf();
const valueOf4 = true.valueOf().valueOf().valueOf();

// retained
true.valueOf().unknown.unknown();
true.valueOf()();
(1).valueOf().unknown.unknown();
(1).valueOf().unknown();
(1).valueOf()[globalThis.unknown]();
true.valueOf()[globalThis.unknown]();
true.valueOf().unknown();
true.valueOf().unknown.unknown();
true.valueOf().unknown.unknown().unknown;

(1).valueOf()();
'ab'.charAt(1).unknown.unknown();
(1).valueOf()[globalThis.unknown]();
(1).valueOf().unknown();
(1).valueOf().unknown.unknown();
(1).valueOf().unknown.unknown().unknown;

'ab'.charAt(1)();
'ab'.charAt(1)[globalThis.unknown]();
'ab'.charAt(1).unknown();
'ab'.charAt(1).unknown.unknown();
'ab'.charAt(1).unknown.unknown().unknown;

null.unknown;

// number prototype
const _toExponential = (1).toExponential( 2 ).trim();
const _toFixed = (1).toFixed( 2 ).trim();
const _toLocaleString = (1).toLocaleString().trim();
const _toPrecision = (1).toPrecision( 2 ).trim();
const _numberValueOf = (1).valueOf().toExponential( 2 );
// boolean prototype
true.valueOf();
true.valueOf().valueOf();
true.valueOf().valueOf().valueOf();
const _booleanToString = true.toString().trim();
const _booleanValueOf = true.valueOf().valueOf();

// inherited
const _numberHasOwnProperty = (1).hasOwnProperty( 'toString' ).valueOf();
const _numberIsPrototypeOf = (1).isPrototypeOf( 1 ).valueOf();
const _numberPropertyIsEnumerable = (1).propertyIsEnumerable( 'toString' ).valueOf();
const _booleanHasOwnProperty = true.hasOwnProperty('toString').valueOf();
const _booleanIsPrototypeOf = true.isPrototypeOf(true).valueOf();
const _booleanPropertyIsEnumerable = true.propertyIsEnumerable('toString').valueOf();
const _booleanToLocaleString = true.toLocaleString().trim();

// number prototype
(1).valueOf();
(1).valueOf().valueOf();
(1).valueOf().valueOf().valueOf();
const _numberToExponential = (1).toExponential(2).trim();
const _numberToFixed = (1).toFixed(2).trim();
const _numberToLocaleString = (1).toLocaleString().trim();
const _numberToPrecision = (1).toPrecision(2).trim();
const _numberToString = (1).toString().trim();
const _numberValueOf = (1).valueOf().toExponential(2);

// inherited
const _numberHasOwnProperty = (1).hasOwnProperty('toString').valueOf();
const _numberIsPrototypeOf = (1).isPrototypeOf(1).valueOf();
const _numberPropertyIsEnumerable = (1).propertyIsEnumerable('toString').valueOf();

// string prototype
const _at = 'ab'.at( 1 )
const _charAt = 'ab'.charAt( 1 ).trim();
const _charCodeAt = 'ab'.charCodeAt( 1 ).toExponential( 2 );
const _codePointAt = 'ab'.codePointAt( 1 );
const _concat = 'ab'.concat( 'c' ).trim();
const _includes = 'ab'.includes( 'a' ).valueOf();
const _endsWith = 'ab'.endsWith( 'a' ).valueOf();
const _indexOf = 'ab'.indexOf( 'a' ).toExponential( 2 );
const _lastIndexOf = 'ab'.lastIndexOf( 'a' ).toExponential( 2 );
const _localeCompare = 'ab'.localeCompare( 'a' ).toExponential( 2 );
const _match = 'ab'.match( /a/ )
const _matchAll = 'ab'.matchAll( /a/ )
const _normalize = 'ab'.normalize().trim();
const _padEnd = 'ab'.padEnd( 4, 'a' ).trim();
const _padStart = 'ab'.padStart( 4, 'a' ).trim();
const _repeat = 'ab'.repeat( 2 ).trim();
const _replace = 'ab'.replace( 'a', () => 'b' ).trim();
const _replaceEffect = 'ab'.replace( 'a', () => console.log( 1 ) || 'b' );
const _replaceAll = 'ab'.replaceAll( 'a', () => 'b' ).trim();
const _replaceAllEffect = 'ab'.replaceAll( 'a', () => console.log( 1 ) || 'b' );
const _search = 'ab'.search( /a/ ).toExponential( 2 );
const _slice = 'ab'.slice( 0, 1 ).trim();
const _split = 'ab'.split( 'a' );
const _startsWith = 'ab'.startsWith( 'a' ).valueOf();
const _substring = 'ab'.substring( 0, 1 ).trim();
const _toLocaleLowerCase = 'ab'.toLocaleLowerCase().trim();
const _toLocaleUpperCase = 'ab'.toLocaleUpperCase().trim();
const _toLowerCase = 'ab'.toLowerCase().trim();
const _toString = 'ab'.trim();
const _toUpperCase = 'ab'.toUpperCase().trim();
const _trim = 'ab'.trim().trim();
const _trimEnd = 'ab'.trimEnd().trim();
const _trimStart = 'ab'.trimStart().trim();
'ab'.valueOf();
'ab'.valueOf().valueOf();
'ab'.valueOf().valueOf().valueOf();
const _stringAt = 'ab'.at(1);
const _stringCharAt = 'ab'.charAt(1).trim();
const _stringCharCodeAt = 'ab'.charCodeAt(1).toExponential(2);
const _stringCodePointAt = 'ab'.codePointAt(1);
const _stringConcat = 'ab'.concat('c').trim();
const _stringEndsWith = 'ab'.endsWith('a').valueOf();
const _stringIncludes = 'ab'.includes('a').valueOf();
const _stringIndexOf = 'ab'.indexOf('a').toExponential(2);
const _stringLastIndexOf = 'ab'.lastIndexOf('a').toExponential(2);
const _stringLocaleCompare = 'ab'.localeCompare('a').toExponential(2);
const _stringMatch = 'ab'.match(/a/);
const _stringMatchAll = 'ab'.matchAll(/a/);
const _stringNormalize = 'ab'.normalize().trim();
const _stringPadEnd = 'ab'.padEnd(4, 'a').trim();
const _stringPadStart = 'ab'.padStart(4, 'a').trim();
const _stringRepeat = 'ab'.repeat(2).trim();
const _stringReplace = 'ab'.replace('a', () => 'b').trim();
const _stringReplaceEffect = 'ab'.replace('a', () => console.log(1) || 'b');
const _stringReplaceAll = 'ab'.replaceAll('a', () => 'b').trim();
const _stringReplaceAllEffect = 'ab'.replaceAll('a', () => console.log(1) || 'b');
const _stringSearch = 'ab'.search(/a/).toExponential(2);
const _stringSlice = 'ab'.slice(0, 1).trim();
const _stringSplit = 'ab'.split('a');
const _stringStartsWith = 'ab'.startsWith('a').valueOf();
const _stringSubstring = 'ab'.substring(0, 1).trim();
const _stringToLocaleLowerCase = 'ab'.toLocaleLowerCase().trim();
const _stringToLocaleUpperCase = 'ab'.toLocaleUpperCase().trim();
const _stringToLowerCase = 'ab'.toLowerCase().trim();
const _stringToString = 'ab'.trim();
const _stringToUpperCase = 'ab'.toUpperCase().trim();
const _stringTrim = 'ab'.trim().trim();
const _stringTrimEnd = 'ab'.trimEnd().trim();
const _stringTrimStart = 'ab'.trimStart().trim();
const _stringValueOf = 'ab'.valueOf().trim();

// DEPRECATED prototype methods
Expand All @@ -83,11 +106,10 @@ const _trimLeft = 'ab'.trimLeft().trim();
const _trimRight = 'ab'.trimRight().trim();

// inherited
const _stringHasOwnProperty = 'ab'.hasOwnProperty( 'toString' ).valueOf();
const _stringIsPrototypeOf = 'ab'.isPrototypeOf( '' ).valueOf();
const _stringPropertyIsEnumerable = 'ab'.propertyIsEnumerable( 'toString' ).valueOf();
const _stringHasOwnProperty = 'ab'.hasOwnProperty('toString').valueOf();
const _stringIsPrototypeOf = 'ab'.isPrototypeOf('').valueOf();
const _stringPropertyIsEnumerable = 'ab'.propertyIsEnumerable('toString').valueOf();
const _stringToLocaleString = 'ab'.toLocaleString().trim();
const _stringToString = 'ab'.toString().trim();

// property access is allowed
const accessBoolean = true.x;
Expand Down
@@ -1,3 +1,8 @@
// retained
`ab`();
`ab`.unknown.unknown();
`ab`.unknown.unknown().unknown;

// deep property access is forbidden
`ab`.x.y;

Expand Down
6 changes: 6 additions & 0 deletions test/form/samples/builtin-prototypes/template-literal/main.js
@@ -1,3 +1,9 @@
// retained
`ab`();
`ab`.unknown.unknown();
`ab`.unknown.unknown().unknown;

// removed
`ab`.trim();
`ab`.trim().trim();
`ab`.toString().trim();
Expand Down
5 changes: 5 additions & 0 deletions test/form/samples/return-after-error/_config.js
@@ -0,0 +1,5 @@
const path = require('path');

module.exports = {
description: 'tree-shakes entities referenced in a return statement after an error'
};
7 changes: 7 additions & 0 deletions test/form/samples/return-after-error/_expected.js
@@ -0,0 +1,7 @@
function getInstance() {
throw new Error('error');
}

console.log(getInstance());

export { getInstance };
8 changes: 8 additions & 0 deletions test/form/samples/return-after-error/main.js
@@ -0,0 +1,8 @@
class Removed {}

export function getInstance() {
throw new Error('error');
return new Removed();
}

console.log(getInstance());
22 changes: 22 additions & 0 deletions test/function/samples/class-prop-returns/_config.js
@@ -0,0 +1,22 @@
const assert = require('assert');

module.exports = {
description: 'does not remove calls to props without value',
minNodeVersion: 12,
exports({ callProp, callStaticProp }) {
let hasError = false;
try {
callStaticProp();
} catch {
hasError = true;
}
assert.ok(hasError);
hasError = false;
try {
callProp();
} catch {
hasError = true;
}
assert.ok(hasError);
}
};
11 changes: 11 additions & 0 deletions test/function/samples/class-prop-returns/main.js
@@ -0,0 +1,11 @@
class Foo {
static staticProp;
prop;
}

export const callStaticProp = () => Foo.staticProp() || false;

export const callProp = () => {
const foo = new Foo();
return foo.prop() || false;
};
11 changes: 10 additions & 1 deletion test/function/samples/conditionals-deoptimization/_config.js
Expand Up @@ -3,6 +3,15 @@ const assert = require('assert');
module.exports = {
description: 'handles deoptimization of conditionals',
exports(exports) {
assert.deepStrictEqual(exports, { first: true, second: true, third: true, fourth: true });
assert.deepStrictEqual(exports, {
cond1a: true,
cond1b: true,
cond2a: true,
cond2b: true,
log1a: true,
log1b: true,
log2a: true,
log2b: true
});
}
};
46 changes: 32 additions & 14 deletions test/function/samples/conditionals-deoptimization/main.js
@@ -1,21 +1,39 @@
export let first = false;
export let second = false;
export let third = false;
export let fourth = false;
export let cond1a = false;
export let cond1b = false;
export let cond2a = false;
export let cond2b = false;
export let log1a = false;
export let log1b = false;
export let log2a = false;
export let log2b = false;

let flag = false;
checkConditional();
checkLogical();
checkConditional1();
checkConditional2();
checkLogical1();
checkLogical2();
flag = true;
checkConditional();
checkLogical();
checkConditional1();
checkConditional2();
checkLogical1();
checkLogical2();

function checkConditional() {
if (flag ? true : false) first = true;
else second = true;
function checkConditional1() {
if (flag ? true : false) cond1a = true;
else cond1b = true;
}

function checkLogical() {
if (flag && true) third = true;
else fourth = true;
function checkConditional2() {
if (!flag ? true : false) cond2a = true;
else cond2b = true;
}

function checkLogical1() {
if (flag && true) log1a = true;
else log1b = true;
}

function checkLogical2() {
if (!flag && true) log2a = true;
else log2b = true;
}
3 changes: 3 additions & 0 deletions test/function/samples/long-path-deopt/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'deoptimizes getting return expression for long property paths'
};

0 comments on commit 01e2461

Please sign in to comment.