This brings up the issue of whether or not there isn't someway to abstract some of these optimizations on the ASTs so that they can be shared by the Java and JS optimizers. I made patch along the same lines to the simplifier mirroring the Java simplifier. It seems like a lot of optimizations are being duplicated.
-Ray On Tue, Jun 9, 2009 at 1:12 AM, <[email protected]> wrote: > > Reviewers: scottb, > > Description: > I'm tackling another small issue on the JS side to get myself up to > speed with the code (esp. the SOYC changes throughout) and help shake > out all the cobwebs since my last set of patches :). > > This is a simple patch to improve the optimization of if statements. > These items show up in compiled output, mostly in the generated > exception handler code. > > It adds two extra optimizations: > > - Empty, but present else statements are pruned: > > if (x) {doSomething();} else {} -> if (x) {doSomething();} > > - If statements with empty "then" conditions are inverted: > > if (x) {} else {doSomething()} -> if (!x) {doSomething();} > > I also cleaned up my previous set of unit tests to use the clearer > assertEquals() instead of assertTrue(x.equals(...)). > > > Please review this at http://gwt-code-reviews.appspot.com/33845 > > Affected files: > dev/core/src/com/google/gwt/dev/js/JsStaticEval.java > dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java > > > Index: dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java > =================================================================== > --- dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java (revision > 5519) > +++ dev/core/test/com/google/gwt/dev/js/JsStaticEvalTest.java (working copy) > @@ -29,20 +29,36 @@ > > public class JsStaticEvalTest extends TestCase { > > + public void testIfWithEmptyThen() throws Exception { > + assertEquals("a();", optimize("if (a()) { }")); > + } > + > + public void testIfWithEmptyThenAndElse() throws Exception { > + assertEquals("if(!a()){b()}", optimize("if (a()) { } else { b(); }")); > + } > + > + public void testIfWithEmptyThenAndEmptyElse() throws Exception { > + assertEquals("a();", optimize("if (a()) { } else { }")); > + } > + > + public void testIfWithThenAndEmptyElse() throws Exception { > + assertEquals("if(a()){b()}", optimize("if (a()) { b() } else { }")); > + } > + > public void testLiteralEqNull() throws Exception { > - assertTrue(optimize("alert('test' == null)").equals("alert(false);")); > + assertEquals("alert(false);", optimize("alert('test' == null)")); > } > > public void testLiteralNeNull() throws Exception { > - assertTrue(optimize("alert('test' != null)").equals("alert(true);")); > + assertEquals("alert(true);", optimize("alert('test' != null)")); > } > > public void testNullEqNull() throws Exception { > - assertTrue(optimize("alert(null == null)").equals("alert(true);")); > + assertEquals("alert(true);", optimize("alert(null == null)")); > } > > public void testNullNeNull() throws Exception { > - assertTrue(optimize("alert(null != null)").equals("alert(false);")); > + assertEquals("alert(false);", optimize("alert(null != null)")); > } > > private String optimize(String js) throws Exception { > Index: dev/core/src/com/google/gwt/dev/js/JsStaticEval.java > =================================================================== > --- dev/core/src/com/google/gwt/dev/js/JsStaticEval.java (revision > 5519) > +++ dev/core/src/com/google/gwt/dev/js/JsStaticEval.java (working copy) > @@ -35,6 +35,7 @@ > import com.google.gwt.dev.js.ast.JsPrefixOperation; > import com.google.gwt.dev.js.ast.JsProgram; > import com.google.gwt.dev.js.ast.JsStatement; > +import com.google.gwt.dev.js.ast.JsUnaryOperation; > import com.google.gwt.dev.js.ast.JsUnaryOperator; > import com.google.gwt.dev.js.ast.JsValueLiteral; > import com.google.gwt.dev.js.ast.JsVars; > @@ -348,8 +349,37 @@ > block.getStatements().add(decls); > } > ctx.replaceMe(accept(block)); > - } else if (isEmpty(thenStmt) && isEmpty(elseStmt)) { > - ctx.replaceMe(expr.makeStmt()); > + } else { > + boolean thenIsEmpty = isEmpty(thenStmt); > + boolean elseIsEmpty = isEmpty(elseStmt); > + > + if (thenIsEmpty && elseIsEmpty) { > + ctx.replaceMe(expr.makeStmt()); > + } else if (thenIsEmpty && !elseIsEmpty) { > + /* > + * If the then block is blank, but the else statement has > statements, > + * invert the test > + */ > + sourceInfo = x.getSourceInfo().makeChild(StaticEvalVisitor.class, > + "Simplified if with empty then statement"); > + > + JsUnaryOperation negatedOperation = new > JsPrefixOperation(sourceInfo, > + JsUnaryOperator.NOT, x.getIfExpr()); > + JsIf newIf = new JsIf(sourceInfo, negatedOperation, elseStmt, > null); > + > + ctx.replaceMe(accept(newIf)); > + } else if (elseIsEmpty && elseStmt != null) { > + /* > + * If the else statement is present but has no effective > statements, > + * prune it > + */ > + sourceInfo = x.getSourceInfo().makeChild(StaticEvalVisitor.class, > + "Pruned empty else statement"); > + > + JsIf newIf = new JsIf(sourceInfo, x.getIfExpr(), thenStmt, null); > + > + ctx.replaceMe(accept(newIf)); > + } > } > } > > > > > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
