Skip to content

Commit 287ac2b

Browse files
committed
Fix allocating instances of managed subclasses of native classes
1 parent 9c7397f commit 287ac2b

File tree

8 files changed

+133
-64
lines changed

8 files changed

+133
-64
lines changed

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_bytes.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,3 +619,9 @@ def test_builtins(self):
619619
assert b.replace(b'e', b'a') == b'hallo'
620620
assert b.upper() == b'HELLO'
621621
assert BytesSubclass(b'a,b').split(BytesSubclass(b',')) == [b'a', b'b']
622+
623+
def test_managed_subclass(self):
624+
class ManagedSubclass(BytesSubclass):
625+
pass
626+
627+
assert is_native_object(ManagedSubclass(b"hello"))

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_err.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,12 @@ def test_init(self):
666666
assert type(e) == ExceptionSubclass
667667
assert isinstance(e, Exception)
668668

669+
def test_managed_subtype(self):
670+
class ManagedSubclass(ExceptionSubclass):
671+
pass
672+
673+
assert is_native_object(ManagedSubclass())
674+
669675
def test_raise_type(self):
670676
try:
671677
raise ExceptionSubclass

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_tuple.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,9 @@ def test_builtins(self):
267267
assert repr(t) == "(1, 2, 3)"
268268
assert t.index(2) == 1
269269
assert t.count(2) == 1
270+
271+
def test_managed_sublclass(self):
272+
class ManagedSubclass(TupleSubclass):
273+
pass
274+
275+
assert is_native_object(ManagedSubclass(1, 2, 3))

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 63 additions & 42 deletions
Large diffs are not rendered by default.

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,13 @@ protected static NativeCAPISymbol getFunction(String typenamePrefix) {
264264
return result;
265265
}
266266

