implemented downcall for native objects in PyObject_Size

tomasstupka · tomasstupka · commit 2cf0e0848b55 · 2022-01-20T19:41:07.000+01:00
diff --git a/graalpython/com.oracle.graal.python.cext/src/abstract.c b/graalpython/com.oracle.graal.python.cext/src/abstract.c
@@ -292,7 +292,7 @@ Py_ssize_t PySequence_Size(PyObject *s) {
 
 // PySequence_Size downcall for native python objects
 // taken from CPython "Objects/abstract.c/Py_Sequence_Size"
-Py_ssize_t PyTruffle_Sequence_Size(PyObject *s) {
+Py_ssize_t PyTruffle_PySequence_Size(PyObject *s) {
     PySequenceMethods *m;
 
     if (s == NULL) {
@@ -372,10 +372,31 @@ PyObject * PyMapping_GetItemString(PyObject *o, const char *key) {
     return _jls_PyObject_GetItem(native_to_java(o), polyglot_from_string(key, SRC_CS));
 }
 
+UPCALL_ID(PyObject_Size);
 Py_ssize_t PyObject_Size(PyObject *o) {
     return UPCALL_CEXT_L(_jls_PyObject_Size, native_to_java(o));
 }
 
+// PyObject_Size downcall for native python objects
+// taken from CPython "Objects/abstract.c/Py_Object_Size"
+Py_ssize_t PyTruffle_PyObject_Size(PyObject *o) {
+    PySequenceMethods *m;
+
+    if (o == NULL) {
+        null_error();
+        return -1;
+    }
+
+    m = o->ob_type->tp_as_sequence;
+    if (m && m->sq_length) {
+        Py_ssize_t len = m->sq_length(o);
+        assert(len >= 0 || PyErr_Occurred());
+        return len;
+    }
+
+    return PyMapping_Size(o);
+}
+
 UPCALL_ID(PyMapping_Keys);
 PyObject * PyMapping_Keys(PyObject *o) {
     return UPCALL_CEXT_O(_jls_PyMapping_Keys, native_to_java(o));
@@ -553,7 +574,7 @@ Py_ssize_t PyMapping_Size(PyObject *s) {
 
 // PyMapping_Size downcall for native python objects
 // partially taken from CPython "Objects/abstract.c/Py_Mapping_Size"
-Py_ssize_t PyTruffle_Mapping_Size(PyObject *o) {
+Py_ssize_t PyTruffle_PyMapping_Size(PyObject *o) {
     PyMappingMethods *m;
 
     if (o == NULL) {
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_abstract.py
@@ -107,22 +107,47 @@ def _reference_next(args):
 
 def _reference_seq_size(args):
     seq = args[0]
-    # XXX check
-    if isinstance(seq, dict) or isinstance(seq, type(type.__dict__)) or isinstance(seq, set) or isinstance(seq, frozenset) or not hasattr(seq, '__len__'):
-        if sys.version_info.minor >= 6:
-            raise SystemError
-        else:
-            raise TypeError
-    return len(seq)
+    if can_be_seq(seq):
+        return len(seq)
+    if sys.version_info.minor >= 6:
+        raise SystemError
+    else:
+        raise TypeError
 
 def _reference_mapping_size(args):
     m = args[0]
-    if not (isinstance(m, dict) or isinstance(m, type(type.__dict__))) or not hasattr(m, '__getitem__'):
-        if sys.version_info.minor >= 6:
-            raise SystemError
-        else:
-            raise TypeError
-    return len(m)    
+    if can_be_mapping(m): 
+        return len(m)    
+    if sys.version_info.minor >= 6:
+        raise SystemError
+    else:
+        raise TypeError
+        
+def _reference_object_size(args):
+    o = args[0]        
+    if can_be_seq(o) or can_be_mapping(o):
+        return len(o)
+    if sys.version_info.minor >= 6:
+        raise SystemError
+    else:
+        raise TypeError()
+
+def can_be_seq(obj):
+    # XXX check seq/frozenseq
+    if isinstanceof(obj, [dict, type(type.__dict__), set, frozenset]):
+        return False
+    return hasattr(obj, '__getitem__')
+
+def can_be_mapping(obj):
+    if isinstanceof(obj, [str, bytes, list, tuple, set, frozenset, memoryview, range]):
+        return False
+    return hasattr(obj, '__getitem__')
+
+def isinstanceof(obj, types):
+    for t in types:
+        if isinstance(obj, t):
+            return True
+    return False    
 
 def _reference_getitem(args):
     seq = args[0]
@@ -195,8 +220,10 @@ def __float__(self):
 class DummySequence():
 
     def __getitem__(self, idx):
-        return idx * 10
-
+        raise IndexError
+    
+    def __len__(self):
+        return 0
 
 class DummyListSubclass(list):
     pass
@@ -774,6 +801,7 @@ def compile_module(self, name):
             ([None],),
             (set(),),
             (frozenset(),),
+            (DummySequence(),),
             (DummyListSubclass(),),
             ('hello',),
             ({},),
@@ -981,6 +1009,7 @@ def compile_module(self, name):
             ([None],),
             (set(),),
             (frozenset(),),
+            (DummySequence(),),
             (DummyListSubclass(),),
             ('hello',),
             ({},),
@@ -1097,3 +1126,31 @@ def compile_module(self, name):
         arguments=["PyObject* s", "PyObject* o"],
         cmpfunc=unhandled_error_compare
     )
+
+    test_PyObject_Size = CPyExtFunction(
+        _reference_object_size,
+        lambda: (
+            (tuple(),),
+            ((1, 2, 3),),
+            ((None,),),
+            ([],),
+            (['a', 'b', 'c'],),
+            ([None],),
+            (set(),),
+            (frozenset(),),
+            (DummySequence(),),
+            (DummyListSubclass(),),
+            ('hello',),
+            ({},),
+            ({1, 1},),
+            (type.__dict__,), #mappingproxy
+            (NoNumber(),),
+            (sys.modules, ),
+        ),
+        resultspec="n",
+        argspec='O',
+        arguments=["PyObject* obj"],
+        cmpfunc=unhandled_error_compare
+    )
+
+    test_PyObject_Length = test_PyObject_Size
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_functions.py
@@ -90,22 +90,7 @@ def is_buffer(x):
     # PyObject_Del
     # PyObject_FREE
     # PyObject_Free
-    def forgiving_len(o):
-        try:
-            return len(o)
-        except TypeError:
-            return -1
 
-    test_PyObject_Length = CPyExtFunction(
-        forgiving_len,
-        lambda: ([], [1, 2, 3, 4], (1,), sys.modules),
-        resultspec="i",
-    )
-    test_PyObject_Size = CPyExtFunction(
-        forgiving_len,
-        lambda: ([], [1, 2, 3, 4], (1,), {1:1}, sys.modules),
-        resultspec="i",
-    )
     # PyObject_MALLOC
     # PyObject_Malloc
     # PyObject_New
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextAbstractBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextAbstractBuiltins.java
@@ -40,8 +40,9 @@
  */
 package com.oracle.graal.python.builtins.modules.cext;
 
-import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_MAPPING_SIZE;
-import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_SEQUENCE_SIZE;
+import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_PY_MAPPING_SIZE;
+import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_PY_OBJECT_SIZE;
+import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_PY_SEQUENCE_SIZE;
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
 import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
 import static com.oracle.graal.python.nodes.ErrorMessages.BASE_MUST_BE;
@@ -993,8 +994,8 @@ static Object doNative(VirtualFrame frame, Object obj,
                         @Cached AsPythonObjectNode asPythonObjectNode,
                         @Cached PCallCapiFunction callCapiFunction,
                         @Cached DefaultCheckFunctionResultNode checkFunctionResultNode) {
-            Object result = callCapiFunction.call(FUN_PY_TRUFFLE_SEQUENCE_SIZE, toSulongNode.execute(obj));
-            checkFunctionResultNode.execute(PythonContext.get(callCapiFunction), FUN_PY_TRUFFLE_SEQUENCE_SIZE.getName(), result);
+            Object result = callCapiFunction.call(FUN_PY_TRUFFLE_PY_SEQUENCE_SIZE, toSulongNode.execute(obj));
+            checkFunctionResultNode.execute(PythonContext.get(callCapiFunction), FUN_PY_TRUFFLE_PY_SEQUENCE_SIZE.getName(), result);
             return asPythonObjectNode.execute(result);
         }
 
@@ -1074,6 +1075,23 @@ static Object doGenericUnboxed(VirtualFrame frame, Object obj,
         protected static boolean isMappingOrSequence(Object obj) {
             return obj instanceof PSequence || obj instanceof PHashingCollection;
         }
+
+        @Specialization(guards = {"isNativeObject(obj)"})
+        static Object size(VirtualFrame frame, Object obj,
+                        @Cached ToSulongNode toSulongNode,
+                        @Cached AsPythonObjectNode asPythonObjectNode,
+                        @Cached PCallCapiFunction callCapiFunction,
+                        @Cached DefaultCheckFunctionResultNode checkFunctionResultNode) {
+            Object result = callCapiFunction.call(FUN_PY_TRUFFLE_PY_OBJECT_SIZE, toSulongNode.execute(obj));
+            checkFunctionResultNode.execute(PythonContext.get(callCapiFunction), FUN_PY_TRUFFLE_PY_OBJECT_SIZE.getName(), result);
+            return asPythonObjectNode.execute(result);
+        }
+
+        @Specialization(guards = {"!isNativeObject(obj)", "!isMappingOrSequence(obj)"})
+        static Object size(VirtualFrame frame, Object obj,                        
+                        @Cached PRaiseNativeNode raiseNativeNode) {
+            return raiseNativeNode.raiseInt(frame, -1, TypeError, ErrorMessages.OBJ_HAS_NO_LEN, obj);
+        }
     }
 
     /////// PyMapping ///////
@@ -1184,9 +1202,8 @@ public Object values(VirtualFrame frame, Object obj,
     @ImportStatic(SpecialMethodNames.class)
     abstract static class PyMappingSizeNode extends PythonUnaryBuiltinNode {
 
-        @Specialization(guards = "checkNode.execute(obj)")
-        static int doSequence(VirtualFrame frame, Object obj,
-                        @SuppressWarnings("unused") @Cached PyMappingCheckNode checkNode,
+        @Specialization
+        static int doSequence(VirtualFrame frame, PHashingCollection obj,
                         @Cached PyObjectSizeNode sizeNode,
                         @Cached TransformExceptionToNativeNode transformExceptionToNativeNode) {
             try {
@@ -1203,17 +1220,20 @@ static Object doNative(VirtualFrame frame, Object obj,
                         @Cached AsPythonObjectNode asPythonObjectNode,
                         @Cached PCallCapiFunction callCapiFunction,
                         @Cached DefaultCheckFunctionResultNode checkFunctionResultNode) {
-            Object result = callCapiFunction.call(FUN_PY_TRUFFLE_MAPPING_SIZE, toSulongNode.execute(obj));
-            checkFunctionResultNode.execute(PythonContext.get(callCapiFunction), FUN_PY_TRUFFLE_MAPPING_SIZE.getName(), result);
+            Object result = callCapiFunction.call(FUN_PY_TRUFFLE_PY_MAPPING_SIZE, toSulongNode.execute(obj));
+            checkFunctionResultNode.execute(PythonContext.get(callCapiFunction), FUN_PY_TRUFFLE_PY_MAPPING_SIZE.getName(), result);
             return asPythonObjectNode.execute(result);
         }
 
-        @Specialization(guards = {"!isNativeObject(obj)", "!checkNode.execute(obj)"})
+        @Specialization(guards = {"!isNativeObject(obj)", "!isMapping(obj)"})
         Object notSequence(VirtualFrame frame, Object obj,
-                        @SuppressWarnings("unused") @Cached PyMappingCheckNode checkNode,
                         @Cached PRaiseNativeNode raiseNativeNode) {
             return raiseNativeNode.raiseInt(frame, -1, TypeError, OBJ_ISNT_MAPPING, obj);
         }
+
+        protected boolean isMapping(Object obj) {
+            return obj instanceof PHashingCollection;
+        }
     }
 
     /////// PyIter ///////
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -164,11 +164,12 @@ public enum NativeCAPISymbol implements NativeCExtSymbol {
     FUN_GET_LONG_BITS_PER_DIGIT("get_long_bits_in_digit"),
     FUN_BULK_SUBREF("PyTruffle_bulk_SUBREF"),
     FUN_TRUFFLE_ADD_SUBOFFSET("truffle_add_suboffset"),
-    FUN_PY_TRUFFLE_MAPPING_SIZE("PyTruffle_Mapping_Size"),
+    FUN_PY_TRUFFLE_PY_MAPPING_SIZE("PyTruffle_PyMapping_Size"),
     FUN_PY_TRUFFLE_MEMORYVIEW_FROM_BUFFER("PyTruffle_MemoryViewFromBuffer"),
     FUN_PY_TRUFFLE_MEMORYVIEW_FROM_OBJECT("PyTruffle_MemoryViewFromObject"),
+    FUN_PY_TRUFFLE_PY_OBJECT_SIZE("PyTruffle_PyObject_Size"),
     FUN_PY_TRUFFLE_RELEASE_BUFFER("PyTruffle_ReleaseBuffer"),
-    FUN_PY_TRUFFLE_SEQUENCE_SIZE("PyTruffle_Sequence_Size"),
+    FUN_PY_TRUFFLE_PY_SEQUENCE_SIZE("PyTruffle_PySequence_Size"),
     FUN_GET_INT_T_TYPEID("get_int_t_typeid"),
     FUN_GET_INT8_T_TYPEID("get_int8_t_typeid"),
     FUN_GET_INT16_T_TYPEID("get_int16_t_typeid"),
