Skip to content

Commit

Permalink
Fix void context transform !cond && ok() => cond || ok()
Browse files Browse the repository at this point in the history
Close #45
  • Loading branch information
Constellation committed Oct 7, 2012
1 parent 8f8c610 commit 569562c
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 7 deletions.
29 changes: 22 additions & 7 deletions lib/pass/remove-context-sensitive-expressions.js
@@ -1,4 +1,5 @@
/*
Copyright (C) 2012 Mihai Bazon <mihai.bazon@gmail.com>
Copyright (C) 2012 Yusuke Suzuki <utatane.tea@gmail.com>
Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -43,13 +44,20 @@
continue;
}
} else if (expr.type === Syntax.LogicalExpression) {
if (expr.left.type === Syntax.UnaryExpression) {
if (expr.left.operator === '!') {
// !cond && ok() => cond || ok()
modified = true;
expr.left = expr.left.argument;
expr.operator = (expr.operator === '||') ? '&&' : '||';
}

This comment has been minimized.

Copy link
@Constellation

Constellation Oct 7, 2012

Author Member

@mishoo:

I took a mistake on boolean context transformation.
This transformation should be done in void context.

Thanks for your report! Your files and movies help me a lot! I added your name to copyright comment.

This comment has been minimized.

Copy link
@mishoo

mishoo Oct 7, 2012

Good catch!

I added your name to copyright comment.

That really wasn't necessary.. :-) I'm glad I could help nailing it down. Indeed I just pulled latest code and the two issues I noticed have disappeared.

if (expr.left.type === Syntax.UnaryExpression && expr.left.operator === '!' &&
expr.right.type === Syntax.UnaryExpression && expr.right.operator === '!') {
// !cond && !ok() => !(cond || ok())
// this introduces more optimizations
modified = true;
expr.left = expr.left.argument;
expr.right = expr.right.argument;
expr.operator = (expr.operator === '||') ? '&&' : '||';
expr = common.moveLocation(expr, {
type: Syntax.UnaryExpression,
operator: '!',
argument: expr
});
continue;
}
} else if (expr.type === Syntax.ConditionalExpression) {
if (expr.test.type === Syntax.UnaryExpression && expr.test.operator === '!') {
Expand All @@ -74,6 +82,13 @@
expr = expr.argument;
continue;
}
} else if (expr.type === Syntax.LogicalExpression) {
if (expr.left.type === Syntax.UnaryExpression && expr.left.operator === '!') {
// !cond && ok() => cond || ok()
modified = true;
expr.left = expr.left.argument;
expr.operator = (expr.operator === '||') ? '&&' : '||';
}
}
break;
} while (true);
Expand Down
@@ -0,0 +1 @@
!ok||ok2()
2 changes: 2 additions & 0 deletions test/compare/remove-context-sensitive-expression-6.js
@@ -0,0 +1,2 @@
// Because of global environment
if (!ok || ok2());
@@ -0,0 +1 @@
(function(){ok&&ok2()}())
3 changes: 3 additions & 0 deletions test/compare/remove-context-sensitive-expression-7.js
@@ -0,0 +1,3 @@
(function () {
if (!ok || ok2());
}());
@@ -0,0 +1 @@
(function(){while(!ok||ok2())ok3()}())
5 changes: 5 additions & 0 deletions test/compare/remove-context-sensitive-expression-8.js
@@ -0,0 +1,5 @@
(function () {
while (!ok || ok2()) {

This comment has been minimized.

Copy link
@Constellation

Constellation Oct 7, 2012

Author Member

Don't transform it to ok && ok2() :)

ok3();
}
}());
@@ -0,0 +1 @@
(function(){while(!(ok&&ok2()))ok3()}())
8 changes: 8 additions & 0 deletions test/compare/remove-context-sensitive-expression-9.js
@@ -0,0 +1,8 @@
// TODO(Constellation):
// This transformation sometimes make script bigger size.
// So we should handle it in post processing pass.
(function () {
while (!ok || !ok2()) {
ok3();
}
}());

0 comments on commit 569562c

Please sign in to comment.