Most of the way there to signing off, with a few questions/suggestions.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java File dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java (right): http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode84 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:84: * Marks as "referenced" any types, methods, and fields that are reachable. s/any the classes/any classes/ http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode382 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:382: This is specific only to "Pruner.CleanupRefsVisitor"? Why is it 'crazy'? Seems pretty straightforward, this is where you are populating the argsLiveExceptForParamRead map. Maybe describe why this useful (or add a comment below where argsLiveExceptForParamRed is declared, as is done for membersLiveExceptForInstantiability). http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode385 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:385: if (program.staticImplFor(method) != null) { Pruner.CleanupRefsVisitor http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode772 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:772: Perhaps add a comment for this map, similar to the "Schrodinger's" comment below for membersLiveExceptForInstantiability. I'm a bit confused by the name "argsLiveExceptForParamRead", how about "argsToAcceptIfParamRead". http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode787 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:787: */ How about "membersToRescueIfInstantiable" http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java File dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java (right): http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode421 dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:421: * devirtualizations. If the instance method has been pruned, then it's Here I think it's assumed "saveCodeGenTypes == true" = "it's the final pass". Perhaps this should be made more clear, or the comment updated to clarify. http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode570 dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:570: // Retain the original arguments, they will be evaluated for side effects. This seems like a good change! http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode600 dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:600: Is it now certain that the Pruner should not need any subsequent passes to complete all possible pruning? Or is it "most of the way there", and this is now fair to cut it off to save time? http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java (right): http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java#newcode145 dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java:145: program.addEntryMethod(findMainMethod(program)); Why is this looping necessary? Since the change in Pruner removes looping, shouldn't we be testing here the behavior with no looping also? Do we have a test for the second arg to Pruner.exec() is false? http://gwt-code-reviews.appspot.com/1436802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
