[GR-57192] More fixes for nanobind

msimacek · msimacek · commit b5226e5e12b6 · 2024-08-16T13:52:18.000Z
PullRequest: graalpython/3444
diff --git a/README.md b/README.md
@@ -19,14 +19,14 @@ GraalPy is ready for production running pure Python code and has experimental su
 **Compatible with the Python ecosystem**
 
 * Use almost any standard Python feature, the CPython tests run on every commit and pass ~85%
+* See if the packages you need work according to our [Python Compatibility Checker](https://www.graalvm.org/python/compatibility/)
+* Support for native extension modules is considered experimental, but you can already install [packages](docs/user/Python-Runtime.md#installing-packages) like *NumPy*, *PyTorch*, or *Tensorflow*; run [Hugging Face](https://huggingface.co/) models like *Stable Diffusion* or *GPT*
 ![](docs/user/assets/mcd.svg#gh-light-mode-only)![](docs/user/assets/mcd-dark.svg#gh-dark-mode-only)<sup>
 We run the tests of the [most depended on PyPI packages](https://libraries.io/pypi) every day.
 For 96% of those packages a recent version can be installed on GraalPy and GraalPy passes about 50% of all tests of all packages combined.
 We assume that CPython not passing 100% of all tests is due to problems in our infrastructure that may also affect GraalPy.
 Packages where CPython fails all tests are marked as "not tested" for both CPython and GraalPy.
 </sup>
-* See if the packages you need work according to our [Python Compatibility Checker](https://www.graalvm.org/python/compatibility/)
-* Support for native extension modules is considered experimental, but you can already install [packages](docs/user/Python-Runtime.md#installing-packages) like *NumPy*, *PyTorch*, or *Tensorflow*; run [Hugging Face](https://huggingface.co/) models like *Stable Diffusion* or *GPT*
 
 **Runs Python code faster**
 
diff --git a/graalpython/com.oracle.graal.python.cext/include/cpython/unicodeobject.h b/graalpython/com.oracle.graal.python.cext/include/cpython/unicodeobject.h
@@ -325,7 +325,7 @@ PyAPI_FUNC(int) _PyTruffleUnicode_KIND(PyObject*);
 #define PyUnicode_KIND(op) ((enum PyUnicode_Kind)_PyTruffleUnicode_KIND(_PyObject_CAST(op)))
 
 /* Return a void pointer to the raw unicode buffer. */
-static inline void* _PyUnicode_COMPACT_DATA(PyObject *op) {
+static inline void* _PyUnicode_COMPACT_DATA(PyObject *Py_UNUSED(op)) {
     // strings are never compact in GraalPy
     return NULL;
 }
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_autopatch_capi.py b/graalpython/com.oracle.graal.python.test/src/tests/test_autopatch_capi.py
@@ -0,0 +1,82 @@
+# Copyright (c) 2024, 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
+#
+# Subject to the condition set forth below, permission is hereby granted to any
+# person obtaining a copy of this software, associated documentation and/or
+# data (collectively the "Software"), free of charge and under any and all
+# copyright rights in the Software, and any and all patent rights owned or
+# freely licensable by each licensor hereunder covering either (i) the
+# unmodified Software as contributed to or provided by such licensor, or (ii)
+# the Larger Works (as defined below), to deal in both
+#
+# (a) the Software, and
+#
+# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+# one is included with the Software each a "Larger Work" to which the Software
+# is contributed by such licensors),
+#
+# without restriction, including without limitation the rights to copy, create
+# derivative works of, display, perform, and distribute the Software and make,
+# use, sell, offer for sale, import, export, have made, and have sold the
+# Software and the Larger Work(s), and to sublicense the foregoing rights on
+# either these or other terms.
+#
+# This license is subject to the following condition:
+#
+# The above copyright notice and either this complete permission notice or at a
+# minimum a reference to the UPL must be included in all copies or substantial
+# portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+import sys
+import tempfile
+import textwrap
+
+if sys.implementation.name == 'graalpy':
+    import autopatch_capi
+
+
+    def check_autopatched(source, expected):
+        source = textwrap.dedent(source).rstrip()
+        expected = textwrap.dedent(expected).rstrip()
+        with tempfile.NamedTemporaryFile('r+') as f:
+            f.write(source)
+            f.flush()
+            autopatch_capi.auto_patch(f.name, False)
+            f.seek(0)
+            actual = f.read()
+            assert actual == expected, f"Autopatch didn't make expected changes. Expected:\n{expected}\nActual:{actual}"
+
+
+    def test_replace_field_access():
+        check_autopatched(
+            """
+            PyObject* type = obj->ob_type;
+            PyObject* type = ((PyObject*)obj)->ob_type;
+            const char* name = obj->ob_type->tp_name;
+            foo = objs[0]->ob_type;
+            return obj->ob_type;
+            return (PyObject*)obj->ob_type;
+            return ((PyObject*)obj)->ob_type;
+            (PyObject*)function_call(a, b, c(0))->ob_type->ob_base;
+            """,
+            """
+            PyObject* type = Py_TYPE(obj);
+            PyObject* type = Py_TYPE(((PyObject*)obj));
+            const char* name = Py_TYPE(obj)->tp_name;
+            foo = Py_TYPE(objs[0]);
+            return Py_TYPE(obj);
+            return (PyObject*)Py_TYPE(obj);
+            return Py_TYPE(((PyObject*)obj));
+            (PyObject*)Py_TYPE(function_call(a, b, c(0)))->ob_base;
+            """
+        )
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/PythonClassNativeWrapper.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/PythonClassNativeWrapper.java
@@ -40,6 +40,7 @@
  */
 package com.oracle.graal.python.builtins.objects.cext.capi;
 
+import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
 import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
 import com.oracle.graal.python.builtins.objects.cext.common.CArrayWrappers.CStringWrapper;
 import com.oracle.graal.python.builtins.objects.cext.structs.CFields;
@@ -49,12 +50,14 @@
 import com.oracle.graal.python.builtins.objects.type.PythonClass;
 import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
 import com.oracle.graal.python.builtins.objects.type.TypeFlags;
+import com.oracle.graal.python.builtins.objects.type.TypeNodes;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetTypeFlagsNode;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.SetTypeFlagsNode;
 import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.SetBasicSizeNodeGen;
 import com.oracle.graal.python.builtins.objects.type.TypeNodesFactory.SetItemSizeNodeGen;
 import com.oracle.graal.python.nodes.HiddenAttr;
 import com.oracle.graal.python.nodes.PGuards;
+import com.oracle.graal.python.nodes.object.GetClassNode;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives;
@@ -188,7 +191,15 @@ public Object getReplacement(InteropLibrary lib) {
              */
             PythonManagedClass clazz = (PythonManagedClass) getDelegate();
             boolean heaptype = (GetTypeFlagsNode.executeUncached(clazz) & TypeFlags.HEAPTYPE) != 0;
-            Object pointerObject = AllocateNode.allocUncached(heaptype ? CStructs.PyHeapTypeObject : CStructs.PyTypeObject);
+            long size = CStructs.PyTypeObject.size();
+            if (heaptype) {
+                size = CStructs.PyHeapTypeObject.size();
+                if (GetClassNode.executeUncached(clazz) instanceof PythonAbstractNativeObject nativeMetatype) {
+                    // TODO should call the metatype's tp_alloc
+                    size = TypeNodes.GetBasicSizeNode.executeUncached(nativeMetatype);
+                }
+            }
+            Object pointerObject = AllocateNode.allocUncached(size);
             replacement = registerReplacement(pointerObject, true, lib);
 
             ToNativeTypeNode.initializeType(this, pointerObject, heaptype);
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/AbstractFunctionBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/AbstractFunctionBuiltins.java
@@ -308,7 +308,7 @@ static TruffleString getBuiltin(PBuiltinFunction self, @SuppressWarnings("unused
 
         @TruffleBoundary
         public static TruffleString signatureToText(Signature signature, boolean skipSelf) {
-            TruffleString[] keywordNames = signature.getKeywordNames();
+            TruffleString[] keywordNames = signature.getVisibleKeywordNames();
             boolean takesVarArgs = signature.takesVarArgs();
             boolean takesVarKeywordArgs = signature.takesVarKeywordArgs();
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/BuiltinFunctionBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/BuiltinFunctionBuiltins.java
@@ -172,7 +172,7 @@ public static Object createInspectSignagure(Signature signature, boolean skipSel
             Object inspectParameter = PyObjectGetAttr.executeUncached(inspect, tsLiteral("Parameter"));
             Object[] parameterKinds = new Object[ParameterKinds.VALUES.length];
 
-            TruffleString[] keywordNames = signature.getKeywordNames();
+            TruffleString[] keywordNames = signature.getVisibleKeywordNames();
             boolean takesVarArgs = signature.takesVarArgs();
             boolean takesVarKeywordArgs = signature.takesVarKeywordArgs();
             TruffleString[] parameterNames = signature.getParameterIds();
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/Signature.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/Signature.java
@@ -27,9 +27,14 @@
 
 import static com.oracle.graal.python.nodes.BuiltinNames.T_SELF;
 import static com.oracle.graal.python.nodes.StringLiterals.T_EMPTY_STRING;
+import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
+
+import java.util.ArrayList;
+import java.util.List;
 
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
+import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.strings.TruffleString;
 
 public final class Signature {
@@ -127,6 +132,21 @@ public final TruffleString[] getKeywordNames() {
         return keywordOnlyNames;
     }
 
+    @TruffleBoundary
+    public TruffleString[] getVisibleKeywordNames() {
+        /*
+         * C slot wrappers (ab)use keyword defaults for storing the function pointers, we need to
+         * filter them out. Their names start with a dollar sign.
+         */
+        List<TruffleString> visibleKeywordNames = new ArrayList<>(keywordOnlyNames.length);
+        for (TruffleString k : keywordOnlyNames) {
+            if (k.byteLength(TS_ENCODING) > 0 && k.codePointAtByteIndexUncached(0, TS_ENCODING) != '$') {
+                visibleKeywordNames.add(k);
+            }
+        }
+        return visibleKeywordNames.toArray(TruffleString[]::new);
+    }
+
     public final boolean takesKeywordArgs() {
         return keywordOnlyNames.length > 0 || takesVarKeywordArgs;
     }
diff --git a/graalpython/lib-graalpython/modules/autopatch_capi.py b/graalpython/lib-graalpython/modules/autopatch_capi.py
@@ -81,35 +81,46 @@ def consume_whitespace_forward(idx):
 
     # scan backwards to capture the whole receiver
     idx = consume_whitespace_backwards(start - 2)
-    empty = True
-    while idx > 1:
+
+    def consume_pairwise_backwards(idx, l, r):
+        level = 1
+        idx -= 1
+        while level and idx:
+            c = contents[idx]
+            if c == l:
+                level -= 1
+            elif c == r:
+                level += 1
+            idx -= 1
+        return idx
+
+    def consume_identifier_backwards(idx):
+        while (contents[idx].isidentifier() or contents[idx].isdigit()) and idx:
+            idx -= 1
+        return idx
+
+    first = True
+    while idx:
         c = contents[idx]
-        c2 = contents[idx - 1: idx + 1]
-        if c == ')' or c == ']' or (c == '>' and c2 != '->'):
-            if level == 0 and c == ')' and not empty:
+        if c == ')' and first:
+            idx = consume_pairwise_backwards(idx, '(', ')')
+        elif c == ']':
+            idx = consume_pairwise_backwards(idx, '[', ']')
+        elif c.isidentifier() or c.isdigit():
+            id_start = consume_identifier_backwards(idx)
+            if contents[id_start + 1: idx + 1] == 'return':
                 idx += 1
-                break  # don't include casts
-            empty = False
-            level += 1
+                break
+            idx = id_start
+        elif c == '.':
             idx -= 1
-        elif level > 0 and (c == '(' or c == '[' or c == '<'):
-            level -= 1
-            idx = consume_whitespace_backwards(idx - 1)
+        elif c == '>' and idx > 1 and contents[idx - 1] == '-':
+            idx -= 2
         else:
-            if level > 0:
-                idx -= 1
-            elif c.isidentifier() or c.isdigit():
-                empty = False
-                idx -= 1
-            elif c == '.':
-                empty = True
-                idx = consume_whitespace_backwards(idx - 1)
-            elif c2 == '->':
-                empty = True
-                idx = consume_whitespace_backwards(idx - 2)
-            else:
-                idx += 1
-                break
+            idx += 1
+            break
+        first = False
+        idx = consume_whitespace_backwards(idx)
 
     receiver_start = consume_whitespace_forward(idx)
     receiver_string = contents[receiver_start: start - 1]
@@ -122,7 +133,6 @@ def consume_whitespace_forward(idx):
             # this looks like an assignment, determine the value
             idx = consume_whitespace_forward(idx + 1)
             value_start = idx
-            empty = True
             while idx < len(contents) - 2:
                 c = contents[idx]
                 c2 = contents[idx: idx + 2]
@@ -134,7 +144,7 @@ def consume_whitespace_forward(idx):
                     idx += 1
                 else:
                     if level == 0 and (c == ')' or c == ';' or c == ','):
-                        break;
+                        break
                     if c2 == '<=' or c2 == '<<' or c2 == '>='or c2 == '>>' or c2 == '->':
                         idx += 2
                     else:
diff --git a/graalpython/lib-graalpython/patches/nanobind/nanobind.patch b/graalpython/lib-graalpython/patches/nanobind/nanobind.patch
@@ -942,26 +942,26 @@ index c49b7bb..3a5be09 100644
  #    define NB_TYPE_GET_SLOT_IMPL 1 // Custom implementation of nb::type_get_slot
  #  else
 diff --git a/nanobind/include/nanobind/nb_types.h b/nanobind/include/nanobind/nb_types.h
-index 2761a58..b83a651 100644
+index b83a651..de16f03 100644
 --- a/nanobind/include/nanobind/nb_types.h
 +++ b/nanobind/include/nanobind/nb_types.h
-@@ -873,14 +873,14 @@ inline detail::dict_iterator dict::end() const { return { }; }
- 
- #if !defined(Py_LIMITED_API) && !defined(PYPY_VERSION)
- inline detail::fast_iterator tuple::begin() const {
--    PySequence_Fast_ITEMS((PyObject*)return ((PyTupleObject *) m_ptr));
-+    return PySequence_Fast_ITEMS((PyObject*)((PyTupleObject *) m_ptr));
+@@ -877,14 +877,14 @@ inline detail::fast_iterator tuple::begin() const {
  }
  inline detail::fast_iterator tuple::end() const {
      PyTupleObject *v = (PyTupleObject *) m_ptr;
-     return PySequence_Fast_ITEMS((PyObject*)v) + v->ob_base.ob_size;
+-    return PySequence_Fast_ITEMS((PyObject*)v) + v->ob_base.ob_size;
++    return PySequence_Fast_ITEMS((PyObject*)v) + Py_SIZE(v);
  }
  inline detail::fast_iterator list::begin() const {
--    PySequence_Fast_ITEMS((PyObject*)return ((PyListObject *) m_ptr));
-+    return PySequence_Fast_ITEMS((PyObject*)((PyListObject *) m_ptr));
+     return PySequence_Fast_ITEMS((PyObject*)((PyListObject *) m_ptr));
  }
  inline detail::fast_iterator list::end() const {
      PyListObject *v = (PyListObject *) m_ptr;
+-    return PySequence_Fast_ITEMS((PyObject*)v) + v->ob_base.ob_size;
++    return PySequence_Fast_ITEMS((PyObject*)v) + Py_SIZE(v);
+ }
+ #endif
+ 
 diff --git a/nanobind/include/nanobind/stl/detail/chrono.h b/nanobind/include/nanobind/stl/detail/chrono.h
 index 39bd5d9..6e9efb0 100644
 --- a/nanobind/include/nanobind/stl/detail/chrono.h

Original file line number	Diff line number	Diff line change
`@@ -325,7 +325,7 @@ PyAPI_FUNC(int) _PyTruffleUnicode_KIND(PyObject*);`
`325`	`325`	`#define PyUnicode_KIND(op) ((enum PyUnicode_Kind)_PyTruffleUnicode_KIND(_PyObject_CAST(op)))`
`326`	`326`
`327`	`327`	`/* Return a void pointer to the raw unicode buffer. */`
`328`		`-static inline void* _PyUnicode_COMPACT_DATA(PyObject *op) {`
	`328`	`+static inline void* _PyUnicode_COMPACT_DATA(PyObject *Py_UNUSED(op)) {`
`329`	`329`	`// strings are never compact in GraalPy`
`330`	`330`	`return NULL;`
`331`	`331`	`}`