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 {
+ [email protected]::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 <[email protected]>
Gerrit-Reviewer: Goktug Gokdogan <[email protected]>
Gerrit-Reviewer: Leeroy Jenkins <[email protected]>
Gerrit-Reviewer: Roberto Lublinerman <[email protected]>
--
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 [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
