In r618, I made a change to stop qualifying statics with the window. This required me to do some comma-expression juggling in assignment statements, since a comma expression could now (illegally) be the LHS of an assignment. Well, I totally missed the fact that there is another context where an LHS is needed - modifying unary operations, "++" and "--". This patch implements the juggling in these cases as well. Putting the patch in neatly without duplicating a bunch of code required refactoring JsPrefixOperation and JsPostfixOperation to have a common superclass; most likely it should have always been that way.
While I was at it, I went ahead and reflected the refactorings into the Java AST, even though it's not critical for this patch.
Review by: mmendez
alex.tkachman
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@728 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java
index 05c55d8..c78e2e0 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -16,41 +16,18 @@
package com.google.gwt.dev.jjs.ast;
/**
- * Java postfix expression.
+ * Java postfix operation expression.
*/
-public class JPostfixOperation extends JExpression {
-
- private JExpression arg;
- private final JUnaryOperator op;
+public class JPostfixOperation extends JUnaryOperation {
public JPostfixOperation(JProgram program, JSourceInfo info,
JUnaryOperator op, JExpression arg) {
- super(program, info);
- this.op = op;
- this.arg = arg;
- }
-
- public JExpression getArg() {
- return arg;
- }
-
- public JUnaryOperator getOp() {
- return op;
- }
-
- public JType getType() {
- // Unary operators don't change the type of their expression
- return arg.getType();
- }
-
- public boolean hasSideEffects() {
- return op == JUnaryOperator.DEC || op == JUnaryOperator.INC
- || arg.hasSideEffects();
+ super(program, info, op, arg);
}
public void traverse(JVisitor visitor, Context ctx) {
if (visitor.visit(this, ctx)) {
- arg = visitor.accept(arg);
+ super.traverse(visitor, ctx);
}
visitor.endVisit(this, ctx);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java
index e550cbd..a82df68 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -18,39 +18,16 @@
/**
* Java prefix operation expression.
*/
-public class JPrefixOperation extends JExpression {
+public class JPrefixOperation extends JUnaryOperation {
- private JExpression arg;
- private final JUnaryOperator op;
-
- public JPrefixOperation(JProgram program, JSourceInfo info, JUnaryOperator op,
- JExpression arg) {
- super(program, info);
- this.op = op;
- this.arg = arg;
- }
-
- public JExpression getArg() {
- return arg;
- }
-
- public JUnaryOperator getOp() {
- return op;
- }
-
- public JType getType() {
- // Unary operators don't change the type of their expression
- return arg.getType();
- }
-
- public boolean hasSideEffects() {
- return getOp() == JUnaryOperator.DEC || getOp() == JUnaryOperator.INC
- || arg.hasSideEffects();
+ public JPrefixOperation(JProgram program, JSourceInfo info,
+ JUnaryOperator op, JExpression arg) {
+ super(program, info, op, arg);
}
public void traverse(JVisitor visitor, Context ctx) {
if (visitor.visit(this, ctx)) {
- arg = visitor.accept(arg);
+ super.traverse(visitor, ctx);
}
visitor.endVisit(this, ctx);
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperation.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperation.java
new file mode 100644
index 0000000..068f3cb
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperation.java
@@ -0,0 +1,53 @@
+/*
+ * 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.jjs.ast;
+
+/**
+ * Java prefix or postfix operation expression.
+ */
+public abstract class JUnaryOperation extends JExpression {
+
+ private JExpression arg;
+ private final JUnaryOperator op;
+
+ public JUnaryOperation(JProgram program, JSourceInfo info,
+ JUnaryOperator op, JExpression arg) {
+ super(program, info);
+ this.op = op;
+ this.arg = arg;
+ }
+
+ public JExpression getArg() {
+ return arg;
+ }
+
+ public JUnaryOperator getOp() {
+ return op;
+ }
+
+ public JType getType() {
+ // Unary operators don't change the type of their expression
+ return arg.getType();
+ }
+
+ public boolean hasSideEffects() {
+ return getOp().isModifying() || arg.hasSideEffects();
+ }
+
+ public void traverse(JVisitor visitor, Context ctx) {
+ arg = visitor.accept(arg);
+ }
+}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java
index 32a2db5..d66851d 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -36,6 +36,10 @@
return symbol;
}
+ public boolean isModifying() {
+ return this == INC || this == DEC;
+ }
+
public String toString() {
return new String(getSymbol());
}
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 c3f8d69..cb5ddce 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -123,6 +123,7 @@
import com.google.gwt.dev.js.ast.JsThisRef;
import com.google.gwt.dev.js.ast.JsThrow;
import com.google.gwt.dev.js.ast.JsTry;
+import com.google.gwt.dev.js.ast.JsUnaryOperation;
import com.google.gwt.dev.js.ast.JsUnaryOperator;
import com.google.gwt.dev.js.ast.JsVars;
import com.google.gwt.dev.js.ast.JsWhile;
@@ -441,32 +442,14 @@
&& 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);
+
+ if (maybeShuffleModifyingBinary(binOp)) {
+ return;
+ }
+
push(binOp);
}
@@ -675,7 +658,7 @@
* 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) {
@@ -983,14 +966,26 @@
// @Override
public void endVisit(JPostfixOperation x, Context ctx) {
- push(new JsPostfixOperation(JavaToJsOperatorMap.get(x.getOp()),
- (JsExpression) pop())); // arg
+ JsUnaryOperation op = new JsPostfixOperation(
+ JavaToJsOperatorMap.get(x.getOp()), ((JsExpression) pop())); // arg
+
+ if (maybeShuffleModifyingUnary(op)) {
+ return;
+ }
+
+ push(op);
}
// @Override
public void endVisit(JPrefixOperation x, Context ctx) {
- push(new JsPrefixOperation(JavaToJsOperatorMap.get(x.getOp()),
- (JsExpression) pop())); // arg
+ JsUnaryOperation op = new JsPrefixOperation(
+ JavaToJsOperatorMap.get(x.getOp()), ((JsExpression) pop())); // arg
+
+ if (maybeShuffleModifyingUnary(op)) {
+ return;
+ }
+
+ push(op);
}
// @Override
@@ -1541,6 +1536,66 @@
return jsInvocation;
}
+ /**
+ * Due to the way clinits are constructed, you can end up with a comma
+ * operation as the argument to a modifying operation, which is illegal.
+ * Juggle things to put the operator inside of the comma expression.
+ *
+ * TODO: move this to a post-generation normalization pass when JS tree
+ * modifying infrastructure is in place.
+ */
+ private boolean maybeShuffleModifyingBinary(JsBinaryOperation x) {
+ JsBinaryOperator myOp = x.getOperator();
+ JsExpression lhs = x.getArg1();
+ JsExpression rhs = x.getArg2();
+
+ 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);
+ }
+ // curArg is now the rightmost comma operation; slide our operation in
+ x.setArg1(curLhs.getArg2());
+ curLhs.setArg2(x);
+ // repush the comma expression
+ push(lhs);
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Due to the way clinits are constructed, you can end up with a comma
+ * operation as the argument to a modifying operation, which is illegal.
+ * Juggle things to put the operator inside of the comma expression.
+ *
+ * TODO: move this to a post-generation normalization pass when JS tree
+ * modifying infrastructure is in place.
+ */
+ private boolean maybeShuffleModifyingUnary(JsUnaryOperation x) {
+ JsUnaryOperator myOp = x.getOperator();
+ JsExpression arg = x.getArg();
+ if (myOp.isModifying() && (arg instanceof JsBinaryOperation)) {
+ // Find the rightmost comma operation
+ JsBinaryOperation curArg = (JsBinaryOperation) arg;
+ assert (curArg.getOperator() == JsBinaryOperator.COMMA);
+ while (curArg.getArg2() instanceof JsBinaryOperation) {
+ curArg = (JsBinaryOperation) curArg.getArg2();
+ assert (curArg.getOperator() == JsBinaryOperator.COMMA);
+ }
+ // curArg is now the rightmost comma operation; slide our operation in
+ x.setArg(curArg.getArg2());
+ curArg.setArg2(x);
+ // repush the comma expression
+ push(arg);
+ return true;
+ }
+ return false;
+ }
+
private JsNode pop() {
return (JsNode) nodeStack.pop();
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
index aa35210..ac1e9a3 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -18,36 +18,19 @@
/**
* A JavaScript postfix operation.
*/
-public final class JsPostfixOperation extends JsExpression {
-
- private JsExpression arg;
-
- private final JsUnaryOperator op;
+public final class JsPostfixOperation extends JsUnaryOperation {
public JsPostfixOperation(JsUnaryOperator op) {
this(op, null);
}
public JsPostfixOperation(JsUnaryOperator op, JsExpression arg) {
- this.op = op;
- this.arg = arg;
- }
-
- public JsExpression getArg() {
- return arg;
- }
-
- public JsUnaryOperator getOperator() {
- return op;
- }
-
- public void setArg(JsExpression arg) {
- this.arg = arg;
+ super(op, arg);
}
public void traverse(JsVisitor v) {
if (v.visit(this)) {
- arg.traverse(v);
+ super.traverse(v);
}
v.endVisit(this);
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
index 0b30e37..c1b19d7 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -18,36 +18,19 @@
/**
* A JavaScript prefix operation.
*/
-public final class JsPrefixOperation extends JsExpression {
-
- private JsExpression arg;
-
- private final JsUnaryOperator op;
+public final class JsPrefixOperation extends JsUnaryOperation {
public JsPrefixOperation(JsUnaryOperator op) {
this(op, null);
}
public JsPrefixOperation(JsUnaryOperator op, JsExpression arg) {
- this.op = op;
- this.arg = arg;
- }
-
- public JsExpression getArg() {
- return arg;
- }
-
- public JsUnaryOperator getOperator() {
- return op;
- }
-
- public void setArg(JsExpression arg) {
- this.arg = arg;
+ super(op, arg);
}
public void traverse(JsVisitor v) {
if (v.visit(this)) {
- arg.traverse(v);
+ super.traverse(v);
}
v.endVisit(this);
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperation.java
new file mode 100644
index 0000000..de9da19
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperation.java
@@ -0,0 +1,51 @@
+/*
+ * 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.ast;
+
+/**
+ * A JavaScript prefix or postfix operation.
+ */
+public abstract class JsUnaryOperation extends JsExpression {
+
+ private JsExpression arg;
+
+ private final JsUnaryOperator op;
+
+ public JsUnaryOperation(JsUnaryOperator op) {
+ this(op, null);
+ }
+
+ public JsUnaryOperation(JsUnaryOperator op, JsExpression arg) {
+ this.op = op;
+ this.arg = arg;
+ }
+
+ public JsExpression getArg() {
+ return arg;
+ }
+
+ public JsUnaryOperator getOperator() {
+ return op;
+ }
+
+ public void setArg(JsExpression arg) {
+ this.arg = arg;
+ }
+
+ public void traverse(JsVisitor v) {
+ arg.traverse(v);
+ }
+}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
index 7c17de9..430e385 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsUnaryOperator.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * 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
@@ -44,4 +44,8 @@
public boolean isKeyword() {
return this == DELETE || this == TYPEOF || this == VOID;
}
+
+ public boolean isModifying() {
+ return this == DEC || this == INC || this == DELETE;
+ }
}
