Revision: 9855
Author: jbrosenb...@google.com
Date: Mon Mar 14 12:59:51 2011
Log: Rolling back jsinliner patch, issues have been identified
Review by: zun...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9855
Modified:
/trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java
/trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Mon Mar 14
11:04:13 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Mon Mar 14
12:59:51 2011
@@ -612,25 +612,10 @@
private boolean maintainsOrder = true;
private final List<JsName> toEvaluate;
private final List<JsName> unevaluated;
- private final Set<JsName> paramsOrLocals = new HashSet<JsName>();
-
- public EvaluationOrderVisitor(List<JsName> toEvaluate, JsFunction
callee) {
+
+ public EvaluationOrderVisitor(List<JsName> toEvaluate) {
this.toEvaluate = toEvaluate;
this.unevaluated = new ArrayList<JsName>(toEvaluate);
- // collect params and locals from callee function
- new JsVisitor() {
- @Override
- public void endVisit(JsParameter x, JsContext ctx) {
- paramsOrLocals.add(x.getName());
- }
-
- @Override
- public boolean visit(JsVar x, JsContext ctx) {
- // record this before visiting initializer
- paramsOrLocals.add(x.getName());
- return true;
- }
- }.accept(callee);
}
@Override
@@ -685,7 +670,12 @@
*/
@Override
public void endVisit(JsInvocation x, JsContext ctx) {
- if (unevaluated.size() > 0) {
+ /*
+ * The check for isExecuteOnce() is potentially incorrect here,
however
+ * the original Java semantics of the clinit would have made the code
+ * incorrect anyway.
+ */
+ if ((isExecuteOnce(x) == null) && unevaluated.size() > 0) {
maintainsOrder = false;
}
}
@@ -719,35 +709,11 @@
return maintainsOrder && unevaluated.size() == 0;
}
- /**
- * Check to see if the evaluation of this JsName will break program
order assumptions given
- * the parameters left to be substituted.
- *
- * The cases are as follows:
- * 1) JsName is a function parameter name which has side effects or is
affected by side effects
- * (hereafter called 'volatile'), so it will be in 'toEvaluate'
- * 2) JsName is a function parameter which is not volatile (not in
toEvaluate)
- * 3) JsName is a reference to a global variable
- * 4) JsName is a reference to a local variable
- *
- * A reference to a global while there are still parameters left to
evaluate / substitute
- * implies an order violation.
- *
- * A reference to a volatile parameter is ok if it is the next
parameter in sequence to
- * be evaluated (beginning of unevaluated list). Else, it is either
being evaluated out of
- * order with respect to other parameters, or it is being evaluated
more than once.
- */
private void checkName(JsName name) {
if (!toEvaluate.contains(name)) {
- // if the name is a non-local/non-parameter (e.g. global) and
there are params left to eval
- if (!paramsOrLocals.contains(name) && unevaluated.size() > 0) {
- maintainsOrder = false;
- }
- // else this may be a local, or all volatile params have already
been evaluated, so it's ok.
return;
}
- // either this param is being evaled twice, or out of order
if (unevaluated.size() == 0 || !unevaluated.remove(0).equals(name)) {
maintainsOrder = false;
}
@@ -1648,7 +1614,7 @@
* code to be inlined, but at a cost of larger JS output.
*/
private static final double MAX_COMPLEXITY_INCREASE =
Double.parseDouble(System.getProperty(
- "gwt.jsinlinerRatio", "1.7"));
+ "gwt.jsinlinerRatio", "5.0"));
/**
* Static entry point used by JavaToJavaScriptCompiler.
@@ -1968,7 +1934,7 @@
* order.
*/
EvaluationOrderVisitor orderVisitor = new EvaluationOrderVisitor(
- requiredOrder, callee);
+ requiredOrder);
orderVisitor.accept(toInline);
if (!orderVisitor.maintainsOrder()) {
return false;
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Mon Mar
14 11:04:13 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Mon Mar
14 12:59:51 2011
@@ -83,120 +83,6 @@
+ "function c1() { return ex2 ? a1() :c1(); } c1()";
compare(expected, input);
}
-
- /**
- * Test that a global array reference breaks argument ordering.
- */
- public void testOrderingArrayGlobal() throws Exception {
- StringBuffer code = new StringBuffer();
-
- code.append("var array; ");
- code.append("function clinit() { clinit = null; }");
-
- // callee references array[0] before evaluating argument
- code.append("function callee(arg) { return array[0] + arg; }");
-
- // caller invokes callee with a multi that runs clinit()
- code.append("function caller() { callee((clinit(),2)); }");
-
- // bootstrap the program
- code.append("caller();");
-
- compare(code.toString(), code.toString());
- }
-
- /**
- * Test that a local reference does not break argument ordering.
- */
- public void testOrderingArrayLocal() throws Exception {
- StringBuffer code = new StringBuffer();
-
- code.append("function clinit() { clinit = null; }");
-
- // callee references array[0] before evaluating argument
- code.append("function callee(arg) { var array; return array[0] + arg;
}");
-
- // caller invokes callee with a multi that runs clinit()
- code.append("function caller() { callee((clinit(),2)); }");
-
- // bootstrap the program
- code.append("caller();");
-
- StringBuffer expected = new StringBuffer();
- expected.append("function clinit() { clinit = null; }");
- expected.append("function caller() { var array; array[0] + (clinit(),
2); }");
- expected.append("caller();");
-
- compare(expected.toString(), code.toString());
- }
-
- /**
- * Test that a field reference breaks argument ordering.
- */
- public void testOrderingField() throws Exception {
- StringBuffer code = new StringBuffer();
-
- code.append("function clinit() { clinit = null; }");
-
- // callee references field.x before evaluating argument
- code.append("function callee(arg) { var field; return field.x + arg;
}");
-
- // caller invokes callee with a multi that runs clinit()
- code.append("function caller() { callee((clinit(),2)); }");
-
- // bootstrap the program
- code.append("caller();");
-
- compare(code.toString(), code.toString());
- }
-
- /**
- * Test that a global variable breaks argument ordering.
- */
- public void testOrderingGlobal() throws Exception {
- StringBuffer code = new StringBuffer();
- // A global variable x
- code.append("var x;");
-
- // clinit() sets up x
- code.append("function clinit() { x = 1; clinit = null; }");
-
- // callee references x before evaluating argument
- code.append("function callee(arg) { alert(x); return arg; }");
-
- // caller invokes callee with a multi that runs clinit()
- code.append("function caller() { callee((clinit(),2)); }");
-
- // bootstrap the program
- code.append("caller();");
-
- compare(code.toString(), code.toString());
- }
-
- /**
- * Test that a local variable does not break argument ordering.
- */
- public void testOrderingLocal() throws Exception {
- StringBuffer code = new StringBuffer();
-
- code.append("function clinit() { clinit = null; }");
-
- // callee references y before evaluating argument
- code.append("function callee(arg) { var y; y=2; return arg; }");
-
- // caller invokes callee with a multi that runs clinit()
- code.append("function caller() { return callee((clinit(),3)); }");
-
- // bootstrap the program
- code.append("caller();");
-
- StringBuffer expected = new StringBuffer();
-
- expected.append("function clinit() { clinit = null; }");
- expected.append("function caller() {var y; return y=2,clinit(),3;}");
- expected.append("caller();");
- compare(expected.toString(), code.toString());
- }
/**
* Test that a new expression breaks argument ordering.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
