http://gwt-code-reviews.appspot.com/130812/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/AssumptionDeducer.java (right):
http://gwt-code-reviews.appspot.com/130812/diff/1/2#newcode135 Line 135: // Only last expression can be reverse engineered. Either htis comment is innacurate or I don't understand what's going on. I believe what's going on is that, from the point of view of a CFG, the only operation under consideration is the passing of the result of the last expression to being the result of the multi-expression. All the other evaluation steps are represented as separate CFG nodes, and those nodes' effects on the assumptions will already be presented in the assumptions list. If that's what's going on, then please update the comment. http://gwt-code-reviews.appspot.com/130812/diff/1/2#newcode154 Line 154: private boolean isDeduciveEqValue(JExpression e) { This method seems to mean, "would two expressions equal to e be substitutable for each other". If that's what it means, please update the name and comment. "Can deduce something" is awfully vague. http://gwt-code-reviews.appspot.com/130812/diff/1/4 File dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAnalysis.java (right): http://gwt-code-reviews.appspot.com/130812/diff/1/4#newcode30 Line 30: * As of now supports only locals & parameters. Document that it also does constant folding. http://gwt-code-reviews.appspot.com/130812/diff/1/5 File dev/core/src/com/google/gwt/dev/jjs/impl/gflow/constants/ConstantsAssumption.java (right): http://gwt-code-reviews.appspot.com/130812/diff/1/5#newcode35 Line 35: public static class CopyOnWrite { It took me a long time to figure out that this class is a builder that is initialized with a previous instance of the class. That's a good pattern, but the name makes it obscure. Please rename it to something with something including "builder" or "updater" or the like. http://gwt-code-reviews.appspot.com/130812/diff/1/5#newcode154 Line 154: values.put(variable, literal); Given that there is a builder for the class, it would seem better to make mutators like this be private. Then, the public API of the class would be immutable. Likewise for the assumption/CopyOnWrite pairs for all the other optimizers. http://gwt-code-reviews.appspot.com/130812/diff/1/5#newcode185 Line 185: private boolean equal(Object o1, Object o2) { This ends up compalir JValueLiteral AST nodes rather than the represented literals. Perhaps change the argument types to JValueLiteral, and compare the underlying literals? Be careful with floats; for safety's sake we might start with only treating them as equal if their underlying bits are the same. http://gwt-code-reviews.appspot.com/130812 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
