Fixes issue 4512. In JsStackEmulation, avoid rewriting references to
non-references. For example, don't rewrite bar['foo']() to
(line="123",bar['foo'])(), because that will result in
"this" being set incorrectly in the invoked function.
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7516 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java b/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
index fb1d93e..ab22d33 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
@@ -38,6 +38,7 @@
import com.google.gwt.dev.js.ast.JsName;
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsNew;
+import com.google.gwt.dev.js.ast.JsNode;
import com.google.gwt.dev.js.ast.JsPostfixOperation;
import com.google.gwt.dev.js.ast.JsPrefixOperation;
import com.google.gwt.dev.js.ast.JsProgram;
@@ -56,8 +57,10 @@
import com.google.gwt.dev.util.collect.Maps;
import java.io.File;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* Emulates the JS stack in order to provide useful stack traces on browers that
@@ -67,8 +70,6 @@
*/
public class JsStackEmulator {
- private static final String PROPERTY_NAME = "compiler.emulatedStack";
-
/**
* Resets the global stack depth to the local stack index and top stack frame
* after calls to Exceptions.caught. This is created by
@@ -593,6 +594,14 @@
private String lastFile;
private int lastLine;
+ /**
+ * Nodes in this set are used in a context that expects a reference, not
+ * just an arbitrary expression. For example, <code>delete</code> takes a
+ * reference. These are tracked because it wouldn't be safe to rewrite
+ * <code>delete foo.bar</code> to <code>delete (line='123',foo).bar</code>.
+ */
+ private final Set<JsNode<?>> nodesInRefContext = new HashSet<JsNode<?>>();
+
public LocationVisitor(JsFunction function) {
super(function);
resetPosition();
@@ -612,6 +621,12 @@
@Override
public void endVisit(JsInvocation x, JsContext<JsExpression> ctx) {
+ nodesInRefContext.remove(x.getQualifier());
+ record(x, ctx);
+ }
+
+ @Override
+ public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
record(x, ctx);
}
@@ -622,16 +637,13 @@
@Override
public void endVisit(JsPostfixOperation x, JsContext<JsExpression> ctx) {
- if (x.getOperator().isModifying()) {
- record(x, ctx);
- }
+ record(x, ctx);
}
@Override
public void endVisit(JsPrefixOperation x, JsContext<JsExpression> ctx) {
- if (x.getOperator().isModifying()) {
- record(x, ctx);
- }
+ record(x, ctx);
+ nodesInRefContext.remove(x.getArg());
}
/**
@@ -663,12 +675,16 @@
}
@Override
- public boolean visit(JsNameRef x, JsContext<JsExpression> ctx) {
- if (ctx.isLvalue()) {
- if (x.getQualifier() != null) {
- accept(x.getQualifier());
- }
- return false;
+ public boolean visit(JsInvocation x, JsContext<JsExpression> ctx) {
+ nodesInRefContext.add(x.getQualifier());
+ return true;
+ }
+
+ @Override
+ public boolean visit(JsPrefixOperation x, JsContext<JsExpression> ctx) {
+ if (x.getOperator() == JsUnaryOperator.DELETE
+ || x.getOperator() == JsUnaryOperator.TYPEOF) {
+ nodesInRefContext.add(x.getArg());
}
return true;
}
@@ -706,6 +722,9 @@
if (ctx.isLvalue()) {
// Assignments to comma expressions aren't legal
return;
+ } else if (nodesInRefContext.contains(x)) {
+ // Don't modify references into non-references
+ return;
} else if (x.getSourceInfo().getStartLine() == lastLine
&& (!recordFileNames || x.getSourceInfo().getFileName().equals(
lastFile))) {
@@ -750,13 +769,13 @@
* with references to our locally-defined, obfuscatable names.
*/
private class ReplaceUnobfuscatableNames extends JsModVisitor {
+ private final JsName rootLineNumbers = program.getRootScope().findExistingUnobfuscatableName(
+ "$location");
// See JsRootScope for the definition of these names
private final JsName rootStack = program.getRootScope().findExistingUnobfuscatableName(
"$stack");
private final JsName rootStackDepth = program.getRootScope().findExistingUnobfuscatableName(
"$stackDepth");
- private final JsName rootLineNumbers = program.getRootScope().findExistingUnobfuscatableName(
- "$location");
@Override
public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) {
@@ -780,6 +799,8 @@
}
}
+ private static final String PROPERTY_NAME = "compiler.emulatedStack";
+
public static void exec(JsProgram program, PropertyOracle[] propertyOracles) {
SelectionProperty property;
try {
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java b/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java
index 4358782..a5f750b 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java
@@ -71,7 +71,7 @@
public void traverse(JsVisitor v, JsContext<JsStatement> ctx) {
if (v.visit(this, ctx)) {
if (iterExpr != null) {
- iterExpr = v.accept(iterExpr);
+ iterExpr = v.acceptLvalue(iterExpr);
}
objExpr = v.accept(objExpr);
body = v.accept(body);
diff --git a/user/test/com/google/gwt/core/StackTraceLineNumbersTest.gwt.xml b/user/test/com/google/gwt/core/StackTraceLineNumbersTest.gwt.xml
new file mode 100644
index 0000000..89ad4d9
--- /dev/null
+++ b/user/test/com/google/gwt/core/StackTraceLineNumbersTest.gwt.xml
@@ -0,0 +1,28 @@
+<!-- -->
+<!-- Copyright 2010 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 -->
+<!-- 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. License for the specific language governing permissions and -->
+<!-- limitations under the License. -->
+
+<!-- Types and resources required to support primitive system operation. -->
+<!-- -->
+<!-- Types from this module are visible to and imported into user code. -->
+<!-- Every module should directly or indirectly inherit this module. -->
+<!-- -->
+
+<module>
+ <inherits name="com.google.gwt.core.Core" />
+
+ <set-configuration-property name="compiler.emulatedStack.recordLineNumbers"
+ value="true" />
+ <set-configuration-property name="compiler.emulatedStack.recordFileNames"
+ value="true" />
+</module>
diff --git a/user/test/com/google/gwt/core/client/impl/StackTraceLineNumbersTest.java b/user/test/com/google/gwt/core/client/impl/StackTraceLineNumbersTest.java
new file mode 100644
index 0000000..1941553
--- /dev/null
+++ b/user/test/com/google/gwt/core/client/impl/StackTraceLineNumbersTest.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2010 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.core.client.impl;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Tests that stack traces work properly with line numbers turned on.
+ */
+public class StackTraceLineNumbersTest extends GWTTestCase {
+ @Override
+ public String getModuleName() {
+ return "com.google.gwt.core.StackTraceLineNumbersTest";
+ }
+
+ /**
+ * Tests that the rewrites do not change a reference to a comma
+ * expression, in contexts where a reference is needed and
+ * not just a value. See issue 4512.
+ */
+ public native void testRefBreakups() /*-{
+ var assertTrue = @junit.framework.Assert::assertTrue(Ljava/lang/String;Z);
+
+ // Check breakups in an invocation context
+ var bar = {
+ foo: function() {
+ return this === bar;
+ }
+ }
+
+ assertTrue("bar['foo']", bar['foo']());
+ assertTrue("bar.foo", bar.foo());
+
+ // Check breakups in for-in statements
+ var c = null;
+ for (a in [0, 1, 2]) {
+ c = a;
+ }
+ assertTrue("for-in", c==2);
+
+ // typeOf works on bad references
+ assertTrue("typeOf", (typeof someNameThatDoesNotExist301402172) == 'undefined');
+
+ // delete needs a reference, not a value
+ delete bar.foo;
+ }-*/;
+}
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
index 1c6d9cf..cb49e19 100644
--- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
+++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
@@ -15,6 +15,7 @@
*/
package com.google.gwt.dev.jjs;
+import com.google.gwt.core.client.impl.StackTraceLineNumbersTest;
import com.google.gwt.dev.jjs.scriptonly.ScriptOnlyTest;
import com.google.gwt.dev.jjs.test.AnnotationsTest;
import com.google.gwt.dev.jjs.test.AutoboxTest;
@@ -93,6 +94,7 @@
suite.addTestSuite(RunAsyncTest.class);
suite.addTestSuite(ScriptOnlyTest.class);
suite.addTestSuite(SingleJsoImplTest.class);
+ suite.addTestSuite(StackTraceLineNumbersTest.class);
suite.addTestSuite(TypeHierarchyTest.class);
suite.addTestSuite(UnstableGeneratorTest.class);
suite.addTestSuite(VarargsTest.class);
