+bobv Bob is also looking at a bug in JsInliner right now. Dunno if it's related to this or not, but maybe Ray+Bob you guys can pair up on those two bugs?
On Thu, Oct 15, 2009 at 4:50 AM, <cromwell...@gmail.com> wrote: > Reviewers: Lex, scottb, > > Description: > Patched AffectedBySideEffectsVisitor so that Object and Array literals > are not incorrectly inlined. > > Note: JsNew is already handled because it's hasSideEffects method > returns true. By contrast, JClassSeed/JNewInstance/JNewArray return > false. A difference we might want to think about. > > In theory, string and number literals can fall prey to this inliner bug, > but in practice, you don't mutate fields on strings and numbers. > > > Please review this at http://gwt-code-reviews.appspot.com/77822 > > Affected files: > dev/core/src/com/google/gwt/dev/js/JsInliner.java > dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java > > > Index: dev/core/src/com/google/gwt/dev/js/JsInliner.java > =================================================================== > --- dev/core/src/com/google/gwt/dev/js/JsInliner.java (revision 5597) > +++ dev/core/src/com/google/gwt/dev/js/JsInliner.java Thu Oct 15 01:39:21 > PDT 2009 > @@ -98,6 +98,12 @@ > } > > @Override > + public void endVisit(JsArrayLiteral x, JsContext<JsExpression> ctx) { > + // fix for b/2153220 > + affectedBySideEffects = true; > + } > + > + @Override > public void endVisit(JsInvocation x, JsContext<JsExpression> ctx) { > /* > * We could make this more accurate by analyzing the function that's > being > @@ -126,7 +132,13 @@ > */ > affectedBySideEffects = true; > } > + > + @Override > + public void endVisit(JsObjectLiteral x, JsContext<JsExpression> ctx) { > + // fix for b/2153220 > + affectedBySideEffects = true; > - } > + } > + } > > /** > * Make comma binary operations left-nested since commas are naturally > Index: dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java > =================================================================== > --- dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java (revision > 5597) > +++ dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Thu Oct 15 > 01:40:41 PDT 2009 > @@ -28,6 +28,7 @@ > public class JsInlinerTest extends OptimizerTestBase { > > private static class FixStaticRefsVisitor extends JsModVisitor { > + > public static void exec(JsProgram program) { > (new FixStaticRefsVisitor()).accept(program); > } > @@ -39,9 +40,21 @@ > } > } > > + public void testInlineArrayLiterals() throws Exception { > + String input = "function a1(arg, x) { arg.x = x; return arg; }" > + + "function b1() { var x=a1([], 10); } b1();"; > + compare(input, input); > + } > + > + public void testInlineObjectLiterals() throws Exception { > + String input = "function a1(arg, x) { arg.x = x; return arg; }" > + + "function b1() { var x=a1({}, 10); } b1();"; > + compare(input, input); > + } > + > /** > * A test for mutually-recursive functions. Setup: > - * > + * > * <pre> > * a -> b, c > * b -> a, c > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
