Author: b...@google.com
Date: Mon Jun 22 10:54:08 2009
New Revision: 5598

Modified:
    branches/snapshot-2009.06.16-r5570/   (props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/core/ext/ServletContainer.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/core/ext/ServletContainerLauncher.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/js/JsInliner.java
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerDisableAggressiveOptimization.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerEnableAssertions.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerOutDir.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerWorkDirOptional.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerWorkDirRequired.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/test/com/google/gwt/dev/AboutTest.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
     
branches/snapshot-2009.06.16-r5570/distro-source/linux/src/webAppCreator    
(props changed)
    branches/snapshot-2009.06.16-r5570/distro-source/mac/src/webAppCreator    
(props changed)
     
branches/snapshot-2009.06.16-r5570/distro-source/windows/src/webAppCreator.cmd  
  
(props changed)
     
branches/snapshot-2009.06.16-r5570/eclipse/samples/Hello/Hello-gwtc.launch    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/dynatable/war/DynaTable.css    
(props changed)
     
branches/snapshot-2009.06.16-r5570/samples/dynatable/war/DynaTable.html    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/hello/war/Hello.html   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/i18n/war/I18N.css   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/i18n/war/I18N.html   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/json/war/JSON.css   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/json/war/JSON.html   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/mail/war/Mail.css   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/mail/war/Mail.html   (props  
changed)
    branches/snapshot-2009.06.16-r5570/samples/mail/war/gradient.gif    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/mail/war/leftCorner.gif    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/mail/war/rightCorner.gif    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/showcase/war/Showcase.html    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/simplerpc/war/SimpleRPC.css    
(props changed)
     
branches/snapshot-2009.06.16-r5570/samples/simplerpc/war/SimpleRPC.html    
(props changed)
    branches/snapshot-2009.06.16-r5570/samples/simplexml/war/SimpleXML.css    
(props changed)
     
branches/snapshot-2009.06.16-r5570/samples/simplexml/war/SimpleXML.html    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/core/client/HttpThrowableReporter.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/core/client/JsonUtils.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/dom/client/HandlesAllFocusEvents.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/dom/client/HandlesAllKeyEvents.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/dom/client/HandlesAllMouseEvents.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/dom/client/KeyCodes.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/dom/client/PrivateMap.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/logical/shared/HasValueChangeHandlers.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/logical/shared/ValueChangeEvent.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/logical/shared/ValueChangeHandler.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/shared/DefaultHandlerRegistration.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/shared/GwtEvent.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/event/shared/HandlerRegistration.java
    
(props changed)
    branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/client/ClientBundle.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/ext/ClientBundleFields.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/ext/ClientBundleRequirements.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/rebind/context/InlineClientBundleGenerator.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/resources/rebind/context/StaticClientBundleGenerator.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/user/RemoteServiceObfuscateTypeNames.gwt.xml
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/user/client/ui/ListenerWrapper.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/user/rebind/StringSourceWriter.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/src/com/google/gwt/user/tools/WebAppCreator.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/core/client/HttpThrowableReporterTest.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/dev/jjs/test/jsoimpls/UnreferencedImplOfJsoInterface.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/dev/jjs/test/jsointfs/JsoInterfaceWithUnreferencedImpl.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/event/dom/client/DomEventTest.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/event/logical/shared/LogicalEventsTest.java
    
(props changed)
    branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/resources/    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/RPCSuiteWithObfuscation.gwt.xml
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/CollectionsTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/CustomFieldSerializerTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/EnumsTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/InheritanceTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/ObjectGraphTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/RemoteServiceServletTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/UnicodeEscapingTestWithTypeObfuscation.java
    
(props changed)
     
branches/snapshot-2009.06.16-r5570/user/test/com/google/gwt/user/client/rpc/ValueTypesTestWithTypeObfuscation.java
    
(props changed)

Log:
Merge r5597 into snapshot to fix JsInliner bug.
$ svn merge -c5597 https://google-web-toolkit.googlecode.com/svn/trunk



Modified:  
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/js/JsInliner.java
==============================================================================
---  
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/js/JsInliner.java
     
(original)
+++  
branches/snapshot-2009.06.16-r5570/dev/core/src/com/google/gwt/dev/js/JsInliner.java
     
Mon Jun 22 10:54:08 2009
@@ -722,6 +722,14 @@
     */
    private static class InliningVisitor extends JsModVisitor {
      private final Set<JsFunction> blacklist = new HashSet<JsFunction>();
+    /**
+     * This reflects the functions that are currently being inlined to  
prevent
+     * infinite expansion.
+     */
+    private final Stack<JsFunction> inlining = new Stack<JsFunction>();
+    /**
+     * This reflects which function the visitor is currently visiting.
+     */
      private final Stack<JsFunction> functionStack = new  
Stack<JsFunction>();
      private final InvocationCountingVisitor invocationCountingVisitor =  
new InvocationCountingVisitor();
      private final Stack<List<JsName>> newLocalVariableStack = new  
Stack<List<JsName>>();
@@ -859,13 +867,13 @@
         * when trying operate on references to external functions, or  
functions
         * as arguments to another function.
         */
-      JsFunction f = isFunction(x.getQualifier());
-      if (f == null) {
+      JsFunction invokedFunction = isFunction(x.getQualifier());
+      if (invokedFunction == null) {
          return;
        }

        // Don't inline blacklisted functions
-      if (blacklist.contains(f)) {
+      if (blacklist.contains(invokedFunction)) {
          return;
        }

@@ -873,14 +881,130 @@
         * The current function has been mutated so as to be self-recursive.  
Ban
         * it from any future inlining to prevent infinite expansion.
         */
-      if (f == callerFunction) {
-        blacklist.add(f);
+      if (invokedFunction == callerFunction) {
+        blacklist.add(invokedFunction);
+        return;
+      }
+
+      /*
+       * We are already in the middle of attempting to inline a call to  
this
+       * function. This check prevents infinite expansion across
+       * mutually-recursive, inlinable functions. Any invocation skipped  
by this
+       * logic will be re-visited in the <code>op = accept(op)</code> call  
in
+       * the outermost JsInvocation.
+       */
+      if (inlining.contains(invokedFunction)) {
+        return;
+      }
+
+      inlining.push(invokedFunction);
+      JsExpression op = process(x, callerFunction, invokedFunction);
+
+      if (x != op) {
+        /*
+         * See if any further inlining can be performed in the current  
context.
+         * By attempting to maximize the level of inlining now, we can  
reduce
+         * the total number of passes required to finalize the AST.
+         */
+        op = accept(op);
+        ctx.replaceMe(op);
+      }
+
+      if (inlining.pop() != invokedFunction) {
+        throw new RuntimeException("Unexpected function popped");
+      }
+    }
+
+    @Override
+    public void endVisit(JsProgramFragment x, JsContext<JsProgramFragment>  
ctx) {
+      if (!functionStack.pop().equals(programFunction)) {
+        throw new InternalCompilerException("Unexpected function popped");
+      }
+
+      assert programFunction.getBody().getStatements().size() ==  
0 : "Should not have moved statements into program";
+
+      List<JsName> newLocalVariables = newLocalVariableStack.pop();
+      assert newLocalVariables.size() == 0 : "Should not have tried to  
create variables in program";
+    }
+
+    @Override
+    public boolean visit(JsExprStmt x, JsContext<JsStatement> ctx) {
+      if (functionStack.peek() == programFunction) {
+        /* Don't inline top-level invocations. */
+        if (x.getExpression() instanceof JsInvocation) {
+          return false;
+        }
+      }
+      return true;
+    }
+
+    @Override
+    public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
+      functionStack.push(x);
+      newLocalVariableStack.push(new ArrayList<JsName>());
+      return true;
+    }
+
+    /**
+     * Create a synthetic context to attempt to simplify statements in the
+     * top-level of the program.
+     */
+    @Override
+    public boolean visit(JsProgramFragment x, JsContext<JsProgramFragment>  
ctx) {
+      programFunction = new JsFunction(program.getSourceInfo(),
+          program.getScope());
+      programFunction.setBody(new JsBlock(x.getSourceInfo()));
+      functionStack.push(programFunction);
+      newLocalVariableStack.push(new ArrayList<JsName>());
+      return true;
+    }
+
+    private void addVars(HasSourceInfo x, JsBlock body,
+        List<JsName> newLocalVariables) {
+      // Nothing to do
+      if (newLocalVariables.isEmpty()) {
          return;
        }

+      List<JsStatement> statements = body.getStatements();
+
+      // The body can't be empty if we have local variables to create
+      assert !statements.isEmpty();
+
+      // Find or create the JsVars as the first statement
+      SourceInfo sourceInfo = x.getSourceInfo().makeChild(
+          InliningVisitor.class, "Synthetic locals");
+      JsVars vars;
+      if (statements.get(0) instanceof JsVars) {
+        vars = (JsVars) statements.get(0);
+      } else {
+        vars = new JsVars(sourceInfo);
+        statements.add(0, vars);
+      }
+
+      // Add all variables
+      for (JsName name : newLocalVariables) {
+        vars.add(new JsVar(sourceInfo, name));
+      }
+    }
+
+    private boolean isInvokedMoreThanOnce(JsFunction f) {
+      Integer count = invocationCountingVisitor.invocationCount(f);
+      return count == null || count > 1;
+    }
+
+    /**
+     * Determine if <code>invokedFunction</code> can be inlined into
+     * <code>callerFunction</code> at callsite <code>x</code>.
+     *
+     * @return An expression equivalent to <code>x</code>
+     */
+    private JsExpression process(JsInvocation x, JsFunction callerFunction,
+        JsFunction invokedFunction) {
        List<JsStatement> statements;
-      if (f.getBody() != null) {
-        statements = new  
ArrayList<JsStatement>(f.getBody().getStatements());
+      if (invokedFunction.getBody() != null) {
+        statements = new ArrayList<JsStatement>(
+            invokedFunction.getBody().getStatements());
        } else {
          /*
           * Will see this with certain classes whose clinits are folded  
into the
@@ -908,7 +1032,7 @@
             * TODO(bobv): maybe it could still be inlined with smart
             * transformation?
             */
-          return;
+          return x;
          }

          /*
@@ -922,7 +1046,7 @@
          JsExpression h = hoistedExpression(program, statement,
              localVariableNames);
          if (h == null) {
-          return;
+          return x;
          }

          if (isReturnStatement(statement)) {
@@ -963,12 +1087,13 @@
        }

        // Confirm that the expression conforms to the desired heuristics
-      if (!isInlinable(program, callerFunction, f, x.getArguments(), op)) {
-        return;
+      if (!isInlinable(program, callerFunction, invokedFunction,
+          x.getArguments(), op)) {
+        return x;
        }

        // Perform the name replacement
-      NameRefReplacerVisitor v = new NameRefReplacerVisitor(x, f);
+      NameRefReplacerVisitor v = new NameRefReplacerVisitor(x,  
invokedFunction);
        for (ListIterator<JsName> nameIterator =  
localVariableNames.listIterator(); nameIterator.hasNext();) {
          JsName name = nameIterator.next();

@@ -978,7 +1103,7 @@
           * function so we'll use a counter for disambiguation.
           */
          String ident;
-        String base = f.getName() + "_" + name.getIdent();
+        String base = invokedFunction.getName() + "_" + name.getIdent();
          JsScope scope = callerFunction.getScope();
          HashMap<String, Integer> startIdent =  
startIdentForScope.get(scope);
          if (startIdent == null) {
@@ -1009,13 +1134,14 @@
        int originalComplexity = complexity(x);
        int inlinedComplexity = complexity(op);
        double ratio = ((double) inlinedComplexity) / originalComplexity;
-      if (ratio > MAX_COMPLEXITY_INCREASE && isInvokedMoreThanOnce(f)) {
-        return;
+      if (ratio > MAX_COMPLEXITY_INCREASE
+          && isInvokedMoreThanOnce(invokedFunction)) {
+        return x;
        }

        if (callerFunction == programFunction && localVariableNames.size() >  
0) {
          // Don't add additional variables to the top-level program.
-        return;
+        return x;
        }

        // We've committed to the inlining, ensure the vars are created
@@ -1024,92 +1150,7 @@
        // update invocation counts according to this inlining
        invocationCountingVisitor.removeCountsFor(x);
        invocationCountingVisitor.accept(op);
-
-      /*
-       * See if any further inlining can be performed in the current  
context. By
-       * attempting to maximize the level of inlining now, we can reduce  
the
-       * total number of passes required to finalize the AST.
-       */
-      op = accept(op);
-      ctx.replaceMe(op);
-    }
-
-    @Override
-    public void endVisit(JsProgramFragment x, JsContext<JsProgramFragment>  
ctx) {
-      if (!functionStack.pop().equals(programFunction)) {
-        throw new InternalCompilerException("Unexpected function popped");
-      }
-
-      assert programFunction.getBody().getStatements().size() ==  
0 : "Should not have moved statements into program";
-
-      List<JsName> newLocalVariables = newLocalVariableStack.pop();
-      assert newLocalVariables.size() == 0 : "Should not have tried to  
create variables in program";
-    }
-
-    @Override
-    public boolean visit(JsExprStmt x, JsContext<JsStatement> ctx) {
-      if (functionStack.peek() == programFunction) {
-        /* Don't inline top-level invocations. */
-        if (x.getExpression() instanceof JsInvocation) {
-          return false;
-        }
-      }
-      return true;
-    }
-
-    @Override
-    public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
-      functionStack.push(x);
-      newLocalVariableStack.push(new ArrayList<JsName>());
-      return true;
-    }
-
-    /**
-     * Create a synthetic context to attempt to simplify statements in the
-     * top-level of the program.
-     */
-    @Override
-    public boolean visit(JsProgramFragment x, JsContext<JsProgramFragment>  
ctx) {
-      programFunction = new JsFunction(program.getSourceInfo(),
-          program.getScope());
-      programFunction.setBody(new JsBlock(x.getSourceInfo()));
-      functionStack.push(programFunction);
-      newLocalVariableStack.push(new ArrayList<JsName>());
-      return true;
-    }
-
-    private void addVars(HasSourceInfo x, JsBlock body,
-        List<JsName> newLocalVariables) {
-      // Nothing to do
-      if (newLocalVariables.isEmpty()) {
-        return;
-      }
-
-      List<JsStatement> statements = body.getStatements();
-
-      // The body can't be empty if we have local variables to create
-      assert !statements.isEmpty();
-
-      // Find or create the JsVars as the first statement
-      SourceInfo sourceInfo = x.getSourceInfo().makeChild(
-          InliningVisitor.class, "Synthetic locals");
-      JsVars vars;
-      if (statements.get(0) instanceof JsVars) {
-        vars = (JsVars) statements.get(0);
-      } else {
-        vars = new JsVars(sourceInfo);
-        statements.add(0, vars);
-      }
-
-      // Add all variables
-      for (JsName name : newLocalVariables) {
-        vars.add(new JsVar(sourceInfo, name));
-      }
-    }
-
-    private boolean isInvokedMoreThanOnce(JsFunction f) {
-      Integer count = invocationCountingVisitor.invocationCount(f);
-      return count == null || count > 1;
+      return op;
      }
    }


Modified:  
branches/snapshot-2009.06.16-r5570/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
==============================================================================
---  
branches/snapshot-2009.06.16-r5570/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
        
(original)
+++  
branches/snapshot-2009.06.16-r5570/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
        
Mon Jun 22 10:54:08 2009
@@ -27,27 +27,50 @@
   */
  public class JsInlinerTest extends OptimizerTestBase {

+  private static class FixStaticRefsVisitor extends JsModVisitor {
+    public static void exec(JsProgram program) {
+      (new FixStaticRefsVisitor()).accept(program);
+    }
+
+    @Override
+    public void endVisit(JsFunction x, JsContext<JsExpression> ctx) {
+      JsName name = x.getName();
+      name.setStaticRef(x);
+    }
+  }
+
+  /**
+   * A test for mutually-recursive functions. Setup:
+   *
+   * <pre>
+   * a -> b, c
+   * b -> a, c
+   * c -> a, c
+   * </pre>
+   */
+  public void testMutualRecursion() throws Exception {
+    String input = "function a1() { return ex ? b1() : c1() }"
+        + "function b1() { return ex2 ? a1(): c1(); }"
+        + "function c1() { return ex2? a1():c1(); } c1()";
+    String expected = "function a1() { return ex ? (ex2 ? a1() : c1()) :  
c1() }"
+        + "function c1() { return ex2 ? a1() :c1(); } c1()";
+    compare(expected, input);
+  }
+
    public void testSelfRecursion() throws Exception {
      String input = "function a1() { return blah && b1() }"
          + "function b1() { return bar && a1()}" + "function c() { a1() }  
c()";
-    input = optimize(input, JsSymbolResolver.class,  
FixStaticRefsVisitor.class,
-        JsInliner.class, JsUnusedFunctionRemover.class);

      String expected = "function a1() { return blah && bar && a1() }"
          + "function c() { a1() } c()";
-    expected = optimize(expected);
-    assertEquals(expected, input);
-  }

-  private static class FixStaticRefsVisitor extends JsModVisitor {
-    @Override
-    public void endVisit(JsFunction x, JsContext<JsExpression> ctx) {
-      JsName name = x.getName();
-      name.setStaticRef(x);
-    }
+    compare(expected, input);
+  }

-    public static void exec(JsProgram program) {
-      (new FixStaticRefsVisitor()).accept(program);
-    }
+  private void compare(String expected, String input) throws Exception {
+    input = optimize(input, JsSymbolResolver.class,  
FixStaticRefsVisitor.class,
+        JsInliner.class, JsUnusedFunctionRemover.class);
+    expected = optimize(expected);
+    assertEquals(expected, input);
    }
  }

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to