Skip to content

Commit 325cdd7

Browse files
msimacekmsimacek
committed
Combine __getattribute__ and __getattr__ in tp_getattro wrapper
1 parent 236ad64 commit 325cdd7

File tree

7 files changed

+179
-9
lines changed

7 files changed

+179
-9
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,15 @@ def richcompare_bool(args):
375375
argspec="OOi",
376376
resultspec="i",
377377
)
378-
378+
379+
class TypeWithGetattr:
380+
def __getattr__(self, item):
381+
return item
382+
379383
__PyObject_GetAttrString_ARGS = (
380384
(MyObject(), "foo"),
381385
([], "__len__"),
386+
(TypeWithGetattr(), "foo"),
382387
)
383388
test_PyObject_GetAttrString = CPyExtFunction(
384389
lambda args: getattr(*args),
@@ -442,6 +447,7 @@ def _ref_hash_not_implemented(args):
442447
__PyObject_GetAttr_ARGS = (
443448
(MyObject(), "foo"),
444449
([], "__len__"),
450+
(TypeWithGetattr(), "foo"),
445451
)
446452
test_PyObject_GetAttr = CPyExtFunction(
447453
lambda args: getattr(*args),
@@ -480,4 +486,4 @@ def _ref_hash_not_implemented(args):
480486
arguments=["PyObject* func"],
481487
argspec="O",
482488
cmpfunc=lambda x, y: type(x) == staticmethod
483-
)
489+
)

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,64 @@ def test_getattro(self):
334334
tester = TestInt()
335335
assert tester.foo == "foo"
336336