267-
@Specialization(guards = "needsNativeAllocation(object)")
267+
@Specialization
268268
Object callNativeConstructor(Object object, Object arg,
269269
@Cached ToSulongNode toSulongNode,
270270
@Cached NativeToPythonNode toJavaNode,
271271
@CachedLibrary(limit = "1") InteropLibrary interopLibrary,
272272
@Cached ImportCAPISymbolNode importCAPISymbolNode) {
273+
assert TypeNodes.NeedsNativeAllocationNode.executeUncached(object);
273274
try {
274275
Object result = interopLibrary.execute(importCAPISymbolNode.execute(getFunction()), toSulongNode.execute(object), arg);
275276
return toJavaNode.execute(result);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/PBaseException.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import com.oracle.graal.python.builtins.objects.traceback.PTraceback;
5353
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
5454
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
55+
import com.oracle.graal.python.builtins.objects.type.TypeNodes;
5556
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
5657
import com.oracle.graal.python.lib.PyExceptionInstanceCheckNode;
5758
import com.oracle.graal.python.nodes.PRaiseNode;
@@ -104,7 +105,7 @@ public final class PBaseException extends PythonObject {
104105

105106
public PBaseException(Object cls, Shape instanceShape, Object[] exceptionAttributes, PTuple args) {
106107
super(cls, instanceShape);
107-
assert !(cls instanceof PythonNativeClass);
108+
assert !TypeNodes.NeedsNativeAllocationNode.executeUncached(cls);
108109
this.exceptionAttributes = exceptionAttributes;
109110
this.args = args;
110111
this.hasMessageFormat = false;
@@ -114,7 +115,7 @@ public PBaseException(Object cls, Shape instanceShape, Object[] exceptionAttribu
114115

115116
public PBaseException(Object cls, Shape instanceShape, Object[] exceptionAttributes) {
116117
super(cls, instanceShape);
117-
assertContainsNoJavaString(exceptionAttributes);
118+
assert !TypeNodes.NeedsNativeAllocationNode.executeUncached(cls);
118119
assert !(cls instanceof PythonNativeClass);
119120
this.exceptionAttributes = exceptionAttributes;
120121
this.args = null;
@@ -125,7 +126,7 @@ public PBaseException(Object cls, Shape instanceShape, Object[] exceptionAttribu
125126

126127
public PBaseException(Object cls, Shape instanceShape, Object[] exceptionAttributes, TruffleString format, Object[] formatArgs) {
127128
super(cls, instanceShape);
128-
assert !(cls instanceof PythonNativeClass);
129+
assert !TypeNodes.NeedsNativeAllocationNode.executeUncached(cls);
129130
this.exceptionAttributes = exceptionAttributes;
130131
this.args = null;
131132
this.hasMessageFormat = true;

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

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,12 +2599,11 @@ private static long getBuiltinTypeItemsize(PythonBuiltinClassType cls) {
25992599
case PBytes -> 1;
26002600
case PInt -> 4;
26012601
case PFrame, PMemoryView, PTuple, PStatResult, PTerminalSize, PUnameResult, PStructTime, PProfilerEntry,
2602-
PProfilerSubentry, PStructPasswd, PStructRusage, PVersionInfo, PFlags, PFloatInfo,
2603-
PIntInfo, PHashInfo, PThreadInfo, PUnraisableHookArgs, PIOBase, PFileIO, PBufferedIOBase,
2604-
PBufferedReader, PBufferedWriter, PBufferedRWPair, PBufferedRandom, PIncrementalNewlineDecoder,
2605-
PTextIOWrapper, CArgObject, CThunkObject, StgDict, Structure, Union, PyCPointer, PyCArray,
2606-
PyCData, SimpleCData, PyCFuncPtr, CField, DictRemover, StructParam ->
2607-
8;
2602+
PProfilerSubentry, PStructPasswd, PStructRusage, PVersionInfo, PFlags, PFloatInfo,
2603+
PIntInfo, PHashInfo, PThreadInfo, PUnraisableHookArgs, PIOBase, PFileIO, PBufferedIOBase,
2604+
PBufferedReader, PBufferedWriter, PBufferedRWPair, PBufferedRandom, PIncrementalNewlineDecoder,
2605+
PTextIOWrapper, CArgObject, CThunkObject, StgDict, Structure, Union, PyCPointer, PyCArray,
2606+
PyCData, SimpleCData, PyCFuncPtr, CField, DictRemover, StructParam -> 8;
26082607
case PythonClass -> 40;
26092608
default -> 0;
26102609
};
@@ -2673,4 +2672,45 @@ void set(PythonManagedClass cls, long value,
26732672
write.execute(cls, TYPE_DICTOFFSET, value);
26742673
}
26752674
}
2675+
2676+
/**
2677+
* Tests if the given {@code cls} is a Python class that needs a native allocation. This is the
2678+
* case if {@code cls} either is a native class or it is a managed class that (indirectly)
2679+
* inherits from a native class.
2680+
*/
2681+
@GenerateUncached
2682+
@GenerateInline
2683+
@GenerateCached(false)
2684+
public abstract static class NeedsNativeAllocationNode extends Node {
2685+
public abstract boolean execute(Node inliningTarget, Object cls);
2686+
2687+
public static boolean executeUncached(Object cls) {
2688+
return TypeNodesFactory.NeedsNativeAllocationNodeGen.getUncached().execute(null, cls);
2689+
}
2690+
2691+
@Specialization
2692+
static boolean doPBCT(@SuppressWarnings("unused") PythonBuiltinClassType cls) {
2693+
return false;
2694+
}
2695+
2696+
@Specialization
2697+
static boolean doBuiltin(@SuppressWarnings("unused") PythonBuiltinClass cls) {
2698+
return false;
2699+
}
2700+
2701+
@Specialization
2702+
static boolean doManaged(PythonManagedClass cls) {
2703+
return cls.needsNativeAllocation();
2704+
}
2705+
2706+
@Specialization
2707+
static boolean doNative(@SuppressWarnings("unused") PythonNativeClass cls) {
2708+
return true;
2709+
}
2710+
2711+
@Fallback
2712+
static boolean doOther(@SuppressWarnings("unused") Object cls) {
2713+
return false;
2714+
}
2715+
}
26762716
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PGuards.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -368,18 +368,6 @@ public static boolean isNativeClass(Object klass) {
368368
return PythonNativeClass.isInstance(klass);
369369
}
370370

371-
/**
372-
* Tests if the given {@code klass} is a Python class that needs a native allocation. This is
373-
* the case if {@code klass} either is a native class or it is a managed class that (indirectly)
374-
* inherits from a native class.
375-
*/
376-
public static boolean needsNativeAllocation(Object klass) {
377-
if (klass instanceof PythonManagedClass managedClass) {
378-
return managedClass.needsNativeAllocation();
379-
}
380-
return PythonNativeClass.isInstance(klass);
381-
}
382-
383371
public static boolean isPythonClass(Object klass) {
384372
return PythonAbstractClass.isInstance(klass) || klass instanceof PythonBuiltinClassType;
385373
}

0 commit comments

Comments
 (0)