[GR-27762] Fix unwrapping PExceptions from interop

PullRequest: graalpython/1426

Author: msimacek

graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/interop/InteropLibraryTest.java

@@ -40,24 +40,27 @@
  */
 package com.oracle.graal.python.test.interop;
 
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
 
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Context.Builder;
+import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Value;
 import org.graalvm.polyglot.proxy.ProxyArray;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
 
 import com.oracle.graal.python.test.PythonTests;
 
-import java.util.Arrays;
-import java.util.Iterator;
-import java.util.List;
-
 public class InteropLibraryTest extends PythonTests {
     private Context context;
 
@@ -165,6 +168,28 @@ public void testForItemInLazyArray() {
         assertEquals(list.size() * 3, tripples.getArraySize());
     }
 
+    @Test
+    public void testException() {
+        try {
+            context.eval("python", "1/0");
+        } catch (PolyglotException e) {
+            Value exception = e.getGuestObject();
+            assertTrue(exception.isException());
+            Value clazz = exception.getMetaObject();
+            assertTrue(clazz.isMetaObject());
+            assertEquals("ZeroDivisionError", clazz.getMetaSimpleName());
+            assertTrue(exception.hasMember("args"));
+            Value args = exception.getMember("args");
+            assertTrue(args.hasArrayElements());
+            assertEquals(1, args.getArraySize());
+            assertEquals("division by zero", args.getArrayElement(0).asString());
+            // This excercises isIdenticalOrUndefined message internally
+            assertNotEquals(exception, args);
+            return;
+        }
+        fail("didn't throw exception");
+    }
+
     private static final class LazyArray implements ProxyArray {
 
         private final Iterator<?> it;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java

@@ -38,6 +38,7 @@
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
+import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
 import com.oracle.graal.python.runtime.PythonContext;
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives;
@@ -618,8 +619,9 @@ static boolean isMetaObject(@SuppressWarnings("unused") PythonBuiltinClassType s
     @ExportMessage
     static boolean isMetaInstance(PythonBuiltinClassType self, Object instance,
                     @CachedLibrary(limit = "3") PythonObjectLibrary lib,
+                    @Cached PForeignToPTypeNode convert,
                     @Cached IsSubtypeNode isSubtype) {
-        return isSubtype.execute(lib.getLazyPythonClass(instance), self);
+        return isSubtype.execute(lib.getLazyPythonClass(convert.executeConvert(instance)), self);
     }
 
     @ExportMessage

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/PythonAbstractObject.java

@@ -235,16 +235,16 @@ private boolean isAbstractMapping(PythonObjectLibrary thisLib) {
 
     @ExportMessage
     public void writeMember(String key, Object value,
-                    @Exclusive @Cached PInteropSubscriptAssignNode setItemNode,
+                    @Shared("setItemNode") @Cached PInteropSubscriptAssignNode setItemNode,
                     @CachedLibrary("this") PythonObjectLibrary dataModelLibrary,
                     @Exclusive @Cached KeyForAttributeAccess getAttributeKey,
                     @Exclusive @Cached KeyForItemAccess getItemKey,
-                    @Cached PInteropSetAttributeNode writeNode,
+                    @Cached PInteropSetAttributeNode setAttributeNode,
                     @Shared("attributeErrorProfile") @Cached IsBuiltinClassProfile attrErrorProfile) throws UnsupportedMessageException, UnknownIdentifierException {
         try {
             String attrKey = getAttributeKey.execute(key);
             if (attrKey != null) {
-                writeNode.execute(this, attrKey, value);
+                setAttributeNode.execute(this, attrKey, value);
                 return;
             }
 
@@ -256,14 +256,14 @@ public void writeMember(String key, Object value,
 
             if (this instanceof PythonObject) {
                 if (objectHasAttribute(this, key)) {
-                    writeNode.execute(this, key, value);
+                    setAttributeNode.execute(this, key, value);
                     return;
                 }
             }
             if (isAbstractMapping(dataModelLibrary)) {
                 setItemNode.execute(this, key, value);
             } else {
-                writeNode.execute(this, key, value);
+                setAttributeNode.execute(this, key, value);
             }
         } catch (PException e) {
             e.expectAttributeError(attrErrorProfile);
@@ -347,7 +347,7 @@ public Object readArrayElement(long key,
     @ExportMessage
     public void writeArrayElement(long key, Object value,
                     @CachedLibrary("this") PythonObjectLibrary dataModelLibrary,
-                    @Exclusive @Cached PInteropSubscriptAssignNode setItemNode) throws UnsupportedMessageException, InvalidArrayIndexException {
+                    @Shared("setItemNode") @Cached PInteropSubscriptAssignNode setItemNode) throws UnsupportedMessageException, InvalidArrayIndexException {
         if (dataModelLibrary.isSequence(this)) {
             try {
                 setItemNode.execute(this, key, value);
@@ -1867,13 +1867,14 @@ public abstract static class PInteropSubscriptAssignNode extends Node {
 
         @Specialization
         static void doSpecialObject(PythonAbstractObject primary, Object key, Object value,
+                        @Cached PForeignToPTypeNode convert,
                         @Cached PInteropGetAttributeNode getAttributeNode,
                         @Cached CallBinaryMethodNode callSetItemNode,
                         @Cached ConditionProfile profile) throws UnsupportedMessageException {
 
             Object attrSetitem = getAttributeNode.execute(primary, __SETITEM__);
             if (profile.profile(attrSetitem != PNone.NO_VALUE)) {
-                callSetItemNode.executeObject(attrSetitem, key, value);
+                callSetItemNode.executeObject(attrSetitem, key, convert.executeConvert(value));
             } else {
                 throw UnsupportedMessageException.create();
             }
@@ -1890,15 +1891,16 @@ public abstract static class PInteropSetAttributeNode extends Node {
         public abstract void execute(Object primary, String attrName, Object value) throws UnsupportedMessageException, UnknownIdentifierException;
 
         @Specialization
-        public void doSpecialObject(PythonAbstractObject primary, String attrName, Object value,
+        public static void doSpecialObject(PythonAbstractObject primary, String attrName, Object value,
+                        @Cached PForeignToPTypeNode convert,
                         @Cached LookupInheritedAttributeNode.Dynamic lookupSetAttrNode,
                         @Cached CallTernaryMethodNode callSetAttrNode,
                         @Cached ConditionProfile profile,
                         @Cached IsBuiltinClassProfile attrErrorProfile) throws UnsupportedMessageException, UnknownIdentifierException {
-            Object attrGetattribute = lookupSetAttrNode.execute(primary, SpecialMethodNames.__SETATTR__);
-            if (profile.profile(attrGetattribute != PNone.NO_VALUE)) {
+            Object attrSetattr = lookupSetAttrNode.execute(primary, SpecialMethodNames.__SETATTR__);
+            if (profile.profile(attrSetattr != PNone.NO_VALUE)) {
                 try {
-                    callSetAttrNode.execute(null, attrGetattribute, primary, attrName, value);
+                    callSetAttrNode.execute(null, attrSetattr, primary, attrName, convert.executeConvert(value));
                 } catch (PException e) {
                     e.expectAttributeError(attrErrorProfile);
                     // TODO(fa) not accurate; distinguish between read-only and non-existing
@@ -2086,9 +2088,11 @@ public static int systemHashCode(Object value) {
     }
 
     @ExportMessage
-    public TriState isIdenticalOrUndefined(Object other,
+    public TriState isIdenticalOrUndefined(Object otherInterop,
+                    @Cached PForeignToPTypeNode convert,
                     @CachedLibrary(limit = "3") InteropLibrary otherLib,
                     @CachedLibrary("this") PythonObjectLibrary objectLib) {
+        Object other = convert.executeConvert(otherInterop);
         if (this == other) {
             return TriState.TRUE;
         } else if (otherLib.hasIdentity(other)) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PythonAbstractNativeObject.java

@@ -67,6 +67,7 @@
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
+import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
 import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerAsserts;
@@ -319,11 +320,12 @@ boolean isMetaObject(
     boolean isMetaInstance(Object instance,
                     @Shared("isType") @Cached TypeNodes.IsTypeNode isType,
                     @CachedLibrary(limit = "3") PythonObjectLibrary plib,
+                    @Cached PForeignToPTypeNode convert,
                     @Cached IsSubtypeNode isSubtype) throws UnsupportedMessageException {
         if (!isType.execute(this)) {
             throw UnsupportedMessageException.create();
         }
-        return isSubtype.execute(plib.getLazyPythonClass(instance), this);
+        return isSubtype.execute(plib.getLazyPythonClass(convert.executeConvert(instance)), this);
     }
 
     @ExportMessage

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/PList.java

@@ -29,6 +29,7 @@
 import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
 import com.oracle.graal.python.builtins.objects.ints.PInt;
 import com.oracle.graal.python.nodes.ErrorMessages;
+import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
 import com.oracle.graal.python.nodes.literal.ListLiteralNode;
 import com.oracle.graal.python.runtime.exception.PException;
 import com.oracle.graal.python.runtime.sequence.PSequence;
@@ -187,9 +188,10 @@ public boolean isArrayElementRemovable(long index,
 
     @ExportMessage
     public void writeArrayElement(long index, Object value,
+                    @Cached PForeignToPTypeNode convert,
                     @Cached.Exclusive @Cached SequenceStorageNodes.SetItemScalarNode setItem) throws InvalidArrayIndexException {
         try {
-            setItem.execute(store, PInt.intValueExact(index), value);
+            setItem.execute(store, PInt.intValueExact(index), convert.executeConvert(value));
         } catch (OverflowException e) {
             CompilerDirectives.transferToInterpreterAndInvalidate();
             throw InvalidArrayIndexException.create(index);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonBuiltinClass.java

@@ -33,6 +33,7 @@
 import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
+import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
@@ -87,8 +88,9 @@ boolean isMetaObject() {
     @SuppressWarnings("static-method")
     boolean isMetaInstance(Object instance,
                     @CachedLibrary(limit = "3") PythonObjectLibrary plib,
+                    @Cached PForeignToPTypeNode convert,
                     @Cached IsSubtypeNode isSubtype) {
-        return isSubtype.execute(plib.getLazyPythonClass(instance), this);
+        return isSubtype.execute(plib.getLazyPythonClass(convert.executeConvert(instance)), this);
     }
 
     @ExportMessage

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java

@@ -29,6 +29,7 @@
 import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
 import com.oracle.graal.python.nodes.attributes.ReadAttributeFromDynamicObjectNode;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
+import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Shared;
 import com.oracle.truffle.api.interop.InteropLibrary;
@@ -64,8 +65,9 @@ boolean isMetaObject() {
     @ExportMessage
     boolean isMetaInstance(Object instance,
                     @CachedLibrary(limit = "3") PythonObjectLibrary plib,
+                    @Cached PForeignToPTypeNode convert,
                     @Cached IsSubtypeNode isSubtype) {
-        return isSubtype.execute(plib.getLazyPythonClass(instance), this);
+        return isSubtype.execute(plib.getLazyPythonClass(convert.executeConvert(instance)), this);
     }
 
     @ExportMessage

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/interop/PForeignToPTypeNode.java

@@ -60,7 +60,7 @@ public abstract class PForeignToPTypeNode extends Node {
     public abstract Object executeConvert(Object value);
 
     protected static boolean isOtherClass(Class<?> clazz) {
-        return !(clazz == Byte.class || clazz == Short.class || clazz == Float.class || clazz == Character.class);
+        return !(clazz == Byte.class || clazz == Short.class || clazz == Float.class || clazz == Character.class || clazz == PException.class);
     }
 
     @Specialization(guards = {"value.getClass() == cachedClass", "isOtherClass(cachedClass)"}, limit = "1")