Revision: 7776
Author: [email protected]
Date: Tue Mar 23 14:56:21 2010
Log: Allow static-dispatch style JS .call invocations to be inlined.
This is particularly relevant because super()/this() calls get translated
as .call() ops, and this patch lets us inline them into the calling
constructor.
http://gwt-code-reviews.appspot.com/201801/show
Review by: spoon
http://code.google.com/p/google-web-toolkit/source/detail?r=7776
Modified:
/trunk/dev/core/src/com/google/gwt/dev/js/JsHoister.java
/trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsHoister.java Thu Mar 11
17:23:20 2010
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsHoister.java Tue Mar 23
14:56:21 2010
@@ -221,16 +221,9 @@
stack.push(x);
}
- /**
- * A "this" reference can only effectively be hoisted if the call site
is
- * qualified by a JsNameRef, so we'll ignore this for now.
- */
@Override
public void endVisit(JsThisRef x, JsContext<JsExpression> ctx) {
- // Set a flag to indicate that we cannot continue, and push a null so
- // we don't run out of elements on the stack.
- successful = false;
- stack.push(null);
+ stack.push(new JsThisRef(x.getSourceInfo()));
}
public JsExpression getExpression() {
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Fri Oct 16
16:16:55 2009
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Tue Mar 23
14:56:21 2010
@@ -136,7 +136,7 @@
*/
affectedBySideEffects = true;
}
-
+
@Override
public void endVisit(JsObjectLiteral x, JsContext<JsExpression> ctx) {
affectedBySideEffects = true;
@@ -600,6 +600,9 @@
* invocations occur.
*/
private static class EvaluationOrderVisitor extends JsVisitor {
+ public static final JsName THIS_NAME = (new JsScope("fake scope") {
+ }).declareName("this");
+
private boolean maintainsOrder = true;
private final List<JsName> toEvaluate;
private final List<JsName> unevaluated;
@@ -673,8 +676,19 @@
@Override
public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
- JsName name = x.getName();
-
+ checkName(x.getName());
+ }
+
+ @Override
+ public void endVisit(JsThisRef x, JsContext<JsExpression> ctx) {
+ checkName(THIS_NAME);
+ }
+
+ public boolean maintainsOrder() {
+ return maintainsOrder && unevaluated.size() == 0;
+ }
+
+ private void checkName(JsName name) {
if (!toEvaluate.contains(name)) {
return;
}
@@ -683,10 +697,6 @@
maintainsOrder = false;
}
}
-
- public boolean maintainsOrder() {
- return maintainsOrder && unevaluated.size() == 0;
- }
/**
* Determine if an expression contains a reference to a strict
parameter.
@@ -913,6 +923,7 @@
}
inlining.push(invokedFunction);
+ x = tryToUnravelExplicitCall(x);
JsExpression op = process(x, callerFunction, invokedFunction);
if (x != op) {
@@ -1030,6 +1041,7 @@
List<JsExpression> hoisted = new ArrayList<JsExpression>(
statements.size());
+ JsExpression thisExpr = ((JsNameRef)
x.getQualifier()).getQualifier();
List<JsName> localVariableNames = new ArrayList<JsName>();
boolean sawReturnStatement = false;
@@ -1102,13 +1114,14 @@
}
// Confirm that the expression conforms to the desired heuristics
- if (!isInlinable(program, callerFunction, invokedFunction,
+ if (!isInlinable(program, callerFunction, invokedFunction, thisExpr,
x.getArguments(), op)) {
return x;
}
// Perform the name replacement
- NameRefReplacerVisitor v = new NameRefReplacerVisitor(x,
invokedFunction);
+ NameRefReplacerVisitor v = new NameRefReplacerVisitor(thisExpr,
+ x.getArguments(), invokedFunction.getParameters());
for (ListIterator<JsName> nameIterator =
localVariableNames.listIterator(); nameIterator.hasNext();) {
JsName name = nameIterator.next();
@@ -1180,21 +1193,12 @@
@Override
public void endVisit(JsInvocation x, JsContext<JsExpression> ctx) {
- JsFunction function = isFunction(x.getQualifier());
- if (function != null) {
- Integer count = invocationCount.get(function);
- if (count == null) {
- assert (!removingCounts);
- count = 1;
- } else {
- if (removingCounts) {
- count -= 1;
- } else {
- count += 1;
- }
- }
- invocationCount.put(function, count);
- }
+ checkFunctionCall(x.getQualifier());
+ }
+
+ @Override
+ public void endVisit(JsNew x, JsContext<JsExpression> ctx) {
+ checkFunctionCall(x.getConstructorExpression());
}
public Integer invocationCount(JsFunction f) {
@@ -1210,6 +1214,24 @@
accept(expr);
removingCounts = false;
}
+
+ private void checkFunctionCall(JsExpression qualifier) {
+ JsFunction function = isFunction(qualifier);
+ if (function != null) {
+ Integer count = invocationCount.get(function);
+ if (count == null) {
+ assert (!removingCounts);
+ count = 1;
+ } else {
+ if (removingCounts) {
+ count -= 1;
+ } else {
+ count += 1;
+ }
+ }
+ invocationCount.put(function, count);
+ }
+ }
}
/**
@@ -1228,15 +1250,13 @@
final Map<JsName, JsExpression> paramsToArgsMap = new
IdentityHashMap<JsName, JsExpression>();
/**
- * Constructor.
- *
- * @param invocation The call site
- * @param function The function that encloses the inlined statement
+ * A replacement expression for this references.
*/
- public NameRefReplacerVisitor(JsInvocation invocation, JsFunction
function) {
- List<JsParameter> parameters = function.getParameters();
- List<JsExpression> arguments = invocation.getArguments();
-
+ private JsExpression thisExpr;
+
+ public NameRefReplacerVisitor(JsExpression thisExpr,
+ List<JsExpression> arguments, List<JsParameter> parameters) {
+ this.thisExpr = thisExpr;
if (parameters.size() != arguments.size()) {
// This shouldn't happen if the cloned JsInvocation has been
properly
// configured
@@ -1269,6 +1289,12 @@
ctx.replaceMe(replacement);
}
}
+
+ @Override
+ public void endVisit(JsThisRef x, JsContext<JsExpression> ctx) {
+ assert thisExpr != null;
+ ctx.replaceMe(thisExpr);
+ }
/**
* Set a replacement JsName for all references to a JsName.
@@ -1753,6 +1779,13 @@
private static JsFunction isFunction(JsExpression e) {
if (e instanceof JsNameRef) {
JsNameRef ref = (JsNameRef) e;
+
+ // Unravel foo.call(...).
+ if (!ref.getName().isObfuscatable()
&& "call".equals(ref.getIdent())) {
+ if (ref.getQualifier() instanceof JsNameRef) {
+ ref = (JsNameRef) ref.getQualifier();
+ }
+ }
JsNode staticRef = ref.getName().getStaticRef();
if (staticRef instanceof JsFunction) {
@@ -1767,7 +1800,8 @@
* Determine if a statement can be inlined into a call site.
*/
private static boolean isInlinable(JsProgram program, JsFunction caller,
- JsFunction callee, List<JsExpression> arguments, JsNode<?> toInline)
{
+ JsFunction callee, JsExpression thisExpr, List<JsExpression>
arguments,
+ JsNode<?> toInline) {
/*
* This will happen with varargs-style JavaScript functions that rely
on the
@@ -1809,19 +1843,31 @@
if (hasCommonIdents(arguments, toInline, parameterIdents)) {
return false;
}
+
+ List<JsExpression> evalArgs;
+ if (thisExpr == null) {
+ evalArgs = arguments;
+ } else {
+ evalArgs = new ArrayList<JsExpression>(1 + arguments.size());
+ evalArgs.add(thisExpr);
+ evalArgs.addAll(arguments);
+ }
/*
* Determine if the evaluation of the invocation's arguments may
create side
* effects. This will determine how aggressively the parameters may be
* reordered.
*/
- if (isVolatile(program, arguments, caller)) {
+ if (isVolatile(program, evalArgs, caller)) {
/*
* Determine the order in which the parameters must be evaluated.
This
* will vary between call sites, based on whether or not the
invocation's
* arguments can be repeated without ill effect.
*/
List<JsName> requiredOrder = new ArrayList<JsName>();
+ if (thisExpr != null && isVolatile(program, thisExpr, callee)) {
+ requiredOrder.add(EvaluationOrderVisitor.THIS_NAME);
+ }
for (int i = 0; i < arguments.size(); i++) {
JsExpression e = arguments.get(i);
JsParameter p = callee.getParameters().get(i);
@@ -1887,6 +1933,34 @@
return hasSideEffects(list)
|| affectedBySideEffects(program, list, context);
}
+
+ /**
+ * Transforms any <code>foo.call(this)</code> into
<code>this.foo()</code> to
+ * be compatible with our inlining algorithm.
+ */
+ private static JsInvocation tryToUnravelExplicitCall(JsInvocation x) {
+ if (!(x.getQualifier() instanceof JsNameRef)) {
+ return x;
+ }
+ JsNameRef ref = (JsNameRef) x.getQualifier();
+ if (ref.getName().isObfuscatable() || !"call".equals(ref.getIdent())) {
+ return x;
+ }
+ List<JsExpression> oldArgs = x.getArguments();
+ if (oldArgs.size() < 1) {
+ return x;
+ }
+
+ JsNameRef oldTarget = (JsNameRef) ref.getQualifier();
+ JsNameRef newTarget = new JsNameRef(oldTarget.getSourceInfo(),
+ oldTarget.getName());
+ newTarget.setQualifier(oldArgs.get(0));
+ JsInvocation newCall = new JsInvocation(x.getSourceInfo());
+ newCall.setQualifier(newTarget);
+ // Don't have to clone because the returned invocation is transient.
+ newCall.getArguments().addAll(oldArgs.subList(1, oldArgs.size()));
+ return newCall;
+ }
/**
* Utility class.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
To unsubscribe from this group, send email to
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with
the words "REMOVE ME" as the subject.
