Compiler now prunes unused parameters!
Patch by: mmastrac
Review by: me
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1494 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java b/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
index 24f3df6..30f90c9 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java
@@ -31,9 +31,12 @@
import com.google.gwt.dev.jjs.ast.js.JsniMethodBody;
import com.google.gwt.dev.js.JsParser;
import com.google.gwt.dev.js.JsParserException;
+import com.google.gwt.dev.js.JsAbstractSymbolResolver;
import com.google.gwt.dev.js.JsParserException.SourceDetail;
import com.google.gwt.dev.js.ast.JsExprStmt;
import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsName;
+import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsProgram;
import com.google.gwt.dev.js.ast.JsStatement;
@@ -582,6 +585,11 @@
JsFunction jsFunction = (JsFunction) jsExprStmt.getExpression();
jsFunction.setFromJava(true);
((JsniMethodBody) newMethod.getBody()).setFunc(jsFunction);
+
+ // Ensure that we've resolved the parameter and local references within
+ // the JSNI method for later pruning.
+ JsParameterResolver localResolver = new JsParameterResolver(jsFunction);
+ localResolver.accept(jsFunction);
} catch (IOException e) {
throw new InternalCompilerException(
"Internal error parsing JSNI in method '" + newMethod
@@ -756,6 +764,31 @@
}
}
+ /**
+ * Resolves the scope of JS identifiers solely within the scope of a method.
+ */
+ private static class JsParameterResolver extends JsAbstractSymbolResolver {
+ private final JsFunction jsFunction;
+
+ public JsParameterResolver(JsFunction jsFunction) {
+ this.jsFunction = jsFunction;
+ }
+
+ @Override
+ public void resolve(JsNameRef x) {
+ // Only resolve unqualified names
+ if (x.getQualifier() == null) {
+ JsName name = getScope().findExistingName(x.getIdent());
+
+ // Ensure that we're resolving a name from the function's parameters
+ if (name != null
+ && jsFunction.getParameters().contains(name.getStaticRef())) {
+ x.resolve(name);
+ }
+ }
+ }
+ }
+
public static TypeDeclaration[] exec(TypeMap typeMap,
CompilationUnitDeclaration[] unitDecls, JsProgram jsProgram) {
createPeersForTypes(unitDecls, typeMap);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
index 5832632..26c71f6 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
@@ -46,23 +46,33 @@
import com.google.gwt.dev.jjs.ast.JVariable;
import com.google.gwt.dev.jjs.ast.JVariableRef;
import com.google.gwt.dev.jjs.ast.JVisitor;
+import com.google.gwt.dev.jjs.ast.js.JMultiExpression;
import com.google.gwt.dev.jjs.ast.js.JsniFieldRef;
+import com.google.gwt.dev.jjs.ast.js.JsniMethodBody;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
+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.JsName;
+import com.google.gwt.dev.js.ast.JsNameRef;
+import com.google.gwt.dev.js.ast.JsVisitor;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
- * Remove globally unreferenced classes, interfaces, methods, and fields from
- * the AST. This algorithm is based on having known "entry points" into the
- * application which serve as the root(s) from which reachability is determined
- * and everything else is rescued. Pruner determines reachability at a global
- * level based on method calls and new operations; it does not perform any local
- * code flow analysis. But, a local code flow optimization pass that can
- * eliminate method calls would allow Pruner to prune additional nodes.
+ * Remove globally unreferenced classes, interfaces, methods, parameters, and
+ * fields from the AST. This algorithm is based on having known "entry points"
+ * into the application which serve as the root(s) from which reachability is
+ * determined and everything else is rescued. Pruner determines reachability at
+ * a global level based on method calls and new operations; it does not perform
+ * any local code flow analysis. But, a local code flow optimization pass that
+ * can eliminate method calls would allow Pruner to prune additional nodes.
*
* Note: references to pruned types may still exist in the tree after this pass
* runs, however, it should only be in contexts that do not rely on any code
@@ -70,13 +80,13 @@
* a pruned type, or to try to cast to a pruned type. These will cause natural
* failures at run time; or later optimizations might be able to hard-code
* failures at compile time.
+ *
+ * Note: this class is limited to pruning parameters of static methods only.
*/
public class Pruner {
/**
- * Remove assignments to pruned fields and locals.
- *
- * TODO(later): prune params too
+ * Remove assignments to pruned fields, locals and params.
*/
private class CleanupRefsVisitor extends JModVisitor {
@@ -88,7 +98,8 @@
if (lhs.hasSideEffects()) {
return;
}
- if (lhs instanceof JFieldRef || lhs instanceof JLocalRef) {
+ if (lhs instanceof JFieldRef || lhs instanceof JLocalRef
+ || lhs instanceof JParameterRef) {
JVariable var = ((JVariableRef) lhs).getTarget();
if (!referencedNonTypes.contains(var)) {
// Just replace with my RHS
@@ -97,6 +108,58 @@
}
}
}
+
+ @Override
+ public void endVisit(JMethodCall x, Context ctx) {
+ JMethod method = x.getTarget();
+
+ // Did we prune the parameters of the method we're calling?
+ if (removedMethodParamsMap.containsKey(method)) {
+ // This must be a static method
+ assert method.isStatic();
+ assert x.getInstance() == null;
+
+ JMethodCall newCall = new JMethodCall(program, x.getSourceInfo(), null,
+ method);
+
+ JMultiExpression currentMulti = null;
+
+ ArrayList<JParameter> removedParams = removedMethodParamsMap.get(method);
+
+ for (int i = 0; i < x.getArgs().size(); i++) {
+ JParameter param = removedParams.get(i);
+ JExpression arg = x.getArgs().get(i);
+
+ if (referencedNonTypes.contains(param)) {
+ // We rescued this parameter
+
+ // Do we need to add the multi we've been building?
+ if (currentMulti == null) {
+ newCall.getArgs().add(arg);
+ } else {
+ currentMulti.exprs.add(arg);
+ newCall.getArgs().add(currentMulti);
+ currentMulti = null;
+ }
+ } else if (arg.hasSideEffects()) {
+ // We didn't rescue this parameter and it has side-effects. Add it
+ // to a new multi
+ if (currentMulti == null) {
+ currentMulti = new JMultiExpression(program, x.getSourceInfo());
+ }
+ currentMulti.exprs.add(arg);
+ }
+ }
+
+ // We have orphaned parameters - add them on the end, wrapped in the
+ // multi (JS ignores extra parameters)
+ if (currentMulti != null) {
+ newCall.getArgs().add(currentMulti);
+ }
+
+ ctx.replaceMe(newCall);
+ }
+ }
}
/**
@@ -173,6 +236,51 @@
}
@Override
+ public boolean visit(JMethod x, Context ctx) {
+ if (x.isStatic()) {
+ /*
+ * Don't prune parameters on unreferenced methods. The methods might not
+ * be reachable through the current method traversal routines, but might
+ * be used or checked elsewhere.
+ *
+ * Basically, if we never actually checked if the method parameters were
+ * used or not, don't prune them. Doing so would leave a number of
+ * dangling JParameterRefs that blow up in later optimizations.
+ */
+ if (!referencedNonTypes.contains(x)) {
+ return true;
+ }
+
+ JsFunction func = x.isNative()
+ ? ((JsniMethodBody) x.getBody()).getFunc() : null;
+
+ ArrayList<JParameter> removedParams = new ArrayList<JParameter>(
+ x.params);
+
+ for (int i = 0; i < x.params.size(); i++) {
+ JParameter param = x.params.get(i);
+
+ if (!referencedNonTypes.contains(param)) {
+ x.params.remove(i);
+
+ didChange = true;
+
+ removedMethodParamsMap.put(x, removedParams);
+
+ // Remove the associated JSNI parameter
+ if (func != null) {
+ func.getParameters().remove(i);
+ }
+
+ i--;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ @Override
public boolean visit(JMethodBody x, Context ctx) {
for (Iterator<JLocal> it = x.locals.iterator(); it.hasNext();) {
JLocal local = it.next();
@@ -287,6 +395,9 @@
} else if (lhs instanceof JLocalRef) {
// locals are ok to skip
doSkip = true;
+ } else if (lhs instanceof JParameterRef) {
+ // parameters are ok to skip
+ doSkip = true;
} else if (lhs instanceof JFieldRef) {
JFieldRef fieldRef = (JFieldRef) lhs;
/*
@@ -303,13 +414,6 @@
} else if (instance instanceof JThisRef) {
// a this ref cannot be null
doSkip = true;
- } else if (instance instanceof JParameterRef) {
- JParameter param = ((JParameterRef) instance).getParameter();
- JMethod enclosingMethod = param.getEnclosingMethod();
- if (program.isStaticImpl(enclosingMethod)
- && enclosingMethod.params.get(0) == param) {
- doSkip = true;
- }
}
}
@@ -404,6 +508,33 @@
}
@Override
+ public boolean visit(final JMethod x, Context ctx) {
+ if (x.isNative()) {
+ // Manually rescue native parameter references
+ final JsniMethodBody body = (JsniMethodBody) x.getBody();
+ final JsFunction func = body.getFunc();
+
+ new JsVisitor() {
+ @Override
+ public void endVisit(JsNameRef nameRef, JsContext<JsExpression> ctx) {
+ JsName ident = nameRef.getName();
+
+ if (ident != null) {
+ // If we're referencing a parameter, rescue the associated
+ // JParameter
+ int index = func.getParameters().indexOf(ident.getStaticRef());
+ if (index != -1) {
+ rescue(x.params.get(index));
+ }
+ }
+ }
+ }.accept(func);
+ }
+
+ return true;
+ }
+
+ @Override
public boolean visit(JMethodCall call, Context ctx) {
JMethod target = call.getTarget();
// JLS 12.4.1: references to static methods rescue the enclosing class
@@ -444,6 +575,13 @@
}
@Override
+ public boolean visit(JParameterRef x, Context ctx) {
+ // rescue the parameter for future pruning purposes
+ rescue(x.getParameter());
+ return true;
+ }
+
+ @Override
public boolean visit(JsniFieldRef x, Context ctx) {
/*
* SPECIAL: this could be an assignment that passes a value from
@@ -467,6 +605,18 @@
for (int i = 0, c = params.size(); i < c; ++i) {
JParameter param = params.get(i);
maybeRescueJavaScriptObjectPassingIntoJava(param.getType());
+
+ /*
+ * Because we're not currently tracking methods through JSNI, we need to
+ * assume that it's not safe to prune parameters of a method referenced
+ * as such.
+ *
+ * A better solution would be to perform basic escape analysis to ensure
+ * that the function reference never escapes, or at minimum, ensure that
+ * the method is immediately called after retrieving the method
+ * reference.
+ */
+ rescue(param);
}
// JsniMethodRef rescues as JMethodCall
return visit((JMethodCall) x, ctx);
@@ -640,10 +790,11 @@
return new Pruner(program, noSpecialTypes).execImpl();
}
- private final boolean saveCodeGenTypes;
private final JProgram program;
private final Set<JNode> referencedNonTypes = new HashSet<JNode>();
private final Set<JReferenceType> referencedTypes = new HashSet<JReferenceType>();
+ private final Map<JMethod, ArrayList<JParameter>> removedMethodParamsMap = new HashMap<JMethod, ArrayList<JParameter>>();
+ private final boolean saveCodeGenTypes;
private JMethod stringValueOfChar = null;
private Pruner(JProgram program, boolean saveCodeGenTypes) {
@@ -681,6 +832,7 @@
referencedTypes.clear();
referencedNonTypes.clear();
+ removedMethodParamsMap.clear();
madeChanges = true;
}
return madeChanges;
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractSymbolResolver.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractSymbolResolver.java
new file mode 100644
index 0000000..b28cfcc
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractSymbolResolver.java
@@ -0,0 +1,91 @@
+/*
+ * Copyright 2007 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;
+
+import com.google.gwt.dev.js.ast.JsCatch;
+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.JsNameRef;
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsScope;
+import com.google.gwt.dev.js.ast.JsVisitor;
+
+import java.util.Stack;
+
+/**
+ * Base class for any recursive resolver classes.
+ */
+public abstract class JsAbstractSymbolResolver extends JsVisitor {
+
+ private final Stack<JsScope> scopeStack = new Stack<JsScope>();
+
+ @Override
+ public void endVisit(JsCatch x, JsContext<JsCatch> ctx) {
+ popScope();
+ }
+
+ @Override
+ public void endVisit(JsFunction x, JsContext<JsExpression> ctx) {
+ popScope();
+ }
+
+ @Override
+ public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
+ if (x.isResolved()) {
+ return;
+ }
+
+ resolve(x);
+ }
+
+ @Override
+ public void endVisit(JsProgram x, JsContext<JsProgram> ctx) {
+ popScope();
+ }
+
+ @Override
+ public boolean visit(JsCatch x, JsContext<JsCatch> ctx) {
+ pushScope(x.getScope());
+ return true;
+ }
+
+ @Override
+ public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
+ pushScope(x.getScope());
+ return true;
+ }
+
+ @Override
+ public boolean visit(JsProgram x, JsContext<JsProgram> ctx) {
+ pushScope(x.getScope());
+ return true;
+ }
+
+ protected JsScope getScope() {
+ return scopeStack.peek();
+ }
+
+ protected abstract void resolve(JsNameRef x);
+
+ private void popScope() {
+ scopeStack.pop();
+ }
+
+ private void pushScope(JsScope scope) {
+ scopeStack.push(scope);
+ }
+}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsSymbolResolver.java b/dev/core/src/com/google/gwt/dev/js/JsSymbolResolver.java
index aad946f..1f0c050 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsSymbolResolver.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsSymbolResolver.java
@@ -15,17 +15,9 @@
*/
package com.google.gwt.dev.js;
-import com.google.gwt.dev.js.ast.JsCatch;
-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.JsName;
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsProgram;
-import com.google.gwt.dev.js.ast.JsScope;
-import com.google.gwt.dev.js.ast.JsVisitor;
-
-import java.util.Stack;
/**
* Resolves any unresolved JsNameRefs.
@@ -35,26 +27,10 @@
/**
* Resolves any unresolved JsNameRefs.
*/
- private class JsResolveSymbolsVisitor extends JsVisitor {
-
- private final Stack<JsScope> scopeStack = new Stack<JsScope>();
+ private class JsResolveSymbolsVisitor extends JsAbstractSymbolResolver {
@Override
- public void endVisit(JsCatch x, JsContext<JsCatch> ctx) {
- popScope();
- }
-
- @Override
- public void endVisit(JsFunction x, JsContext<JsExpression> ctx) {
- popScope();
- }
-
- @Override
- public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
- if (x.isResolved()) {
- return;
- }
-
+ protected void resolve(JsNameRef x) {
JsName name;
String ident = x.getIdent();
if (x.getQualifier() == null) {
@@ -74,41 +50,6 @@
}
x.resolve(name);
}
-
- @Override
- public void endVisit(JsProgram x, JsContext<JsProgram> ctx) {
- popScope();
- }
-
- @Override
- public boolean visit(JsCatch x, JsContext<JsCatch> ctx) {
- pushScope(x.getScope());
- return true;
- }
-
- @Override
- public boolean visit(JsFunction x, JsContext<JsExpression> ctx) {
- pushScope(x.getScope());
- return true;
- }
-
- @Override
- public boolean visit(JsProgram x, JsContext<JsProgram> ctx) {
- pushScope(x.getScope());
- return true;
- }
-
- private JsScope getScope() {
- return scopeStack.peek();
- }
-
- private void popScope() {
- scopeStack.pop();
- }
-
- private void pushScope(JsScope scope) {
- scopeStack.push(scope);
- }
}
public static void exec(JsProgram program) {
