Goktug Gokdogan has submitted this change and it was merged.
Change subject: Fixes Exception wrapping/unwrapping in compiler
......................................................................
Fixes Exception wrapping/unwrapping in compiler
Change-Id: I419204b3b6f76bd9171c322a87fa164a7c72ab94
Review-Link: https://gwt-review.googlesource.com/#/c/3601/
---
M dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
M dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
M dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
M
dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
M user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
5 files changed, 214 insertions(+), 44 deletions(-)
Approvals:
Roberto Lublinerman: Looks good to me, approved
Leeroy Jenkins: Verified
diff --git
a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
index f903e8b..d24ad18 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
@@ -21,6 +21,7 @@
import com.google.gwt.dev.jjs.ast.JBinaryOperator;
import com.google.gwt.dev.jjs.ast.JBlock;
import com.google.gwt.dev.jjs.ast.JDeclarationStatement;
+import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JIfStatement;
import com.google.gwt.dev.jjs.ast.JInstanceOf;
@@ -30,6 +31,7 @@
import com.google.gwt.dev.jjs.ast.JMethodBody;
import com.google.gwt.dev.jjs.ast.JMethodCall;
import com.google.gwt.dev.jjs.ast.JModVisitor;
+import com.google.gwt.dev.jjs.ast.JNewInstance;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType;
@@ -51,6 +53,7 @@
* Collapses all multi-catch blocks into a single catch block.
*/
private class CollapseCatchBlocks extends JModVisitor {
+ JMethod wrapMethod = program.getIndexedMethod("Exceptions.wrap");
// @Override
@Override
@@ -71,9 +74,8 @@
JBlock newCatchBlock = new JBlock(catchInfo);
{
- // $e = Exceptions.caught($e)
- JMethod caughtMethod =
program.getIndexedMethod("Exceptions.caught");
- JMethodCall call = new JMethodCall(catchInfo, null, caughtMethod);
+ // $e = Exceptions.wrap($e)
+ JMethodCall call = new JMethodCall(catchInfo, null, wrapMethod);
call.addArg(new JLocalRef(catchInfo, exVar));
newCatchBlock.addStmt(JProgram.createAssignmentStmt(catchInfo, new
JLocalRef(catchInfo,
exVar), call));
@@ -144,6 +146,38 @@
}
}
+ private class UnwrapJavaScriptExceptionVisitor extends JModVisitor {
+ JDeclaredType jseType =
+
program.getFromTypeMap("com.google.gwt.core.client.JavaScriptException");
+ JMethod unwrapMethod = program.getIndexedMethod("Exceptions.unwrap");
+
+ @Override
+ public void endVisit(JThrowStatement x, Context ctx) {
+ assert jseType != null;
+
+ JExpression expr = x.getExpr();
+
+ // Optimization: unwrap not needed if "new BlahException()"
+ if (expr instanceof JNewInstance && !expr.getType().equals(jseType))
{
+ return;
+ }
+
+ // Optimization: unwrap not needed if expression can never be
JavaScriptException
+ if (!program.typeOracle.canTheoreticallyCast((JReferenceType)
expr.getType(), jseType)) {
+ return;
+ }
+
+ // throw x; -> throw Exceptions.unwrap(x);
+ ctx.replaceMe(createUnwrappedThrow(x));
+ }
+
+ private JThrowStatement createUnwrappedThrow(JThrowStatement x) {
+ JMethodCall call = new JMethodCall(x.getSourceInfo(), null,
unwrapMethod);
+ call.addArg(x.getExpr());
+ return new JThrowStatement(x.getSourceInfo(), call);
+ }
+ }
+
public static void exec(JProgram program) {
new CatchBlockNormalizer(program).execImpl();
}
@@ -165,6 +199,8 @@
private void execImpl() {
CollapseCatchBlocks collapser = new CollapseCatchBlocks();
collapser.accept(program);
+ UnwrapJavaScriptExceptionVisitor unwrapper = new
UnwrapJavaScriptExceptionVisitor();
+ unwrapper.accept(program);
}
private JLocal popTempLocal() {
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 5df8898..33a7233 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java
@@ -81,7 +81,7 @@
/**
* Resets the global stack depth to the local stack index and top stack
frame
- * after calls to Exceptions.caught. This is created by
+ * after calls to Exceptions.wrap. This is created by
* {@link EntryExitVisitor#visit(JsCatch, JsContext)}.
*/
private class CatchStackReset extends JsModVisitor {
@@ -97,7 +97,7 @@
@Override
public void endVisit(JsExprStmt x, JsContext ctx) {
- // Looking for e = caught(e);
+ // Looking for e = wrap(e);
JsExpression expr = x.getExpression();
if (!(expr instanceof JsBinaryOperation)) {
@@ -120,8 +120,8 @@
return;
}
- // caughtFunction is the JsFunction translated from Exceptions.caught
- if (name.getStaticRef() != caughtFunction) {
+ // caughtFunction is the JsFunction translated from Exceptions.wrap
+ if (name.getStaticRef() != wrapFunction) {
return;
}
@@ -196,7 +196,7 @@
* try {
* foo();
* } catch (e) {
- * e = caught(e);
+ * e = wrap(e);
* $stackDepth = stackIndex;
* throw e;
* } finally {
@@ -450,21 +450,21 @@
private JsCatch makeSyntheticCatchBlock(JsTry x) {
/*
- * catch (e) { e = caught(e); throw e; }
+ * catch (e) { e = wrap(e); throw e; }
*/
SourceInfo info = x.getSourceInfo();
JsCatch c = new JsCatch(info, currentFunction.getScope(), "e");
JsName paramName = c.getParameter().getName();
- // caught(e)
- JsInvocation caughtCall = new JsInvocation(info);
- caughtCall.setQualifier(caughtFunction.getName().makeRef(info));
- caughtCall.getArguments().add(paramName.makeRef(info));
+ // wrap(e)
+ JsInvocation wrapCall = new JsInvocation(info);
+ wrapCall.setQualifier(wrapFunction.getName().makeRef(info));
+ wrapCall.getArguments().add(paramName.makeRef(info));
- // e = caught(e)
+ // e = wrap(e)
JsBinaryOperation asg = new JsBinaryOperation(info,
JsBinaryOperator.ASG,
- paramName.makeRef(info), caughtCall);
+ paramName.makeRef(info), wrapCall);
// throw e
JsThrow throwStatement = new JsThrow(info, paramName.makeRef(info));
@@ -852,7 +852,7 @@
return stackMode;
}
- private JsFunction caughtFunction;
+ private JsFunction wrapFunction;
private JsName lineNumbers;
private JProgram jprogram;
private final JsProgram jsProgram;
@@ -886,8 +886,8 @@
}
private void execImpl() {
- caughtFunction = jsProgram.getIndexedFunction("Exceptions.caught");
- if (caughtFunction == null) {
+ wrapFunction = jsProgram.getIndexedFunction("Exceptions.wrap");
+ if (wrapFunction == null) {
// No exceptions caught? Weird, but possible.
return;
}
diff --git a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
index 15e1354..4798106 100644
--- a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
+++ b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
@@ -18,8 +18,8 @@
import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.dev.util.Name;
-import com.google.gwt.dev.util.Util;
import com.google.gwt.dev.util.Name.BinaryName;
+import com.google.gwt.dev.util.Util;
import com.google.gwt.dev.util.log.speedtracer.DevModeEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
diff --git
a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
index 09e4bab..4e075f5 100644
---
a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
+++
b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
@@ -16,19 +16,39 @@
package com.google.gwt.lang;
import com.google.gwt.core.client.JavaScriptException;
+import com.google.gwt.core.client.JavaScriptObject;
/**
* This is a magic class the compiler uses to throw and check exceptions.
*/
final class Exceptions {
- static Object caught(Object e) {
+ static Object wrap(Object e) {
if (e instanceof Throwable) {
return e;
}
- return new JavaScriptException(e);
+ return e == null ? new JavaScriptException(null) :
getCachableJavaScriptException(e);
}
+ static Object unwrap(Object e) {
+ if (e instanceof JavaScriptException) {
+ JavaScriptObject exception = ((JavaScriptException)
e).getException();
+ if (exception != null) {
+ return exception;
+ }
+ }
+ return e;
+ }
+
+ private static native JavaScriptException
getCachableJavaScriptException(Object e)/*-{
+ var jse = e.__gwt$exception;
+ if (!jse) {
+ jse =
@com.google.gwt.core.client.JavaScriptException::new(Ljava/lang/Object;)(e);
+ e.__gwt$exception = jse;
+ }
+ return jse;
+ }-*/;
+
static boolean throwAssertionError() {
throw new AssertionError();
}
diff --git
a/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
index d4b193e..ef3acdf 100644
--- a/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
+++ b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
@@ -15,6 +15,8 @@
*/
package com.google.gwt.core.client;
+import com.google.gwt.junit.DoNotRunWith;
+import com.google.gwt.junit.Platform;
import com.google.gwt.junit.client.GWTTestCase;
import com.google.gwt.junit.client.WithProperties;
import com.google.gwt.junit.client.WithProperties.Property;
@@ -28,7 +30,7 @@
*/
public class JavaScriptExceptionTest extends GWTTestCase {
- static native JavaScriptObject makeJSO() /*-{
+ private static native JavaScriptObject makeJSO() /*-{
return {
toString:function() {
return "jso";
@@ -39,23 +41,67 @@
};
}-*/;
- static void throwJava(Throwable t) throws Throwable {
- throw t;
+ private static Object makeJavaObject() {
+ Object o = new Object() {
+ @Override public String toString() {
+ return "myLameObject";
+ }
+ };
+ return o;
}
- static native void throwNative(Object e) /*-{
+ private static native void throwNative(Object e) /*-{
throw e;
}-*/;
- static void throwSandwichJava(Object e) {
+ private static void throwSandwichJava(Object e) {
throwNative(e);
}
- static native void throwSandwichNative(Throwable t) /*-{
-
@com.google.gwt.core.client.JavaScriptExceptionTest::throwJava(Ljava/lang/Throwable;)(t);
+ private static Throwable catchJava(Runnable runnable) {
+ try {
+ runnable.run();
+ } catch (Throwable e) {
+ return e;
+ }
+ return null;
+ }
+
+ private static native Object catchNative(Runnable runnable) /*-{
+ try {
+ runnab...@java.lang.Runnable::run()();
+ } catch(e) {
+ return e;
+ }
}-*/;
- public void assertJsoProperties(boolean extraPropertiesShouldBePresent) {
+ private static Runnable createThrowRunnable(final Throwable e) {
+ return new Runnable() {
+ @Override public void run() {
+ throw sneakyThrow(e);
+ }
+
+ @SuppressWarnings("unchecked")
+ <T extends RuntimeException> T sneakyThrow(Throwable e) {
+ return (T) e;
+ }
+ };
+ }
+
+ private static Runnable createThrowNativeRunnable(final Object e) {
+ return new Runnable() {
+ @Override public void run() {
+ throwNative(e);
+ }
+ };
+ }
+
+ private static void assertJavaScriptException(Object expected, Throwable
exception) {
+ assertTrue(exception instanceof JavaScriptException);
+ assertEquals(expected, ((JavaScriptException)
exception).getException());
+ }
+
+ private static void assertJsoProperties(boolean
extraPropertiesShouldBePresent) {
JavaScriptObject jso = makeJSO();
try {
throwNative(jso);
@@ -107,13 +153,84 @@
return "com.google.gwt.core.Core";
}
- public void testJavaExceptionSandwich() {
+ public void testCatch() {
RuntimeException e = new RuntimeException();
- try {
- throwSandwichNative(e);
- } catch (Throwable t) {
- assertSame(e, t);
- }
+ assertSame(e, catchJava(createThrowRunnable(e)));
+
+ JavaScriptObject jso = makeJSO();
+ e = new JavaScriptException(jso);
+ assertJavaScriptException(jso, catchJava(createThrowRunnable(e)));
+ }
+
+ // java throw -> jsni catch -> jsni throw -> java catch
+ public void testJavaNativeJavaSandwichCatch() {
+ RuntimeException e = new RuntimeException();
+ assertSame(e, javaNativeJavaSandwich(e));
+
+ JavaScriptObject jso = makeJSO();
+ e = new JavaScriptException(jso);
+ assertJavaScriptException(jso, javaNativeJavaSandwich(e));
+ }
+
+ private Throwable javaNativeJavaSandwich(RuntimeException e) {
+ return
catchJava(createThrowNativeRunnable(catchJava(createThrowRunnable(e))));
+ }
+
+ public void testCatchThrowNative() {
+ Object e;
+
+ e = makeJSO();
+ assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
+
+ // TODO(goktug): non-jso objects will be supported in follow up CL
+ //
+ // e = "testing";
+ // assertJavaScriptException(e,
catchJava(createThrowNativeRunnable(e)));
+ //
+ // e = makeJavaObject();
+ // assertJavaScriptException(e,
catchJava(createThrowNativeRunnable(e)));
+ //
+ // e = null;
+ // assertJavaScriptException(e,
catchJava(createThrowNativeRunnable(e)));
+
+ e = new RuntimeException();
+ assertSame(e, catchJava(createThrowNativeRunnable(e)));
+
+ e = new JavaScriptException(makeJSO());
+ assertSame(e, catchJava(createThrowNativeRunnable(e)));
+ }
+
+ // jsni throw -> java catch -> java throw -> jsni catch
+ public void testNativeJavaNativeSandwichCatch() {
+ Object e;
+
+ e = makeJSO();
+ assertSame(e, nativeJavaNativeSandwich(e));
+
+ // TODO(goktug): non-jso objects will be supported in follow up CL
+ //
+ // e = "testing";
+ // assertEquals(e, nativeJavaNativeSandwich(e));
+ // if (GWT.isScript()) { // Devmode will not preserve the same String
instance
+ // assertSame(e, nativeJavaNativeSandwich(e));
+ // }
+ //
+ // e = makeJavaObject();
+ // assertJavaScriptException(e,
catchJava(createThrowNativeRunnable(e)));
+ //
+ // e = null;
+ // assertSame(e, nativeJavaNativeSandwich(e));
+
+ e = new RuntimeException();
+ assertSame(e, nativeJavaNativeSandwich(e));
+
+ JavaScriptObject jso = makeJSO();
+ e = new JavaScriptException(jso);
+ assertSame(jso, nativeJavaNativeSandwich(e));
+ }
+
+ private Object nativeJavaNativeSandwich(Object e) {
+ return
catchNative(createThrowRunnable(catchJava(createThrowNativeRunnable(e))));
}
@WithProperties({
@@ -128,7 +245,8 @@
*/
assertJsoProperties(false);
}
-
+
+ @DoNotRunWith(Platform.HtmlUnitUnknown)
@WithProperties({
@Property(name = "compiler.stackMode", value = "native")
})
@@ -142,7 +260,8 @@
*/
assertJsoProperties(GWT.isScript());
}
-
+
+ @DoNotRunWith(Platform.HtmlUnitUnknown)
@WithProperties({
@Property(name = "compiler.stackMode", value = "strip")
})
@@ -170,12 +289,7 @@
}
public void testObject() {
- Object o = new Object() {
- @Override
- public String toString() {
- return "myLameObject";
- }
- };
+ Object o = makeJavaObject();
try {
throwNative(o);
fail();
@@ -200,7 +314,7 @@
}
}
- private void assertDescription(JavaScriptException e, String
description) {
+ private static void assertDescription(JavaScriptException e, String
description) {
if (!GWT.isScript()) {
assertTrue("Should start with method name",
e.getDescription().startsWith(
--
To view, visit https://gwt-review.googlesource.com/3601
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I419204b3b6f76bd9171c322a87fa164a7c72ab94
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Leeroy Jenkins <jenk...@gwtproject.org>
Gerrit-Reviewer: Roberto Lublinerman <rlu...@google.com>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