337+
def test_getattro_inheritance(self):
338+
TestWithGetattro = CPyExtType(
339+
'TestWithGetattro',
340+
'''
341+
static PyObject* getattro(PyObject* self, PyObject* key) {
342+
if (PyObject_IsTrue(key)) {
343+
Py_INCREF(key);
344+
return key;
345+
}
346+
return PyErr_Format(PyExc_AttributeError, "Nope");
347+
}
348+
349+
static PyObject* call_getattro_slot(PyObject* unused, PyObject* args) {
350+
PyObject* object;
351+
PyObject* key;
352+
if (!PyArg_ParseTuple(args, "OO", &object, &key))
353+
return NULL;
354+
return Py_TYPE(object)->tp_getattro(object, key);
355+
}
356+
''',
357+
tp_getattro='getattro',
358+
tp_methods='{"call_getattro_slot", (PyCFunction)call_getattro_slot, METH_VARARGS | METH_STATIC, ""}'
359+
)
360+
361+
def validate(cls, has_getattro, has_getattr):
362+
foo = cls()
363+
if has_getattro:
364+
assert foo.asdf == 'asdf'
365+
assert TestWithGetattro.call_getattro_slot(foo, 'asdf') == 'asdf'
366+
elif has_getattr:
367+
assert foo.asdf == 3
368+
assert TestWithGetattro.call_getattro_slot(foo, 'asdf') == 3
369+
if has_getattro and not has_getattr:
370+
assert_raises(AttributeError, lambda: getattr(foo, ''))
371+
assert_raises(AttributeError, TestWithGetattro.call_getattro_slot, foo, '')
372+
if has_getattr:
373+
assert getattr(foo, '') == 3
374+
assert TestWithGetattro.call_getattro_slot(foo, '') == 3
375+
376+
validate(TestWithGetattro, True, False)
377+
378+
class Subclass(TestWithGetattro):
379+
pass
380+
381+
validate(Subclass, True, False)
382+
383+
class ObjWithGetattr:
384+
def __getattr__(self, item):
385+
return 3
386+
387+
validate(ObjWithGetattr, False, True)
388+
389+
class SubclassWithGetattr(TestWithGetattro):
390+
def __getattr__(self, item):
391+
return 3
392+
393+
validate(SubclassWithGetattr, True, True)
394+
337395
def test_dict(self):
338396
TestDict = CPyExtType("TestDict",
339397
"""static PyObject* custom_dict = NULL;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextSlotBuiltins.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,8 +1416,8 @@ static Object get(PythonManagedClass object,
14161416
public abstract static class Py_get_PyTypeObject_tp_getattro extends CApiUnaryBuiltinNode {
14171417

14181418
@Specialization
1419-
Object get(Object object) {
1420-
return LookupNativeSlotNode.executeUncached(object, SlotMethodDef.TP_GETATTRO);
1419+
Object get(PythonManagedClass object) {
1420+
return LookupNativeSlotNode.LookupNativeGetattroSlotNode.executeUncached(object);
14211421
}
14221422
}
14231423

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
import com.oracle.graal.python.runtime.PythonOptions;
104104
import com.oracle.graal.python.runtime.exception.PException;
105105
import com.oracle.graal.python.util.PythonUtils;
106+
import com.oracle.graal.python.util.Supplier;
106107
import com.oracle.graal.python.util.WeakIdentityHashMap;
107108
import com.oracle.truffle.api.CallTarget;
108109
import com.oracle.truffle.api.CompilerAsserts;
@@ -1116,12 +1117,12 @@ public long registerClosure(String nfiSignature, Object executable, Object deleg
11161117
private final WeakIdentityHashMap<PythonManagedClass, PyProcsWrapper[]> procWrappers = new WeakIdentityHashMap<>();
11171118

11181119
@TruffleBoundary
1119-
public Object getOrCreateProcWrapper(PythonManagedClass owner, SlotMethodDef slot, Object callable) {
1120+
public Object getOrCreateProcWrapper(PythonManagedClass owner, SlotMethodDef slot, Supplier<PyProcsWrapper> supplier) {
11201121
PyProcsWrapper[] slotWrappers = procWrappers.computeIfAbsent(owner, key -> new PyProcsWrapper[SlotMethodDef.values().length]);
11211122
int idx = slot.ordinal();
11221123
PyProcsWrapper wrapper = slotWrappers[idx];
11231124
if (wrapper == null) {
1124-
wrapper = slot.wrapperFactory.apply(callable);
1125+
wrapper = supplier.get();
11251126
slotWrappers[idx] = wrapper;
11261127
}
11271128
return wrapper;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ static Object doTpSetattr(@SuppressWarnings("unused") PythonManagedClass object,
634634

635635
@Specialization(guards = "eq(TP_GETATTRO, key)")
636636
static Object doTpGetattro(PythonManagedClass object, @SuppressWarnings("unused") PythonNativeWrapper nativeWrapper, @SuppressWarnings("unused") String key,
637-
@Cached(parameters = "TP_GETATTRO") LookupNativeSlotNode lookup) {
637+
@Cached LookupNativeSlotNode.LookupNativeGetattroSlotNode lookup) {
638638
return lookup.execute(object);
639639
}
640640

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,15 @@
5959
import com.oracle.graal.python.nodes.call.special.CallTernaryMethodNode;
6060
import com.oracle.graal.python.nodes.call.special.CallUnaryMethodNode;
6161
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
62+
import com.oracle.graal.python.nodes.object.BuiltinClassProfiles;
6263
import com.oracle.graal.python.nodes.util.CastToJavaIntLossyNode;
6364
import com.oracle.graal.python.runtime.GilNode;
6465
import com.oracle.graal.python.runtime.PythonContext;
6566
import com.oracle.graal.python.runtime.exception.PException;
6667
import com.oracle.graal.python.util.PythonUtils;
6768
import com.oracle.truffle.api.CompilerDirectives;
6869
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
70+
import com.oracle.truffle.api.dsl.Bind;
6971
import com.oracle.truffle.api.dsl.Cached;
7072
import com.oracle.truffle.api.dsl.Cached.Exclusive;
7173
import com.oracle.truffle.api.dsl.Specialization;
@@ -75,6 +77,7 @@
7577
import com.oracle.truffle.api.interop.UnsupportedTypeException;
7678
import com.oracle.truffle.api.library.ExportLibrary;
7779
import com.oracle.truffle.api.library.ExportMessage;
80+
import com.oracle.truffle.api.nodes.Node;
7881
import com.oracle.truffle.api.profiles.ConditionProfile;
7982
import com.oracle.truffle.llvm.spi.NativeTypeLibrary;
8083

@@ -175,6 +178,59 @@ protected String getSignature() {
175178
}
176179
}
177180

181+
@ExportLibrary(InteropLibrary.class)
182+
public static final class GetAttrCombinedWrapper extends PyProcsWrapper {
183+
184+
private final Object getattr;
185+
186+
public GetAttrCombinedWrapper(Object getattro, Object getattr) {
187+
super(getattro);
188+
this.getattr = getattr;
189+
}
190+
191+
@ExportMessage
192+
protected Object execute(Object[] arguments,
193+
@Bind("$node") Node inliningTarget,
194+
@Cached PythonToNativeNewRefNode toNativeNode,
195+
@Cached CallBinaryMethodNode executeNode,
196+
@Cached NativeToPythonNode toJavaNode,
197+
@Cached BuiltinClassProfiles.IsBuiltinObjectProfile errorProfile,
198+
@Cached TransformExceptionToNativeNode transformExceptionToNativeNode,
199+
@Exclusive @Cached GilNode gil) throws ArityException {
200+
boolean mustRelease = gil.acquire();
201+
CApiTiming.enter();
202+
try {
203+
if (arguments.length != 2) {
204+
CompilerDirectives.transferToInterpreterAndInvalidate();
205+
throw ArityException.create(2, 2, arguments.length);
206+
}
207+
try {
208+
Object object = toJavaNode.execute(arguments[0]);
209+
Object key = toJavaNode.execute(arguments[1]);
210+
try {
211+
return toNativeNode.execute(executeNode.executeObject(null, getDelegate(), object, key));
212+
} catch (PException e) {
213+
e.expectAttributeError(inliningTarget, errorProfile);
214+
}
215+
return toNativeNode.execute(executeNode.executeObject(null, getattr, object, key));
216+
} catch (Throwable t) {
217+
throw checkThrowableBeforeNative(t, "GetAttrWrapper", getDelegate());
218+
}
219+
} catch (PException e) {
220+
transformExceptionToNativeNode.execute(null, e);
221+
return PythonContext.get(gil).getNativeNull().getPtr();
222+
} finally {
223+
CApiTiming.exit(timing);
224+
gil.release(mustRelease);
225+
}
226+
}
227+
228+
@Override
229+
protected String getSignature() {
230+
return "(POINTER,POINTER):POINTER";
231+
}
232+
}
233+
178234
@ExportLibrary(InteropLibrary.class)
179235
public static final class BinaryFuncWrapper extends PyProcsWrapper {
180236

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupNativeSlotNode.java

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
5252
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ToSulongNode;
5353
import com.oracle.graal.python.builtins.objects.cext.capi.NativeMember;
54+
import com.oracle.graal.python.builtins.objects.cext.capi.PyProcsWrapper;
5455
import com.oracle.graal.python.builtins.objects.cext.capi.SlotMethodDef;
5556
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageGuards;
5657
import com.oracle.graal.python.builtins.objects.dict.PDict;
@@ -59,6 +60,7 @@
5960
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
6061
import com.oracle.graal.python.builtins.objects.type.PythonClass;
6162
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
63+
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
6264
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetMroStorageNode;
6365
import com.oracle.graal.python.builtins.objects.type.TypeNodes.InlinedIsSameTypeNode;
6466
import com.oracle.graal.python.nodes.PNodeWithContext;
@@ -71,7 +73,10 @@
7173
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
7274
import com.oracle.truffle.api.dsl.Bind;
7375
import com.oracle.truffle.api.dsl.Cached;
76+
import com.oracle.truffle.api.dsl.Cached.Shared;
77+
import com.oracle.truffle.api.dsl.Fallback;
7478
import com.oracle.truffle.api.dsl.GenerateCached;
79+
import com.oracle.truffle.api.dsl.GenerateUncached;
7580
import com.oracle.truffle.api.dsl.ImportStatic;
7681
import com.oracle.truffle.api.dsl.ReportPolymorphism.Megamorphic;
7782
import com.oracle.truffle.api.dsl.Specialization;
@@ -302,14 +307,18 @@ private Object wrapManagedMethod(PythonBuiltinClassType owner, Object value) {
302307
return getNULL();
303308
}
304309
PythonContext context = getContext();
305-
return context.getCApiContext().getOrCreateProcWrapper(context.lookupType(owner), slot, value);
310+
return wrapCallable(context, context.lookupType(owner), value);
306311
}
307312

308313
private Object wrapManagedMethod(PythonManagedClass owner, Object value) {
309314
if (value instanceof PNone) {
310315
return getNULL();
311316
}
312-
return getContext().getCApiContext().getOrCreateProcWrapper(owner, slot, value);
317+
return wrapCallable(getContext(), owner, value);
318+
}
319+
320+
private Object wrapCallable(PythonContext context, PythonManagedClass owner, Object value) {
321+
return context.getCApiContext().getOrCreateProcWrapper(owner, slot, () -> slot.wrapperFactory.apply(value));
313322
}
314323

315324
@GenerateCached(false)
@@ -333,4 +342,44 @@ public Object execute(Object type) {
333342
InteropLibrary.getUncached());
334343
}
335344
}
345+
346+
@GenerateUncached
347+
@ImportStatic({SpecialMethodSlot.class, SlotMethodDef.class})
348+
public abstract static class LookupNativeGetattroSlotNode extends Node {
349+
public abstract Object execute(Object type);
350+
351+
public static Object executeUncached(Object type) {
352+
return LookupNativeSlotNodeGen.LookupNativeGetattroSlotNodeGen.getUncached().execute(type);
353+
}
354+
355+
@Specialization
356+
Object get(PythonManagedClass type,
357+
@Shared @Cached(parameters = "GetAttr") LookupCallableSlotInMRONode lookupGetattr,
358+
@Shared @Cached(parameters = "GetAttribute") LookupCallableSlotInMRONode lookupGetattribute,
359+
@Shared @Cached(parameters = "TP_GETATTRO") LookupNativeSlotNode lookupNativeGetattro) {
360+
Object getattr = lookupGetattr.execute(type);
361+
if (getattr == PNone.NO_VALUE) {
362+
return lookupNativeGetattro.execute(type);
363+
} else {
364+
Object getattribute = lookupGetattribute.execute(type);
365+
return PythonContext.get(this).getCApiContext().getOrCreateProcWrapper(type, SlotMethodDef.TP_GETATTRO, () -> new PyProcsWrapper.GetAttrCombinedWrapper(getattribute, getattr));
366+
}
367+
}
368+
369+
@Specialization
370+
Object get(PythonBuiltinClassType type,
371+
@Shared @Cached(parameters = "GetAttr") LookupCallableSlotInMRONode lookupGetattr,
372+
@Shared @Cached(parameters = "GetAttribute") LookupCallableSlotInMRONode lookupGetattribute,
373+
@Shared @Cached(parameters = "TP_GETATTRO") LookupNativeSlotNode lookupNativeGetattro) {
374+
return get(PythonContext.get(this).lookupType(type), lookupGetattr, lookupGetattribute, lookupNativeGetattro);
375+
}
376+
377+
@Fallback
378+
Object get(Object type,
379+
@Shared @Cached(parameters = "TP_GETATTRO") LookupNativeSlotNode lookupNativeGetattro) {
380+
// I don't think we ever get here, but if we do, for a native class we just return its
381+
// slot
382+
return lookupNativeGetattro.execute(type);
383+
}
384+
}
336385
}

0 commit comments

Comments
 (0)