Skip to content

Commit ecccabc

Browse files
msimacekmsimacek
committed
[GR-59058] Fix default tp_free etc slots for builtin classes
PullRequest: graalpython/3562
2 parents 7886cf4 + 164ee05 commit ecccabc

File tree

5 files changed

+34
-45
lines changed

5 files changed

+34
-45
lines changed

Diff for: ‎graalpython/com.oracle.graal.python.cext/src/capi.c

+4
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,10 @@ PyAPI_FUNC(int) truffle_no_op_clear(PyObject* o) {
901901
return 0;
902902
}
903903

904+
PyAPI_FUNC(int) truffle_no_op_traverse(PyObject *self, visitproc visit, void *arg) {
905+
return 0;
906+
}
907+
904908
// defined in 'exceptions.c'
905909
void initialize_exceptions();
906910
// defined in 'pyhash.c'

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

+27-26
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,13 @@
4343
import static com.oracle.graal.python.builtins.objects.PNone.NO_VALUE;
4444
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_GRAALPY_OBJECT_GC_DEL;
4545
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_NO_OP_CLEAR;
46+
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_NO_OP_TRAVERSE;
4647
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PTR_COMPARE;
4748
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_DEALLOC;
4849
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_OBJECT_FREE;
4950
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_MEMORYVIEW_FROM_OBJECT;
5051
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TYPE_GENERIC_ALLOC;
52+
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_SUBTYPE_TRAVERSE;
5153
import static com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper.IMMORTAL_REFCNT;
5254
import static com.oracle.graal.python.builtins.objects.cext.structs.CConstants.PYLONG_BITS_IN_DIGIT;
5355
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyFloatObject__ob_fval;
@@ -142,7 +144,6 @@
142144
import com.oracle.graal.python.builtins.objects.traceback.MaterializeLazyTracebackNode;
143145
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
144146
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
145-
import com.oracle.graal.python.builtins.objects.type.PythonClass;
146147
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
147148
import com.oracle.graal.python.builtins.objects.type.TypeFlags;
148149
import com.oracle.graal.python.builtins.objects.type.TypeNodes;
@@ -898,33 +899,33 @@ public static PCallCapiFunction getUncached() {
898899
*/
899900
@TruffleBoundary
900901
public static Object lookupNativeMemberInMRO(PythonManagedClass cls, @SuppressWarnings("unused") CFields nativeMemberName, HiddenAttr managedMemberName) {
901-
if (cls instanceof PythonClass) {
902-
NativeCAPISymbol symbol = null;
903-
// We need to point to PyType_GenericAlloc or PyObject_GC_Del
904-
if (managedMemberName == HiddenAttr.ALLOC) {
905-
symbol = FUN_PY_TYPE_GENERIC_ALLOC;
906-
} else if (managedMemberName == HiddenAttr.FREE) {
907-
/*
908-
* See 'typeobject.c: inherit_slots': A bit of magic to plug in the correct default
909-
* tp_free function when a derived class adds gc, didn't define tp_free, and the
910-
* base uses the default non-gc tp_free.
911-
*/
912-
if ((GetTypeFlagsNode.executeUncached(cls) & TypeFlags.HAVE_GC) != 0) {
913-
symbol = FUN_GRAALPY_OBJECT_GC_DEL;
914-
} else {
915-
symbol = FUN_PY_OBJECT_FREE;
916-
}
917-
} else if (managedMemberName == HiddenAttr.CLEAR) {
918-
// This will need to be subtype_clear when we implement native GC
919-
symbol = FUN_NO_OP_CLEAR;
902+
NativeCAPISymbol symbol = null;
903+
// We need to point to PyType_GenericAlloc or PyObject_GC_Del
904+
if (managedMemberName == HiddenAttr.ALLOC) {
905+
symbol = FUN_PY_TYPE_GENERIC_ALLOC;
906+
} else if (managedMemberName == HiddenAttr.FREE) {
907+
/*
908+
* See 'typeobject.c: inherit_slots': A bit of magic to plug in the correct default
909+
* tp_free function when a derived class adds gc, didn't define tp_free, and the base
910+
* uses the default non-gc tp_free.
911+
*/
912+
if ((GetTypeFlagsNode.executeUncached(cls) & TypeFlags.HAVE_GC) != 0) {
913+
symbol = FUN_GRAALPY_OBJECT_GC_DEL;
914+
} else {
915+
symbol = FUN_PY_OBJECT_FREE;
920916
}
921-
if (symbol != null) {
922-
Object func = HiddenAttr.ReadNode.executeUncached(cls, managedMemberName, null);
923-
if (func != null) {
924-
return func;
925-
}
926-
return CApiContext.getNativeSymbol(null, symbol);
917+
} else if (managedMemberName == HiddenAttr.TRAVERSE) {
918+
symbol = cls instanceof PythonBuiltinClass ? FUN_NO_OP_TRAVERSE : FUN_SUBTYPE_TRAVERSE;
919+
} else if (managedMemberName == HiddenAttr.CLEAR) {
920+
// This will need to be subtype_clear when we implement native GC
921+
symbol = FUN_NO_OP_CLEAR;
922+
}
923+
if (symbol != null) {
924+
Object func = HiddenAttr.ReadNode.executeUncached(cls, managedMemberName, null);
925+
if (func != null) {
926+
return func;
927927
}
928+
return CApiContext.getNativeSymbol(null, symbol);
928929
}
929930
MroSequenceStorage mroStorage = GetMroStorageNode.executeUncached(cls);
930931
int n = mroStorage.length();

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeCAPISymbol.java

+1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public enum NativeCAPISymbol implements NativeCExtSymbol {
7272
FUN_VA_ARG_POINTER("truffle_va_arg_pointer", Pointer, Pointer),
7373
FUN_CONVERT_POINTER("truffle_convert_pointer", Pointer, Py_ssize_t),
7474
FUN_NO_OP_CLEAR("truffle_no_op_clear", Int, PyObject),
75+
FUN_NO_OP_TRAVERSE("truffle_no_op_traverse", Int, PyObject, Pointer, Pointer),
7576

7677
FUN_PYTRUFFLE_CONSTANTS("PyTruffle_constants", PY_SSIZE_T_PTR),
7778
FUN_PYTRUFFLE_STRUCT_OFFSETS("PyTruffle_struct_offsets", PY_SSIZE_T_PTR),

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/ToNativeTypeNode.java

+1-17
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import com.oracle.graal.python.builtins.objects.method.PDecoratedMethod;
7777
import com.oracle.graal.python.builtins.objects.type.MethodsFlags;
7878
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
79-
import com.oracle.graal.python.builtins.objects.type.PythonClass;
8079
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
8180
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
8281
import com.oracle.graal.python.builtins.objects.type.TpSlots;
@@ -321,22 +320,7 @@ static void initializeType(PythonClassNativeWrapper obj, Object mem, boolean hea
321320
Object tpTraverse = nullValue;
322321
Object tpIsGc = nullValue;
323322
if ((flags & TypeFlags.HAVE_GC) != 0) {
324-
/*
325-
* In CPython, classes created via 'type_new' will always have 'subtype_traverse' (this
326-
* is done in 'typeobject.c: type_new_alloc'). We don't set this in our 'CreateTypeNode'
327-
* already because we don't know if the class will ever be used in native code. So, we
328-
* do it here.
329-
*/
330-
if (clazz instanceof PythonClass) {
331-
tpTraverse = CApiContext.getNativeSymbol(null, NativeCAPISymbol.FUN_SUBTYPE_TRAVERSE);
332-
} else {
333-
tpTraverse = lookup(clazz, PyTypeObject__tp_traverse, HiddenAttr.TRAVERSE);
334-
}
335-
/*
336-
* We allow 'tp_traverse' to be null for built-in classes until we've implemented for
337-
* all of them. Once this is done, 'tp_traverse' must not be null here for all classes.
338-
*/
339-
assert clazz instanceof PythonBuiltinClass || tpTraverse != nullValue;
323+
tpTraverse = lookup(clazz, PyTypeObject__tp_traverse, HiddenAttr.TRAVERSE);
340324
tpIsGc = lookup(clazz, PyTypeObject__tp_is_gc, HiddenAttr.IS_GC);
341325
}
342326
writePtrNode.write(mem, CFields.PyTypeObject__tp_traverse, tpTraverse);

Diff for: ‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/transitions/CApiTransitions.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -994,8 +994,7 @@ static long doGeneric(Node inliningTarget, PythonAbstractObjectNativeWrapper wra
994994
@Cached(inline = false) CStructAccess.ReadI32Node readI32Node,
995995
@Cached(inline = false) CStructAccess.WriteIntNode writeIntNode,
996996
@Cached(inline = false) CStructAccess.GetElementPtrNode getElementPtrNode,
997-
@Cached CoerceNativePointerToLongNode coerceToLongNode,
998-
@Cached PyObjectGCTrackNode gcTrackNode) {
997+
@Cached CoerceNativePointerToLongNode coerceToLongNode) {
999998

1000999
log(wrapper);
10011000
pollReferenceQueue();

0 commit comments

Comments
 (0)