Goktug Gokdogan has uploaded a new change for review.

  https://gwt-review.googlesource.com/3601


Change subject: Fixes Exception wrapping/unwrapping in compiler
......................................................................

Fixes Exception wrapping/unwrapping in compiler

Change-Id: I419204b3b6f76bd9171c322a87fa164a7c72ab94
---
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, 213 insertions(+), 43 deletions(-)



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..3de5195 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;
@@ -71,8 +73,8 @@
       JBlock newCatchBlock = new JBlock(catchInfo);

       {
-        // $e = Exceptions.caught($e)
- JMethod caughtMethod = program.getIndexedMethod("Exceptions.caught");
+        // $e = Exceptions.wrap($e)
+        JMethod caughtMethod = program.getIndexedMethod("Exceptions.wrap");
         JMethodCall call = new JMethodCall(catchInfo, null, caughtMethod);
         call.addArg(new JLocalRef(catchInfo, exVar));
newCatchBlock.addStmt(JProgram.createAssignmentStmt(catchInfo, new JLocalRef(catchInfo,
@@ -144,6 +146,38 @@
     }
   }

+  private class UnwrapJavaScriptExceptionVisitor extends JModVisitor {
+    JDeclaredType jseType =
+ program.getFromTypeMap("com.google.gwt.core.client.JavaScriptException");
+
+    @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) {
+      JMethod unwrapMethod = program.getIndexedMethod("Exceptions.unwrap");
+ 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: newchange
Gerrit-Change-Id: I419204b3b6f76bd9171c322a87fa164a7c72ab94
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <[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.


Reply via email to