Revision: 7516
Author: sp...@google.com
Date: Mon Feb 1 07:58:03 2010
Log: 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.
http://code.google.com/p/google-web-toolkit/source/detail?r=7516
Added:
/trunk/user/test/com/google/gwt/core/StackTraceLineNumbersTest.gwt.xml
/trunk/user/test/com/google/gwt/core/client/impl/StackTraceLineNumbersTest.java
Modified:
/trunk/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
/trunk/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java
/trunk/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
=======================================
--- /dev/null
+++ /trunk/user/test/com/google/gwt/core/StackTraceLineNumbersTest.gwt.xml
Mon Feb 1 07:58:03 2010
@@ -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>
=======================================
--- /dev/null
+++
/trunk/user/test/com/google/gwt/core/client/impl/StackTraceLineNumbersTest.java
Mon Feb 1 07:58:03 2010
@@ -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;
+ }-*/;
+}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java Wed Oct
28 09:10:53 2009
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java Mon Feb
1 07:58:03 2010
@@ -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,8 +621,14 @@
@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);
+ }
@Override
public void endVisit(JsNew x, JsContext<JsExpression> 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) {
@@ -779,6 +798,8 @@
ctx.replaceMe(newRef);
}
}
+
+ private static final String PROPERTY_NAME = "compiler.emulatedStack";
public static void exec(JsProgram program, PropertyOracle[]
propertyOracles) {
SelectionProperty property;
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java Fri Sep 26
08:20:00 2008
+++ /trunk/dev/core/src/com/google/gwt/dev/js/ast/JsForIn.java Mon Feb 1
07:58:03 2010
@@ -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);
=======================================
--- /trunk/user/test/com/google/gwt/dev/jjs/CompilerSuite.java Mon Jan 11
09:04:38 2010
+++ /trunk/user/test/com/google/gwt/dev/jjs/CompilerSuite.java Mon Feb 1
07:58:03 2010
@@ -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);
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
