[GR-15913] Avoid interop exceptions.

PullRequest: graalpython/542

Author: fangerer

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

@@ -264,9 +264,9 @@ public void writeArrayElement(long key, Object value,
                 // it's a sequence, so we assume the index is wrong
                 throw InvalidArrayIndexException.create(key);
             }
+        } else {
+            throw UnsupportedMessageException.create();
         }
-
-        throw UnsupportedMessageException.create();
     }
 
     @ExportMessage
@@ -281,9 +281,9 @@ public void removeArrayElement(long key,
                 // it's a sequence, so we assume the index is wrong
                 throw InvalidArrayIndexException.create(key);
             }
+        } else {
+            throw UnsupportedMessageException.create();
         }
-
-        throw UnsupportedMessageException.create();
     }
 
     private static Object iterateToKey(LookupInheritedAttributeNode.Dynamic lookupNextNode, CallNode callNextNode, Object iter, long key) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/foreign/AccessForeignItemNodes.java

@@ -126,7 +126,7 @@ public Object doForeignObjectSlice(Object object, PSlice idxSlice,
             try {
                 mslice = materializeSlice(idxSlice, object, lib);
             } catch (UnsupportedMessageException e) {
-                throw raise(AttributeError, "%s instance has no attribute '__getitem__'", object);
+                throw raiseAttributeError(object);
             }
             Object[] values = new Object[mslice.length];
             for (int i = mslice.start, j = 0; i < mslice.stop; i += mslice.step, j++) {
@@ -138,17 +138,19 @@ public Object doForeignObjectSlice(Object object, PSlice idxSlice,
         @Specialization
         public Object doForeignKey(Object object, String key,
                         @CachedLibrary(limit = "3") InteropLibrary lib) {
-            try {
-                return lib.readMember(object, key);
-            } catch (UnsupportedMessageException e) {
-                if (lib.hasArrayElements(object)) {
-                    throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
-                } else {
-                    throw raise(AttributeError, "%s instance has no attribute '__getitem__'", object);
+            if (lib.hasMembers(object)) {
+                if (lib.isMemberReadable(object, key)) {
+                    try {
+                        return lib.readMember(object, key);
+                    } catch (UnsupportedMessageException e) {
+                        throw raiseAttributeErrorDisambiguated(object, key, lib);
+                    } catch (UnknownIdentifierException e) {
+                        // fall through
+                    }
                 }
-            } catch (UnknownIdentifierException e) {
                 throw raise(KeyError, key);
             }
+            throw raiseAttributeErrorDisambiguated(object, key, lib);
         }
 
         @Specialization(guards = {"!isSlice(idx)", "!isString(idx)"})
@@ -158,14 +160,36 @@ public Object doForeignObject(Object object, Object idx,
             return readForeignValue(object, castIndex.execute(idx), lib);
         }
 
+        private PException raiseAttributeErrorDisambiguated(Object object, String key, InteropLibrary lib) {
+            if (lib.hasArrayElements(object)) {
+                throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
+            } else {
+                throw raiseAttributeError(object);
+            }
+        }
+
         private Object readForeignValue(Object object, long index, InteropLibrary lib) {
-            try {
-                return getToPythonNode().executeConvert(lib.readArrayElement(object, index));
-            } catch (UnsupportedMessageException ex) {
-                throw raise(AttributeError, "%s instance has no attribute '__getitem__'", object);
-            } catch (InvalidArrayIndexException ex) {
-                throw raise(IndexError, "invalid index %s", index);
+            if (lib.hasArrayElements(object)) {
+                if (lib.isArrayElementReadable(object, index)) {
+                    try {
+                        return getToPythonNode().executeConvert(lib.readArrayElement(object, index));
+                    } catch (UnsupportedMessageException ex) {
+                        throw raiseAttributeError(object);
+                    } catch (InvalidArrayIndexException ex) {
+                        throw raiseIndexError(index);
+                    }
+                }
+                throw raiseIndexError(index);
             }
+            throw raiseAttributeError(object);
+        }
+
+        private PException raiseIndexError(long index) {
+            return raise(IndexError, "invalid index %s", index);
+        }
+
+        private PException raiseAttributeError(Object object) {
+            return raise(AttributeError, "%s instance has no attribute '__getitem__'", object);
         }
 
         private PForeignToPTypeNode getToPythonNode() {
@@ -207,22 +231,40 @@ public Object doForeignObjectSlice(VirtualFrame frame, Object object, PSlice idx
         @Specialization
         public Object doForeignKey(Object object, String key, Object value,
                         @CachedLibrary(limit = "3") InteropLibrary lib) {
-            try {
-                lib.writeMember(object, key, value);
-                return PNone.NONE;
-            } catch (UnsupportedMessageException e) {
-                if (lib.hasArrayElements(object)) {
-                    throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
-                } else {
-                    throw raise(AttributeError, "attribute %s is read-only", key);
+            if (lib.hasMembers(object)) {
+                if (lib.isMemberWritable(object, key)) {
+                    try {
+                        lib.writeMember(object, key, value);
+                        return PNone.NONE;
+                    } catch (UnsupportedMessageException e) {
+                        throw raiseAttributeReadOnlyDisambiguated(object, key, lib);
+                    } catch (UnknownIdentifierException e) {
+                        throw raiseAttributeError(key);
+                    } catch (UnsupportedTypeException e) {
+                        throw raiseAttributeReadOnly(key);
+                    }
                 }
-            } catch (UnknownIdentifierException e) {
-                throw raise(AttributeError, "foreign object has no attribute '%s'", key);
-            } catch (UnsupportedTypeException e) {
-                throw raise(AttributeError, "attribute %s is read-only", key);
+                throw raiseAttributeReadOnlyDisambiguated(object, key, lib);
+            }
+            throw raiseAttributeError(key);
+        }
+
+        private PException raiseAttributeReadOnlyDisambiguated(Object object, String key, InteropLibrary lib) {
+            if (lib.hasArrayElements(object)) {
+                throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
+            } else {
+                throw raiseAttributeReadOnly(key);
             }
         }
 
+        private PException raiseAttributeReadOnly(String key) {
+            return raise(AttributeError, "attribute %s is read-only", key);
+        }
+
+        private PException raiseAttributeError(String key) {
+            return raise(AttributeError, "foreign object has no attribute '%s'", key);
+        }
+
         @Specialization(guards = {"!isSlice(idx)", "!isString(idx)"})
         public Object doForeignObject(Object object, Object idx, Object value,
                         @CachedLibrary(limit = "3") InteropLibrary lib,
@@ -239,13 +281,16 @@ public Object doForeignObject(Object object, Object idx, Object value,
         }
 
         private void writeForeignValue(Object object, int idx, Object value, InteropLibrary lib) throws UnsupportedMessageException, UnsupportedTypeException {
-            if (lib.isArrayElementModifiable(object, idx)) {
-                try {
-                    lib.writeArrayElement(object, idx, value);
-                    return;
-                } catch (InvalidArrayIndexException ex) {
-                    throw raise(IndexError, "invalid index %s", idx);
+            if (lib.hasArrayElements(object)) {
+                if (lib.isArrayElementWritable(object, idx)) {
+                    try {
+                        lib.writeArrayElement(object, idx, value);
+                        return;
+                    } catch (InvalidArrayIndexException ex) {
+                        // fall through
+                    }
                 }
+                throw raise(IndexError, "invalid index %s", idx);
             }
             throw raise(AttributeError, "%s instance has no attribute '__setitem__'", object);
         }
@@ -270,37 +315,58 @@ public Object doForeignObjectSlice(Object object, PSlice idxSlice,
                 }
                 return PNone.NONE;
             } catch (UnsupportedMessageException e) {
-                throw raise(AttributeError, "%s instance has no attribute '__delitem__'", object);
+                throw raiseAttributeError(object);
             }
         }
 
         @Specialization
         public Object doForeignKey(Object object, String key,
                         @CachedLibrary(limit = "3") InteropLibrary lib) {
-            try {
-                lib.removeMember(object, key);
-                return PNone.NONE;
-            } catch (UnsupportedMessageException e) {
-                if (lib.hasArrayElements(object)) {
-                    throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
-                } else {
-                    throw raise(AttributeError, "attribute %s is read-only", key);
+            if (lib.hasMembers(object)) {
+                if (lib.isMemberRemovable(object, key)) {
+                    try {
+                        lib.removeMember(object, key);
+                        return PNone.NONE;
+                    } catch (UnsupportedMessageException e) {
+                        throw raiseAttributeReadOnlyDisambiguated(object, key, lib);
+                    } catch (UnknownIdentifierException e) {
+                        throw raiseAttributeError(key);
+                    }
                 }
-            } catch (UnknownIdentifierException e) {
-                throw raise(AttributeError, "foreign object has no attribute '%s'", key);
+                throw raiseAttributeReadOnlyDisambiguated(object, key, lib);
+            }
+            throw raiseAttributeError(key);
+        }
+
+        private PException raiseAttributeError(String key) {
+            return raise(AttributeError, "foreign object has no attribute '%s'", key);
+        }
+
+        private PException raiseAttributeReadOnlyDisambiguated(Object object, String key, InteropLibrary lib) {
+            if (lib.hasArrayElements(object)) {
+                throw raise(TypeError, "'%p' object cannot be interpreted as an integer", key);
+            } else {
+                throw raise(AttributeError, "attribute %s is read-only", key);
             }
         }
 
         @Specialization(guards = "!isSlice(idx)")
         public Object doForeignObject(Object object, Object idx,
                         @Cached CastToIndexNode castIndex,
                         @CachedLibrary(limit = "3") InteropLibrary lib) {
-            try {
-                int convertedIdx = castIndex.execute(idx);
-                return removeForeignValue(object, convertedIdx, lib);
-            } catch (UnsupportedMessageException e) {
-                throw raise(AttributeError, "%s instance has no attribute '__delitem__'", object);
+            if (lib.hasArrayElements(object)) {
+                try {
+                    int convertedIdx = castIndex.execute(idx);
+                    return removeForeignValue(object, convertedIdx, lib);
+                } catch (UnsupportedMessageException e) {
+                    // fall through
+                }
             }
+            throw raiseAttributeError(object);
+        }
+
+        private PException raiseAttributeError(Object object) {
+            return raise(AttributeError, "%s instance has no attribute '__delitem__'", object);
         }
 
         private Object removeForeignValue(Object object, int idx, InteropLibrary lib) throws UnsupportedMessageException {
@@ -311,7 +377,7 @@ private Object removeForeignValue(Object object, int idx, InteropLibrary lib) th
                     throw raise(IndexError, "invalid index %s", idx);
                 }
             }
-            throw raise(AttributeError, "%s instance has no attribute '__delitem__'", object);
+            throw raiseAttributeError(object);
         }
 
         public static RemoveForeignItemNode create() {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/foreign/ForeignObjectBuiltins.java

@@ -181,7 +181,7 @@ public long len(Object self,
     }
 
     abstract static class ForeignBinaryNode extends PythonBinaryBuiltinNode {
-        @Child LookupAndCallBinaryNode op;
+        @Child private LookupAndCallBinaryNode op;
         protected final boolean reverse;
 
         protected ForeignBinaryNode(LookupAndCallBinaryNode op, boolean reverse) {
@@ -423,7 +423,7 @@ abstract static class RFloorDivNode extends ForeignBinaryNode {
     }
 
     public abstract static class ForeignBinaryComparisonNode extends PythonBinaryBuiltinNode {
-        @Child BinaryComparisonNode comparisonNode;
+        @Child private BinaryComparisonNode comparisonNode;
 
         protected ForeignBinaryComparisonNode(BinaryComparisonNode genericOp) {
             this.comparisonNode = genericOp;
@@ -634,7 +634,7 @@ public static CallNode create() {
     @Builtin(name = __GETITEM__, minNumOfPositionalArgs = 2)
     @GenerateNodeFactory
     abstract static class GetitemNode extends PythonBinaryBuiltinNode {
-        @Child AccessForeignItemNodes.GetForeignItemNode getForeignItemNode = AccessForeignItemNodes.GetForeignItemNode.create();
+        @Child private AccessForeignItemNodes.GetForeignItemNode getForeignItemNode = AccessForeignItemNodes.GetForeignItemNode.create();
 
         @Specialization
         Object doit(Object object, Object key) {
@@ -645,7 +645,7 @@ Object doit(Object object, Object key) {
     @Builtin(name = __GETATTR__, minNumOfPositionalArgs = 2)
     @GenerateNodeFactory
     abstract static class GetattrNode extends PythonBinaryBuiltinNode {
-        @Child PForeignToPTypeNode toPythonNode = PForeignToPTypeNode.create();
+        @Child private PForeignToPTypeNode toPythonNode = PForeignToPTypeNode.create();
 
         @Specialization
         protected Object doIt(Object object, String member,
@@ -705,7 +705,7 @@ protected PNone doIt(Object object, String key,
     @Builtin(name = __DELITEM__, minNumOfPositionalArgs = 2)
     @GenerateNodeFactory
     abstract static class DelitemNode extends PythonBinaryBuiltinNode {
-        AccessForeignItemNodes.RemoveForeignItemNode delForeignItemNode = AccessForeignItemNodes.RemoveForeignItemNode.create();
+        @Child private AccessForeignItemNodes.RemoveForeignItemNode delForeignItemNode = AccessForeignItemNodes.RemoveForeignItemNode.create();
 
         @Specialization
         PNone doit(Object object, Object key) {