This is working towards issue #699. Historically, we have always hung static fields directly on the window object, which works fine in the iframe environment. Now that we need to run in the outer page, that won't work so well.
- Use "var" decls for all static fields
- No longer ref static fields off the window object, either implicitly or explicitly
- When the static field needs a clinit call, a comma expression is formed
- When the resulting comma expression is the lhs of an assignment, some juggling is performed to avoid an illegal state.
Review by: mmendez
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@618 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
index 340dd9d..2f04892 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -400,8 +400,6 @@
private final JsName prototype = objectScope.declareName("prototype");
- private final JsName window = jsProgram.getRootScope().declareName("window");
-
{
globalTemp.setObfuscatable(false);
prototype.setObfuscatable(false);
@@ -443,7 +441,31 @@
&& x.getLhs().getType() instanceof JReferenceType
&& x.getRhs().getType() instanceof JReferenceType) {
myOp = JsBinaryOperator.REF_NEQ;
+ }
+
+ /*
+ * Due to the way clinits are constructed, you can end up with a comma
+ * operation on the lhs, which is illegal. Juggle things to put the
+ * operator and rhs inside of the comma expression.
+ */
+ if (myOp.isAssignment() && (lhs instanceof JsBinaryOperation)) {
+ // Find the rightmost comma operation
+ JsBinaryOperation curLhs = (JsBinaryOperation) lhs;
+ assert (curLhs.getOperator() == JsBinaryOperator.COMMA);
+ while (curLhs.getArg2() instanceof JsBinaryOperation) {
+ curLhs = (JsBinaryOperation) curLhs.getArg2();
+ assert (curLhs.getOperator() == JsBinaryOperator.COMMA);
+ }
+ // curLhs is now the rightmost comma operation; create an assignment to
+ // the last arg from this binary operation's rhs
+ JsBinaryOperation asg = new JsBinaryOperation(myOp, curLhs.getArg2(), rhs);
+ // replace the rightmost comma expression's last arg with the asg
+ curLhs.setArg2(asg);
+ // repush the entire comma expression
+ push(lhs);
+ return;
}
+
JsBinaryOperation binOp = new JsBinaryOperation(myOp, lhs, rhs);
push(binOp);
}
@@ -546,9 +568,19 @@
}
// setup fields
+ JsVars vars = new JsVars();
for (int i = 0; i < jsFields.size(); ++i) {
- JsStatement stmt = (JsStatement) jsFields.get(i);
- globalStmts.add(stmt);
+ JsNode node = (JsNode) jsFields.get(i);
+ if (node instanceof JsVar) {
+ vars.add((JsVar) node);
+ } else {
+ assert (node instanceof JsStatement);
+ JsStatement stmt = (JsStatement) jsFields.get(i);
+ globalStmts.add(stmt);
+ }
+ }
+ if (!vars.isEmpty()) {
+ globalStmts.add(vars);
}
}
@@ -599,56 +631,69 @@
// @Override
public void endVisit(JField x, Context ctx) {
- if (x.hasInitializer() && x.constInitializer == null) {
- // do nothing
- push(null);
- return;
- }
-
// if we need an initial value, create an assignment
if (x.constInitializer != null) {
+ // setup the constant value
accept(x.constInitializer);
- } else {
+ } else if (!x.hasInitializer()) {
+ // setup a default value
accept(x.getType().getDefaultValue());
+ } else {
+ // the variable is setup during clinit, no need to initialize here
+ push(null);
}
+ JsExpression rhs = (JsExpression) pop();
+ JsName name = getName(x);
- JsNameRef fieldRef = getName(x).makeRef();
- if (!x.isStatic()) {
- fieldRef.setQualifier(globalTemp.makeRef());
+ if (x.isStatic()) {
+ // setup a var for the static
+ JsVar var = new JsVar(name);
+ var.setInitExpr(rhs);
+ push(var);
+ } else {
+ // for non-statics, only setup an assignment if needed
+ if (rhs != null) {
+ JsNameRef fieldRef = name.makeRef();
+ fieldRef.setQualifier(globalTemp.makeRef());
+ JsExpression asg = createAssignment(fieldRef, (JsExpression) pop());
+ push(new JsExprStmt(asg));
+ } else {
+ push(null);
+ }
}
- JsExpression asg = createAssignment(fieldRef, (JsExpression) pop());
- push(new JsExprStmt(asg));
}
// @Override
public void endVisit(JFieldRef x, Context ctx) {
- JsName jsFieldName = getName(x.getField());
+ JField field = x.getField();
+ JsName jsFieldName = getName(field);
JsNameRef nameRef = jsFieldName.makeRef();
- JsExpression qualifier = null;
+ JsExpression curExpr = nameRef;
+
+ /*
+ * Note: the comma expressions here would cause an illegal tree state if
+ * the result expression ended up on the lhs of an assignment. A hack in
+ * in endVisit(JBinaryOperation) rectifies the situation.
+ */
+
+ // See if we need a clinit
+ JsInvocation jsInvocation = maybeCreateClinitCall(field);
+ if (jsInvocation != null) {
+ curExpr = createCommaExpression(jsInvocation, curExpr);
+ }
if (x.getInstance() != null) {
- qualifier = (JsExpression) pop();
+ JsExpression qualifier = (JsExpression) pop();
+ if (field.isStatic()) {
+ // unnecessary qualifier, create a comma expression
+ curExpr = createCommaExpression(qualifier, curExpr);
+ } else {
+ // necessary qualifier, qualify the name ref
+ nameRef.setQualifier(qualifier);
+ }
}
- JField field = x.getField();
- if (field.isStatic()) {
- JsExpression realQualifier = maybeCreateClinitCall(field);
- if (qualifier != null) {
- // There is an unused qualifier. We must create a comma expression
- // since there's no way to reference the field from the qualifier.
- if (realQualifier == null) {
- // ugh, must synthesize a call to window to be the real qualifier
- // TODO: this will BREAK when we use inline JS
- realQualifier = topScope.findExistingUnobfuscatableName("window").makeRef();
- }
- realQualifier = createCommaExpression(qualifier, realQualifier);
- }
- // the name ref's qualifier is either a clinit, or nothing
- nameRef.setQualifier(realQualifier);
- } else {
- nameRef.setQualifier(qualifier); // instance
- }
- push(nameRef);
+ push(curExpr);
}
// @Override
@@ -734,9 +779,14 @@
}
// setup fields
+ JsVars vars = new JsVars();
for (int i = 0; i < jsFields.size(); ++i) {
- JsStatement stmt = (JsStatement) jsFields.get(i);
- globalStmts.add(stmt);
+ JsNode node = (JsNode) jsFields.get(i);
+ assert (node instanceof JsVar);
+ vars.add((JsVar) node);
+ }
+ if (!vars.isEmpty()) {
+ globalStmts.add(vars);
}
}
@@ -839,7 +889,7 @@
}
}
- if (vars.iterator().hasNext()) {
+ if (!vars.isEmpty()) {
jsFunc.getBody().getStatements().add(0, vars);
}
@@ -1289,12 +1339,8 @@
private void generateNullFunc(JsStatements globalStatements) {
// handle null method
- // return 'window' so we can reference global fields from the invocation
- JsReturn jsReturn = new JsReturn(window.makeRef());
- JsBlock body = new JsBlock();
- body.getStatements().add(jsReturn);
JsFunction nullFunc = new JsFunction(topScope, nullMethodName);
- nullFunc.setBody(body);
+ nullFunc.setBody(new JsBlock());
globalStatements.add(nullFunc.makeStmt());
}
@@ -1443,10 +1489,6 @@
JsExpression asg = createAssignment(clinitFunc.getName().makeRef(),
nullMethodName.makeRef());
clinitFunc.getBody().getStatements().add(0, asg.makeStmt());
-
- // return 'window' so that fields can be referenced
- JsReturn jsReturn = new JsReturn(window.makeRef());
- clinitFunc.getBody().getStatements().add(jsReturn);
}
private JsInvocation maybeCreateClinitCall(JField x) {
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
index 5bc8ad8..54eaf3e 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithAllVisits.java
@@ -57,6 +57,7 @@
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsVisitor;
import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
/**
* Implements stubs for the <code>endVisit()</code> and <code>visit()</code>interface
@@ -181,6 +182,9 @@
public void endVisit(JsTry x) {
}
+ public void endVisit(JsVar x) {
+ }
+
public void endVisit(JsVars x) {
}
@@ -343,6 +347,10 @@
return true;
}
+ public boolean visit(JsVar x) {
+ return true;
+ }
+
public boolean visit(JsVars x) {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
index abf78e5..b4096c1 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithEndVisits.java
@@ -57,6 +57,7 @@
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsVisitor;
import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
/**
* Implements stubs for the <code>endVisit()</code> interface methods.
@@ -180,6 +181,9 @@
public void endVisit(JsTry x) {
}
+ public void endVisit(JsVar x) {
+ }
+
public void endVisit(JsVars x) {
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
index 4fd389d..56017f0 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsAbstractVisitorWithVisits.java
@@ -57,6 +57,7 @@
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsVisitor;
import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
/**
* Implements stubs for the <code>visit()</code> interface methods.
@@ -219,6 +220,10 @@
return true;
}
+ public boolean visit(JsVar x) {
+ return true;
+ }
+
public boolean visit(JsVars x) {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
index 1f41312..82bccb2 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsPrecedenceVisitor.java
@@ -57,6 +57,7 @@
import com.google.gwt.dev.js.ast.JsTry;
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
/**
* Precedence indices from "JavaScript - The Definitive Guide" 4th Edition (page
@@ -272,6 +273,10 @@
throw new RuntimeException("Only expressions have precedence.");
}
+ public boolean visit(JsVar x) {
+ throw new RuntimeException("Only expressions have precedence.");
+ }
+
public boolean visit(JsVars x) {
throw new RuntimeException("Only expressions have precedence.");
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
index 3e2d395..1bf7883 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -62,6 +62,7 @@
import com.google.gwt.dev.js.ast.JsUnaryOperator;
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsWhile;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
import com.google.gwt.dev.util.TextOutput;
import java.util.Iterator;
@@ -708,6 +709,20 @@
return false;
}
+ public boolean visit(JsVar x) {
+ _nameOf(x);
+ JsExpression initExpr = x.getInitExpr();
+ if (initExpr != null) {
+ _spaceOpt();
+ _assignment();
+ _spaceOpt();
+ _parenPushIfCommaExpr(initExpr);
+ initExpr.traverse(this);
+ _parenPopIfCommaExpr(initExpr);
+ }
+ return false;
+ }
+
public boolean visit(JsVars x) {
_var();
_space();
@@ -715,16 +730,7 @@
for (Iterator iter = x.iterator(); iter.hasNext();) {
sep = _sepCommaOptSpace(sep);
JsVars.JsVar var = (JsVars.JsVar) iter.next();
- _nameOf(var);
- JsExpression initExpr = var.getInitExpr();
- if (initExpr != null) {
- _spaceOpt();
- _assignment();
- _spaceOpt();
- _parenPushIfCommaExpr(initExpr);
- initExpr.traverse(this);
- _parenPopIfCommaExpr(initExpr);
- }
+ var.traverse(this);
}
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
index 98453ce..40e37a0 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperator.java
@@ -91,6 +91,14 @@
super(symbol, precedence, mask);
}
+ public boolean isAssignment() {
+ /*
+ * Beware, flaky! Maybe I should have added Yet Another Field to
+ * BinaryOperator?
+ */
+ return (getPrecedence() == ASG.getPrecedence());
+ }
+
public boolean isKeyword() {
return this == INSTANCEOF || this == INOP;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsVars.java b/dev/core/src/com/google/gwt/dev/js/ast/JsVars.java
index e4c418f..3976d82 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsVars.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsVars.java
@@ -27,7 +27,7 @@
/**
* A var declared using the JavaScript <code>var</code> statement.
*/
- public static class JsVar implements HasName {
+ public static class JsVar extends JsNode implements HasName {
private JsExpression initExpr;
@@ -48,6 +48,15 @@
public void setInitExpr(JsExpression initExpr) {
this.initExpr = initExpr;
}
+
+ public void traverse(JsVisitor v) {
+ if (v.visit(this)) {
+ if (initExpr != null) {
+ initExpr.traverse(v);
+ }
+ }
+ v.endVisit(this);
+ }
}
private final List vars = new ArrayList();
@@ -59,6 +68,10 @@
vars.add(var);
}
+ public boolean isEmpty() {
+ return vars.isEmpty();
+ }
+
// Iterator returns JsVar objects
public Iterator iterator() {
return vars.iterator();
@@ -68,10 +81,7 @@
if (v.visit(this)) {
for (Iterator iter = vars.iterator(); iter.hasNext();) {
JsVar var = (JsVar) iter.next();
- JsExpression expr = var.getInitExpr();
- if (expr != null) {
- expr.traverse(v);
- }
+ var.traverse(v);
}
}
v.endVisit(this);
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java b/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
index df4ebee..897810d 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsVisitor.java
@@ -15,6 +15,8 @@
*/
package com.google.gwt.dev.js.ast;
+import com.google.gwt.dev.js.ast.JsVars.JsVar;
+
/**
* Implemented by nodes that will visit child nodes.
*/
@@ -98,6 +100,8 @@
void endVisit(JsTry x);
+ void endVisit(JsVar var);
+
void endVisit(JsVars x);
void endVisit(JsWhile x);
@@ -180,6 +184,8 @@
boolean visit(JsTry x);
+ boolean visit(JsVar var);
+
boolean visit(JsVars x);
boolean visit(JsWhile x);
diff --git a/eclipse/settings/english.dictionary b/eclipse/settings/english.dictionary
index f75a384..f3d3628 100644
--- a/eclipse/settings/english.dictionary
+++ b/eclipse/settings/english.dictionary
@@ -47228,14 +47228,16 @@
servlet
deprecated
recognize
-minimalist
-cacheable
-initializing
-superclasses
-superinterfaces
-overridable
-subpackage
-generalized
-classpath
-subpackages
-slurp
+minimalist
+cacheable
+initializing
+superclasses
+superinterfaces
+overridable
+subpackage
+generalized
+classpath
+subpackages
+slurp
+rectifies
+initialize
