Skip to content

Commit d76d971

Browse files
steve-ssteve-s
committed
Do not put builtin descriptors to slots for __new__ and __init__ so that slot lookup can be used in type's __call__
1 parent ed778a2 commit d76d971

File tree

2 files changed

+45
-17
lines changed

2 files changed

+45
-17
lines changed

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

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ public enum SpecialMethodSlot {
159159
Iter(__ITER__),
160160
Next(__NEXT__),
161161

162-
New(__NEW__),
163-
Init(__INIT__),
162+
New(__NEW__, false),
163+
Init(__INIT__, false),
164164
Prepare(__PREPARE__),
165165
SetName(__SET_NAME__),
166166
InstanceCheck(__INSTANCECHECK__),
@@ -204,9 +204,32 @@ public enum SpecialMethodSlot {
204204

205205
public static final SpecialMethodSlot[] VALUES = values();
206206
private final String name;
207+
/**
208+
* Indicates if given slot may or must not store context independent (AST cacheable)
209+
* {@link BuiltinMethodDescriptor} objects.
210+
*
211+
* Values of some slots are always or mostly passed to call node variants that can handle
212+
* {@link BuiltinMethodDescriptor}. This does not hold most notably for slots that are passed to
213+
* {@link com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode}, like
214+
* {@code __new__}. For those we do no allow storing the {@link BuiltinMethodDescriptor} in the
215+
* slot, so that lookup using that slot always resolves to context dependent runtime object,
216+
* such as {@link PBuiltinFunction}.
217+
*
218+
* An alternative would be to update the whole calling machinery ({@code InvokeNode},
219+
* {@code GetSignature}, ...) to handle {@link BuiltinMethodDescriptor} and extend
220+
* {@link BuiltinMethodDescriptor} to contain all the information that is necessary for this
221+
* (GR-32148).
222+
*/
223+
private final boolean allowsBuiltinDescriptors;
207224

208225
SpecialMethodSlot(String name) {
209226
this.name = name;
227+
this.allowsBuiltinDescriptors = true;
228+
}
229+
230+
SpecialMethodSlot(String name, boolean allowsBuiltinDescriptors) {
231+
this.name = name;
232+
this.allowsBuiltinDescriptors = allowsBuiltinDescriptors;
210233
}
211234

212235
public String getName() {
@@ -227,7 +250,7 @@ private void setValue(PythonManagedClass klass, Object value, PythonContext cont
227250
// For builtin classes, we should see these updates only during initialization
228251
assert !context.isInitialized() || !(klass instanceof PythonBuiltinClass) ||
229252
((PythonBuiltinClass) klass).getType().getSpecialMethodSlots() == null : String.format("%s.%s = %s", klass, getName(), value);
230-
klass.specialMethodSlots[ordinal()] = asSlotValue(value, context.getLanguage());
253+
klass.specialMethodSlots[ordinal()] = asSlotValue(this, value, context.getLanguage());
231254
if (klass instanceof PythonClass) {
232255
((PythonClass) klass).invalidateSlotsFinalAssumption();
233256
}
@@ -305,12 +328,12 @@ private static void initializeBuiltinTypeSlotsImpl(Python3Core core) {
305328
continue;
306329
}
307330
Object value = slot.getValue(klass);
308-
if (value instanceof PBuiltinFunction) {
331+
if (value instanceof PBuiltinFunction && slot.allowsBuiltinDescriptors) {
309332
BuiltinMethodDescriptor info = BuiltinMethodDescriptor.get((PBuiltinFunction) value);
310333
if (info != null) {
311334
typeSlots[slot.ordinal()] = info;
312335
}
313-
} else if (value instanceof BuiltinMethodDescriptor || PythonLanguage.canCache(value)) {
336+
} else if ((value instanceof BuiltinMethodDescriptor && slot.allowsBuiltinDescriptors) || PythonLanguage.canCache(value)) {
314337
typeSlots[slot.ordinal()] = value;
315338
}
316339
}
@@ -447,7 +470,7 @@ private static void setSlotsFromManaged(Object[] slots, PythonManagedClass sourc
447470
for (SpecialMethodSlot slot : VALUES) {
448471
final Object value = domLib.getOrDefault(source, slot.getName(), PNone.NO_VALUE);
449472
if (value != PNone.NO_VALUE) {
450-
slots[slot.ordinal()] = asSlotValue(value, language);
473+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
451474
}
452475
}
453476
} else {
@@ -456,7 +479,7 @@ private static void setSlotsFromManaged(Object[] slots, PythonManagedClass sourc
456479
for (SpecialMethodSlot slot : VALUES) {
457480
final Object value = hlib.getItem(storage, slot.getName());
458481
if (value != null) {
459-
slots[slot.ordinal()] = asSlotValue(value, language);
482+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
460483
}
461484
}
462485
}
@@ -467,7 +490,7 @@ private static void setSlotsFromGeneric(Object[] slots, PythonAbstractClass base
467490
for (SpecialMethodSlot slot : VALUES) {
468491
Object value = readAttNode.execute(base, slot.getName());
469492
if (value != PNone.NO_VALUE) {
470-
slots[slot.ordinal()] = asSlotValue(value, language);
493+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
471494
}
472495
}
473496
}
@@ -520,7 +543,7 @@ private static void fixupSpecialMethodSlotInternal(PythonManagedClass klass, Spe
520543
// If the newly written value is not NO_VALUE, then should either override the slot with
521544
// the new value or leave it unchanged if it inherited the value from some other class
522545
assert currentNewValue != PNone.NO_VALUE;
523-
assert asSlotValue(currentNewValue, context.getLanguage()) == currentOldValue || currentNewValue == newValue;
546+
assert asSlotValue(slot, currentNewValue, context.getLanguage()) == currentOldValue || currentNewValue == newValue;
524547
}
525548
// Else if the newly written value was NO_VALUE, then we either remove the slot or we pull
526549
// its value from some other class in the MRO
@@ -547,8 +570,8 @@ private static void fixupSpecialMethodInSubClasses(java.util.Set<PythonAbstractC
547570
}
548571
}
549572

550-
private static Object asSlotValue(Object value, PythonLanguage language) {
551-
if (value instanceof PBuiltinFunction) {
573+
private static Object asSlotValue(SpecialMethodSlot slot, Object value, PythonLanguage language) {
574+
if (value instanceof PBuiltinFunction && slot.allowsBuiltinDescriptors) {
552575
BuiltinMethodDescriptor info = BuiltinMethodDescriptor.get((PBuiltinFunction) value);
553576
if (info != null) {
554577
language.registerBuiltinDescriptorCallTarget(info, ((PBuiltinFunction) value).getCallTarget());

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import static com.oracle.graal.python.nodes.SpecialMethodNames.__GET__;
5050
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INIT__;
5151
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INSTANCECHECK__;
52-
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEW__;
5352
import static com.oracle.graal.python.nodes.SpecialMethodNames.__PREPARE__;
5453
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REPR__;
5554
import static com.oracle.graal.python.nodes.SpecialMethodNames.__SUBCLASSCHECK__;
@@ -78,6 +77,7 @@
7877
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
7978
import com.oracle.graal.python.builtins.objects.common.SequenceNodes.GetObjectArrayNode;
8079
import com.oracle.graal.python.builtins.objects.dict.PDict;
80+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor;
8181
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
8282
import com.oracle.graal.python.builtins.objects.function.PFunction;
8383
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -114,7 +114,7 @@
114114
import com.oracle.graal.python.nodes.call.special.CallTernaryMethodNode;
115115
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
116116
import com.oracle.graal.python.nodes.call.special.LookupAndCallBinaryNode;
117-
import com.oracle.graal.python.nodes.call.special.LookupSpecialMethodNode;
117+
import com.oracle.graal.python.nodes.call.special.LookupSpecialMethodSlotNode;
118118
import com.oracle.graal.python.nodes.classes.AbstractObjectGetBasesNode;
119119
import com.oracle.graal.python.nodes.classes.AbstractObjectIsSubclassNode;
120120
import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
@@ -372,6 +372,11 @@ static Object doBuiltin(PBuiltinFunction descriptor, @SuppressWarnings("unused")
372372
return descriptor;
373373
}
374374

375+
@Specialization
376+
static Object doBuiltinDescriptor(BuiltinMethodDescriptor descriptor, @SuppressWarnings("unused") Object type) {
377+
return descriptor;
378+
}
379+
375380
@Specialization
376381
static Object doFunction(PFunction descriptor, @SuppressWarnings("unused") Object type) {
377382
return descriptor;
@@ -397,10 +402,10 @@ public static BindNew create() {
397402
@ReportPolymorphism
398403
protected abstract static class CallNodeHelper extends PNodeWithRaise {
399404
@Child private CallVarargsMethodNode dispatchNew = CallVarargsMethodNode.create();
400-
@Child private LookupAttributeInMRONode lookupNew = LookupAttributeInMRONode.create(__NEW__);
405+
@Child private LookupCallableSlotInMRONode lookupNew = LookupCallableSlotInMRONode.create(SpecialMethodSlot.New);
401406
@Child private BindNew bindNew = BindNew.create();
402407
@Child private CallVarargsMethodNode dispatchInit;
403-
@Child private LookupSpecialMethodNode lookupInit;
408+
@Child private LookupSpecialMethodSlotNode lookupInit;
404409
@Child private IsSubtypeNode isSubTypeNode;
405410
@Child private TypeNodes.GetNameNode getNameNode;
406411
@Child private GetClassNode getClassNode;
@@ -514,10 +519,10 @@ private void callInit(Object newInstance, Object self, VirtualFrame frame, boole
514519
}
515520
}
516521

517-
private LookupSpecialMethodNode getInitNode() {
522+
private LookupSpecialMethodSlotNode getInitNode() {
518523
if (lookupInit == null) {
519524
CompilerDirectives.transferToInterpreterAndInvalidate();
520-
lookupInit = insert(LookupSpecialMethodNode.create(__INIT__));
525+
lookupInit = insert(LookupSpecialMethodSlotNode.create(SpecialMethodSlot.Init));
521526
}
522527
return lookupInit;
523528
}

0 commit comments

Comments
 (0)