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
-~----------~----~----~----~------~----~------~--~---

Reply via email to