Fixes JsDuplicateFunctionRemover to be more robust.
Any JsNameRef JsName that is *not* the immediate LHS of a JsInvocation is blacklisted, and therefore, cannot be replaced nor replace another ref. For example, defineSeed(ctor1, ctor2) would stop ctor1 or ctor2 from being considered, and so would ctor1.prototype = ... .
It looks like JsNameOf nodes can refer to functions as well, so these are blacklisted too.
http://gwt-code-reviews.appspot.com/169801
Patch by: cromwellian
Review by: me
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7701 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java b/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
index 99bb1c5..93c8972 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
@@ -19,14 +19,20 @@
import com.google.gwt.dev.js.ast.JsContext;
import com.google.gwt.dev.js.ast.JsExpression;
import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsInvocation;
import com.google.gwt.dev.js.ast.JsModVisitor;
import com.google.gwt.dev.js.ast.JsName;
+import com.google.gwt.dev.js.ast.JsNameOf;
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsProgram;
import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.collect.IdentityHashSet;
import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.Map;
+import java.util.Set;
+import java.util.Stack;
/**
* Replace references to functions which have post-obfuscation duplicate bodies
@@ -37,8 +43,43 @@
private class DuplicateFunctionBodyRecorder extends JsVisitor {
- Map<String, JsName> uniqueBodies = new HashMap<String, JsName>();
- Map<JsName, JsName> duplicateOriginalMap = new HashMap<JsName, JsName>();
+ private final Set<JsName> dontReplace = new IdentityHashSet<JsName>();
+
+ private final Map<JsName, JsName> duplicateOriginalMap = new IdentityHashMap<JsName, JsName>();
+
+ private final Stack<JsNameRef> invocationQualifiers = new Stack<JsNameRef>();
+
+ private final Map<String, JsName> uniqueBodies = new HashMap<String, JsName>();
+
+ public DuplicateFunctionBodyRecorder() {
+ // Add sentinel to stop Stack.peek() from throwing exception.
+ invocationQualifiers.add(null);
+ }
+
+ @Override
+ public void endVisit(JsInvocation x, JsContext<JsExpression> ctx) {
+ if (x.getQualifier() instanceof JsNameRef) {
+ invocationQualifiers.pop();
+ }
+ }
+
+ @Override
+ public void endVisit(JsNameOf x, JsContext<JsExpression> ctx) {
+ dontReplace.add(x.getName());
+ }
+
+ @Override
+ public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
+ if (x != invocationQualifiers.peek()) {
+ if (x.getName() != null) {
+ dontReplace.add(x.getName());
+ }
+ }
+ }
+
+ public Set<JsName> getBlacklist() {
+ return dontReplace;
+ }
public Map<JsName, JsName> getDuplicateMap() {
return duplicateOriginalMap;
@@ -47,13 +88,9 @@
@Override
public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
/*
- * At this point, unpruned zero-arg functions with empty
- * bodies are Js constructor seed functions.
- * If constructors are ever inlined into seed functions, revisit this.
* Don't process anonymous functions.
*/
- if (x.getName() != null && x.getParameters().size() > 0 ||
- x.getBody().getStatements().size() > 0) {
+ if (x.getName() != null) {
String fnSource = x.toSource();
String body = fnSource.substring(fnSource.indexOf("("));
JsName original = uniqueBodies.get(body);
@@ -63,33 +100,51 @@
uniqueBodies.put(body, x.getName());
}
}
- return false;
+ return true;
+ }
+
+ @Override
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ if (x.getQualifier() instanceof JsNameRef) {
+ invocationQualifiers.push((JsNameRef) x.getQualifier());
+ }
+ return true;
}
}
private class ReplaceDuplicateInvocationNameRefs extends JsModVisitor {
- private Map<JsName, JsName> duplicateMap;
- public ReplaceDuplicateInvocationNameRefs(
- Map<JsName, JsName> duplicateMap) {
+ private final Set<JsName> blacklist;
+
+ private final Map<JsName, JsName> duplicateMap;
+
+ public ReplaceDuplicateInvocationNameRefs(Map<JsName, JsName> duplicateMap,
+ Set<JsName> blacklist) {
this.duplicateMap = duplicateMap;
+ this.blacklist = blacklist;
}
@Override
public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
JsName orig = duplicateMap.get(x.getName());
- if (orig != null && x.getName() != null &&
- x.getName().getEnclosing() == program.getScope()) {
+ if (orig != null && x.getName() != null
+ && x.getName().getEnclosing() == program.getScope()
+ && !blacklist.contains(x.getName()) && !blacklist.contains(orig)) {
ctx.replaceMe(orig.makeRef(x.getSourceInfo()));
}
}
}
+ // Needed for OptimizerTestBase
+ public static boolean exec(JsProgram program) {
+ return new JsDuplicateFunctionRemover(program).execImpl(program.getFragmentBlock(0));
+ }
+
public static boolean exec(JsProgram program, JsBlock fragment) {
return new JsDuplicateFunctionRemover(program).execImpl(fragment);
}
- private JsProgram program;
+ private final JsProgram program;
public JsDuplicateFunctionRemover(JsProgram program) {
this.program = program;
@@ -98,8 +153,8 @@
private boolean execImpl(JsBlock fragment) {
DuplicateFunctionBodyRecorder dfbr = new DuplicateFunctionBodyRecorder();
dfbr.accept(fragment);
- ReplaceDuplicateInvocationNameRefs rdup
- = new ReplaceDuplicateInvocationNameRefs(dfbr.getDuplicateMap());
+ ReplaceDuplicateInvocationNameRefs rdup = new ReplaceDuplicateInvocationNameRefs(
+ dfbr.getDuplicateMap(), dfbr.getBlacklist());
rdup.accept(fragment);
return rdup.didChange();
}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java b/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java
new file mode 100644
index 0000000..9ce5659
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2009 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.js;
+
+/**
+ * Tests the JsStaticEval optimizer.
+ */
+public class JsDuplicateFunctionRemoverTest extends OptimizerTestBase {
+
+ public void testDontRemoveCtors() throws Exception {
+ // As fieldref qualifier
+ assertEquals("function a(){}\n;function b(){}\nb.prototype={};a();b();",
+ optimize("function a(){};function b(){} b.prototype={}; a(); b();"));
+ // As parameter
+ assertEquals(
+ "function defineSeed(a,b){}\n;function a(){}\n;function b(){}\ndefineSeed(a,b);a();b();",
+ optimize("function defineSeed(a,b){};function a(){};function b(){} defineSeed(a,b); a(); b();"));
+ }
+
+ public void testRemoveDuplicates() throws Exception {
+ assertEquals("function a(){}\n;a();a();",
+ optimize("function a(){};function b(){} a(); b();"));
+ }
+
+ private String optimize(String js) throws Exception {
+ return optimize(js, JsSymbolResolver.class,
+ JsDuplicateFunctionRemover.class, JsUnusedFunctionRemover.class);
+ }
+}
\ No newline at end of file
