Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Fix if_return & dce ifelse..return within loops #266

Merged
merged 1 commit into from
Nov 15, 2016
Merged

Fix if_return & dce ifelse..return within loops #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@ jest.autoMockOff();

const babel = require("babel-core");
const unpad = require("../../../utils/unpad");
const deadcode = require("../src/index");
const simplify = require("../../babel-plugin-minify-simplify/src/index");

function transform(code, options, babelOpts) {
return babel.transform(code, Object.assign({}, {
plugins: [[require("../src/index"), options]],
plugins: [[deadcode, options]],
}, babelOpts)).code.trim();
}

function transformWithSimplify(code) {
Copy link
Member Author

@boopathi boopathi Nov 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is not necessary. Will remove it.

Will keep it and add more tests. since this covers one more optimization from simplify.

Edit: ^ DONE

return babel.transform(code, {
plugins: [deadcode, simplify]
}).code;
}

describe("dce-plugin", () => {
it("should remove bindings with no references", () => {
const source = "function foo() {var x = 1;}";
Expand Down Expand Up @@ -2209,6 +2217,7 @@ describe("dce-plugin", () => {
`);
expect(transform(source)).toBe(expected);
});

it("should NOT remove fn params for setters", () => {
const source = unpad(`
function foo() {
Expand All @@ -2225,4 +2234,55 @@ describe("dce-plugin", () => {
`);
expect(transform(source)).toBe(source);
});

// https://github.com/babel/babili/issues/265
it("should not remove return void 0; statement if inside a loop", () => {
const source = unpad(`
function getParentConditionalPath(path) {
let parentPath;
while (parentPath = path.parentPath) {
if (parentPath.isIfStatement() || parentPath.isConditionalExpression()) {
if (path.key === "test") {
return;
} else {
return parentPath;
}
} else {
path = parentPath;
}
}
}
`);

expect(transform(source)).toBe(source);
});

// https://github.com/babel/babili/issues/265
it("should integrate with simplify plugin changing scopes", () => {
const source = unpad(`
function getParentConditionalPath(path) {
let parentPath;
while (parentPath = path.parentPath) {
if (parentPath.isIfStatement() || parentPath.isConditionalExpression()) {
if (path.key === "test") {
return;
} else {
return parentPath;
}
} else {
path = parentPath;
}
}
}
`);
const expected = unpad(`
function getParentConditionalPath(path) {
for (let parentPath; parentPath = path.parentPath;) {
if (parentPath.isIfStatement() || parentPath.isConditionalExpression()) return path.key === "test" ? void 0 : parentPath;
path = parentPath;
}
}
`);
expect(transformWithSimplify(source)).toBe(expected);
});
});
16 changes: 16 additions & 0 deletions packages/babel-plugin-minify-dead-code-elimination/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,12 @@ module.exports = ({ types: t, traverse }) => {
let noNext = true;
let parentPath = path.parentPath;
while (parentPath && !parentPath.isFunction() && noNext) {
// https://github.com/babel/babili/issues/265
if (hasLoopParent(parentPath)) {
noNext = false;
break;
}

const nextPath = parentPath.getSibling(parentPath.key + 1);
if (nextPath.node) {
if (nextPath.isReturnStatement()) {
Expand Down Expand Up @@ -984,4 +990,14 @@ module.exports = ({ types: t, traverse }) => {
} while (path = path.parentPath);
return null;
}

function hasLoopParent(path) {
let parent = path;
do {
if (parent.isLoop()) {
return true;
}
} while (parent = parent.parentPath);
return false;
}
};
55 changes: 51 additions & 4 deletions packages/babel-plugin-minify-simplify/__tests__/simplify-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,10 @@ describe("simplify-plugin", () => {
}
function bar(a) {
if (!lol) try {
doThings();
} catch (e) {
doOtherThings();
}
doThings();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this happened.

} catch (e) {
doOtherThings();
}
}
function baz() {
for (; wow;) if (lol) return;
Expand Down Expand Up @@ -2484,4 +2484,51 @@ describe("simplify-plugin", () => {

expect(transform(source)).toBe(source);
});

it("should consider hoisted definitions in if_return", () => {
const source = unpad(`
function foo() {
bar();
if(x) return;
const {a}=b;
function bar () {
baz();
bar();
}
}
`);
const expected = unpad(`
function foo() {
function bar() {
baz(), bar();
}

if (bar(), !x) {
const { a } = b;
}
}
`);
expect(transform(source)).toBe(expected);
});

it("should optimize if..else..returns", () => {
const source = unpad(`
function foo() {
if (a) {
if (x) return;
else return x;
}
const b = 1;
return "doesn't matter if this is reached or not";
}
`);
const expected = unpad(`
function foo() {
if (a) return x ? void 0 : x;
const b = 1;
return "doesn't matter if this is reached or not";
}
`);
expect(transform(source)).toBe(expected);
});
});
58 changes: 30 additions & 28 deletions packages/babel-plugin-minify-simplify/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,33 +835,34 @@ module.exports = ({ types: t }) => {
return;
}

// Easy: consequent and alternate are return -- conditional.
if (!path.getSibling(path.key + 1).node
&& t.isReturnStatement(node.consequent)
&& t.isReturnStatement(node.alternate)
) {
if (!node.consequent.argument && !node.alternate.argument) {
path.replaceWith(t.expressionStatement(node.test));
return;
}

path.replaceWith(
t.returnStatement(
t.conditionalExpression(
node.test,
node.consequent.argument || VOID_0,
node.alternate.argument || VOID_0
)
)
);
return;
}

// There is nothing after this block. And one or both
// of the consequent and alternate are either expression statment
// or return statements.
if (!path.getSibling(path.key + 1).node && path.parentPath &&
path.parentPath.parentPath && path.parentPath.parentPath.isFunction()
) {
// Easy: consequent and alternate are return -- conditional.
if (t.isReturnStatement(node.consequent)
&& t.isReturnStatement(node.alternate)
) {
if (!node.consequent.argument && !node.alternate.argument) {
path.replaceWith(t.expressionStatement(node.test));
return;
}

path.replaceWith(
t.returnStatement(
t.conditionalExpression(
node.test,
node.consequent.argument || VOID_0,
node.alternate.argument || VOID_0
)
)
);
return;
}

// Only the consequent is a return, void the alternate.
if (t.isReturnStatement(node.consequent) && t.isExpressionStatement(node.alternate)) {
if (!node.consequent.argument) {
Expand Down Expand Up @@ -1461,7 +1462,9 @@ module.exports = ({ types: t }) => {
function genericEarlyExitTransform(path) {
const { node } = path;

const statements = path.container.slice(path.key + 1);
const statements = path.container.slice(path.key + 1)
.filter((stmt) => !t.isFunctionDeclaration(stmt));

if (!statements.length) {
path.replaceWith(t.expressionStatement(node.test));
return;
Expand All @@ -1480,15 +1483,14 @@ module.exports = ({ types: t }) => {

let l = statements.length;
while (l-- > 0) {
path.getSibling(path.key + 1).remove();
if (!t.isFunctionDeclaration(statements[l])) {
path.getSibling(path.key + 1).remove();
}
}

if (statements.length === 1) {
node.consequent = statements[0];
} else {
node.consequent = t.blockStatement(statements);
}
node.consequent = t.blockStatement(statements);

// this should take care of removing the block
path.visit();
}

Expand Down