Fixes Exception wrapping/unwrapping in compiler Change-Id: I419204b3b6f76bd9171c322a87fa164a7c72ab94 Review-Link: https://gwt-review.googlesource.com/#/c/3601/
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 { + runnable.@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(