Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not include unused return expressions #4573

Merged
merged 3 commits into from Jul 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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'
};