Skip to content

Commit

Permalink
Do not entirely move else branch if another else branch might acciden…
Browse files Browse the repository at this point in the history
…tally be referenced.
  • Loading branch information
lukastaegert committed Aug 14, 2020
1 parent 13f18ef commit ad6b54d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
17 changes: 17 additions & 0 deletions src/ast/nodes/IfStatement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { DeoptimizableEntity } from '../DeoptimizableEntity';
import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
import { EMPTY_PATH, SHARED_RECURSION_TRACKER } from '../utils/PathTracker';
import { LiteralValueOrUnknown, UnknownValue } from '../values';
import BlockStatement from './BlockStatement';
import * as NodeType from './NodeType';
import { ExpressionNode, IncludeChildren, StatementBase, StatementNode } from './shared/Node';

Expand Down Expand Up @@ -86,6 +87,8 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
code.prependLeft(this.alternate.start, ' ');
}
this.alternate.render(code, options);
} else if (this.shouldKeepAlternateBranch()) {
code.overwrite(this.alternate.start, this.alternate.end, ';');
} else {
code.remove(this.consequent.end, this.alternate.end);
}
Expand Down Expand Up @@ -142,4 +145,18 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow;
}
}

private shouldKeepAlternateBranch() {
let currentParent = this.parent;
do {
if (currentParent instanceof IfStatement && currentParent.alternate) {
return true;
}
if (currentParent instanceof BlockStatement) {
return false;
}
currentParent = (currentParent as any).parent;
} while (currentParent);
return false;
}
}
7 changes: 7 additions & 0 deletions test/function/samples/simplified-nested-if/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
description:
'do not catch else branches from parent if statements when simplifiying if-statements',
context: {
unknown: true
}
};
22 changes: 22 additions & 0 deletions test/function/samples/simplified-nested-if/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
if (unknown)
for (var x = 1; x--; x > 0)
if (!unknown) console.log('effect');
else 'Side effect free code to be dropped';
else throw new Error("Shouldn't be reached");

if (unknown)
for (var x = 1; x--; x > 0)
if ((console.log(), false)) console.log('effect');
else 'Side effect free code to be dropped';
else throw new Error("Shouldn't be reached");

if (unknown) {
for (var x = 1; x--; x > 0)
if (!unknown) console.log('effect');
else 'Side effect free code to be dropped';
} else throw new Error("Shouldn't be reached");

if (unknown)
for (var x = 1; x--; x > 0)
if (!unknown) console.log('effect');
else 'Side effect free code to be dropped';

0 comments on commit ad6b54d

Please sign in to comment.