Migrate foreign.__delattr__ to tp_setattro

Author: steve-s

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

@@ -37,7 +37,6 @@
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___AND__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___CALL__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___CONTAINS__;
-import static com.oracle.graal.python.nodes.SpecialMethodNames.J___DELATTR__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___DELITEM__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___DIR__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.J___DIVMOD__;
@@ -1038,13 +1037,13 @@ static Object doIt(Node inliningTarget, Object object, Object memberObj,
     @Slot(value = SlotKind.tp_setattro, isComplex = true)
     @GenerateNodeFactory
     abstract static class SetattrNode extends SetAttrBuiltinNode {
-        @Specialization
-        static void doIt(Object object, Object key, Object value,
+        @Specialization(guards = "!isNoValue(value)")
+        static void doSet(Object object, Object key, Object value,
                         @Bind("this") Node inliningTarget,
-                        @CachedLibrary(limit = "3") InteropLibrary lib,
-                        @Cached CastToJavaStringNode castToString,
-                        @Cached GilNode gil,
-                        @Cached PRaiseNode.Lazy raiseNode) {
+                        @Shared @CachedLibrary(limit = "3") InteropLibrary lib,
+                        @Shared @Cached CastToJavaStringNode castToString,
+                        @Shared @Cached GilNode gil,
+                        @Shared @Cached PRaiseNode.Lazy raiseNode) {
             gil.release(true);
             try {
                 lib.writeMember(object, castToString.execute(key), value);
@@ -1056,30 +1055,14 @@ static void doIt(Object object, Object key, Object value,
                 gil.acquire();
             }
         }
-    }
-
-    @Builtin(name = J___SETITEM__, minNumOfPositionalArgs = 3)
-    @GenerateNodeFactory
-    abstract static class SetitemNode extends PythonTernaryBuiltinNode {
-        @Child private AccessForeignItemNodes.SetForeignItemNode setForeignItemNode = AccessForeignItemNodes.SetForeignItemNode.create();
-
-        @Specialization
-        Object doit(VirtualFrame frame, Object object, Object key, Object value) {
-            setForeignItemNode.execute(frame, object, key, value);
-            return PNone.NONE;
-        }
-    }
 
-    @Builtin(name = J___DELATTR__, minNumOfPositionalArgs = 2)
-    @GenerateNodeFactory
-    abstract static class DelattrNode extends PythonBinaryBuiltinNode {
-        @Specialization
-        static PNone doIt(Object object, Object key,
+        @Specialization(guards = "isNoValue(value)")
+        static void doDelete(Object object, Object key, @SuppressWarnings("unused") PNone value,
                         @Bind("this") Node inliningTarget,
-                        @CachedLibrary(limit = "3") InteropLibrary lib,
-                        @Cached CastToJavaStringNode castToString,
-                        @Cached GilNode gil,
-                        @Cached PRaiseNode.Lazy raiseNode) {
+                        @Shared @CachedLibrary(limit = "3") InteropLibrary lib,
+                        @Shared @Cached CastToJavaStringNode castToString,
+                        @Shared @Cached GilNode gil,
+                        @Shared @Cached PRaiseNode.Lazy raiseNode) {
             gil.release(true);
             try {
                 lib.removeMember(object, castToString.execute(key));
@@ -1090,6 +1073,17 @@ static PNone doIt(Object object, Object key,
             } finally {
                 gil.acquire();
             }
+        }
+    }
+
+    @Builtin(name = J___SETITEM__, minNumOfPositionalArgs = 3)
+    @GenerateNodeFactory
+    abstract static class SetitemNode extends PythonTernaryBuiltinNode {
+        @Child private AccessForeignItemNodes.SetForeignItemNode setForeignItemNode = AccessForeignItemNodes.SetForeignItemNode.create();
+
+        @Specialization
+        Object doit(VirtualFrame frame, Object object, Object key, Object value) {
+            setForeignItemNode.execute(frame, object, key, value);
             return PNone.NONE;
         }
     }

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

@@ -845,20 +845,21 @@ public static void initializeBuiltinSlots(PythonLanguage language) {
         }
     }
 
-    private static boolean checkNoMagicOverrides(PythonBuiltinClassType type) {
+    private static boolean checkNoMagicOverrides(Python3Core core, PythonBuiltinClassType type) {
         // Check that no one is trying to define magic methods directly
         // If the assertion fires: you should define @Slot instead of @Builtin
         // We do not look in MRO, we may have already called addOperatorsToBuiltin on super
         var readAttr = ReadAttributeFromObjectNode.getUncachedForceType();
+        PythonBuiltinClass typeObj = core.lookupType(type);
         for (TruffleString name : SPECIAL2SLOT.keySet()) {
-            assert readAttr.execute(type, name) == PNone.NO_VALUE : name;
+            assert readAttr.execute(typeObj, name) == PNone.NO_VALUE : type.name() + ":" + name;
         }
         return true;
     }
 
     public static void addOperatorsToBuiltin(Map<TruffleString, BoundBuiltinCallable<?>> builtins, Python3Core core, PythonBuiltinClassType type) {
         TpSlots slots = type.getDeclaredSlots();
-        assert checkNoMagicOverrides(type);
+        assert checkNoMagicOverrides(core, type);
 
         // Similar to CPython:add_operators
         for (var slotDefGroup : SLOTDEFS.entrySet()) {
@@ -868,7 +869,7 @@ public static void addOperatorsToBuiltin(Map<TruffleString, BoundBuiltinCallable
                 continue;
             }
             for (TpSlotDef slotDef : slotDefGroup.getValue()) {
-                if (slotDef.wrapper() != null) {
+                if (slotDef.wrapper() != null && !builtins.containsKey(slotDef.name())) {
                     var value = builtinSlot.createBuiltin(core, type, slotDef.name(), slotDef.wrapper());
                     builtins.put(slotDef.name(), value);
                 }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectSetAttr.java

@@ -97,7 +97,7 @@ public final void deleteCached(Frame frame, Object receiver, TruffleString name)
     }
 
     public final void delete(Frame frame, Node inliningTarget, Object receiver, TruffleString name) {
-        execute(frame, inliningTarget, receiver, name, null);
+        execute(frame, inliningTarget, receiver, name, PNone.NO_VALUE);
     }
 
     @Specialization(guards = {"name == cachedName", "value != null"}, limit = "1")