LGTM, just commenting nits.

http://gwt-code-reviews.appspot.com/1442807/diff/1065/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
File dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
(right):

http://gwt-code-reviews.appspot.com/1442807/diff/1065/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java#newcode49
dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java:49: if
(x.isVolatile() || !x.canBePolymorphic()) {
Update comment to account for the x.isVolatile() check?

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java (right):

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java#newcode34
dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java:34: */
thanks for the docs.

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java (right):

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java#newcode98
dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java:98: * Explcitly
traverse the onSuccessCall.
This comment doesn't really tell us much more than the code itself.
The bug that led to this code was tricky - maybe add a bit more
explanation?

http://gwt-code-reviews.appspot.com/1442807/diff/6025/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/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode371
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:371: *
Magic magic magic: don't allow code flow from the AsyncFragmentLoader
(again, not magic...)

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):

http://gwt-code-reviews.appspot.com/1442807/diff/6025/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java#newcode66
dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:66: *
Magic magic magic: don't optimizations on the calls from from
don't optimizations ==> don't run tightening optimizations

I don't think the term 'magic' is appropriate.  I think the code below
can be explained through science and reason.  For example, you could
just add another sentence or two to explain why tightening optimizations
might defeat code splitting.

http://gwt-code-reviews.appspot.com/1442807/

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

Reply via email to