Re: JsDuplicateFunctionRemover

[sp...@google.com]

The code looks good, but I'd like to try it out before a final approval.
Can you repost a patch with the missing little bits fixed up?

http://gwt-code-reviews.appspot.com/126818/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(right):

http://gwt-code-reviews.appspot.com/126818/diff/1/2#newcode264
Line 264: options.getOutput(), symbolTable, propertyOracles);
Did you end up needing to change GenerateJavaScriptAST? If so, please
add it to the patch. However, it looks like this change could be
reverted.

http://gwt-code-reviews.appspot.com/126818/diff/1/2#newcode319
Line 319: JsStackEmulator.StackMode.STRIP &&
These are good helper methods but are missing from the patch.

http://gwt-code-reviews.appspot.com/126818/diff/1/3
File dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
(right):

http://gwt-code-reviews.appspot.com/126818/diff/1/3#newcode47
Line 47: public boolean visit(JsFunction x, JsContext<JsExpression> ctx)
{
There should be a check for functions that have no name, shouldn't
there?

http://gwt-code-reviews.appspot.com/126818/diff/1/3#newcode51
Line 51: * TODO: if constructors ever inlined into seed functions,
revisit this
This TODO is a good point. To make it more helpful: (a) drop the "TODO"
from here, because there's nothing in particular planned to be done, and
(b) add a note on
GenerateJavaScriptAST.GenerateJavaScriptVisitor.generateSeedFuncAndPrototype
that any change there needs to be reflected over here.

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