Skip to content

Commit cd15375

Browse files
steve-ssteve-s
committed
[GR-34386] Restrict some slots to only runtime values, so that their lookup can be used in any situation.
PullRequest: graalpython/1991
2 parents b030df9 + 791a356 commit cd15375

File tree

6 files changed

+86
-32
lines changed

6 files changed

+86
-32
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2142,9 +2142,9 @@ Object typeNew(VirtualFrame frame, Object cls, Object wName, PTuple bases, PDict
21422142
@Cached GetClassNode getClassNode,
21432143
@CachedLibrary(limit = "3") HashingStorageLibrary hashingStoragelib,
21442144
@Cached BranchProfile updatedStorage,
2145-
@Cached("create(__NEW__)") LookupInheritedAttributeNode getNewFuncNode,
2145+
@Cached("create(New)") LookupInheritedSlotNode getNewFuncNode,
21462146
@Cached("create(__INIT_SUBCLASS__)") GetAttributeNode getInitSubclassNode,
2147-
@Cached("create(__SET_NAME__)") LookupInheritedAttributeNode getSetNameNode,
2147+
@Cached("create(SetName)") LookupInheritedSlotNode getSetNameNode,
21482148
@Cached("create(__MRO_ENTRIES__)") LookupInheritedAttributeNode lookupMroEntriesNode,
21492149
@Cached CastToJavaStringNode castStr,
21502150
@Cached CallNode callSetNameNode,

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

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.type;
4242

43+
import static com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot.Flags.NO_BUILTIN_DESCRIPTORS;
4344
import static com.oracle.graal.python.nodes.SpecialAttributeNames.__CLASS__;
4445
import static com.oracle.graal.python.nodes.SpecialAttributeNames.__DICT__;
4546
import static com.oracle.graal.python.nodes.SpecialMethodNames.__ADD__;
@@ -76,7 +77,6 @@
7677
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEW__;
7778
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEXT__;
7879
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NE__;
79-
import static com.oracle.graal.python.nodes.SpecialMethodNames.__PREPARE__;
8080
import static com.oracle.graal.python.nodes.SpecialMethodNames.__RAND__;
8181
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REPR__;
8282
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REVERSED__;
@@ -159,13 +159,12 @@ public enum SpecialMethodSlot {
159159
Iter(__ITER__),
160160
Next(__NEXT__),
161161

162-
New(__NEW__),
163-
Init(__INIT__),
164-
Prepare(__PREPARE__),
165-
SetName(__SET_NAME__),
162+
New(__NEW__, NO_BUILTIN_DESCRIPTORS),
163+
Init(__INIT__, NO_BUILTIN_DESCRIPTORS),
164+
SetName(__SET_NAME__, NO_BUILTIN_DESCRIPTORS),
166165
InstanceCheck(__INSTANCECHECK__),
167166
Subclasscheck(__SUBCLASSCHECK__),
168-
Call(__CALL__),
167+
Call(__CALL__, NO_BUILTIN_DESCRIPTORS),
169168

170169
GetItem(__GETITEM__),
171170
SetItem(__SETITEM__),
@@ -202,11 +201,38 @@ public enum SpecialMethodSlot {
202201
Reversed(__REVERSED__),
203202
Bytes(__BYTES__);
204203

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

208228
SpecialMethodSlot(String name) {
209229
this.name = name;
230+
this.allowsBuiltinDescriptors = true;
231+
}
232+
233+
SpecialMethodSlot(String name, boolean allowsBuiltinDescriptors) {
234+
this.name = name;
235+
this.allowsBuiltinDescriptors = allowsBuiltinDescriptors;
210236
}
211237

212238
public String getName() {
@@ -227,7 +253,7 @@ private void setValue(PythonManagedClass klass, Object value, PythonContext cont
227253
// For builtin classes, we should see these updates only during initialization
228254
assert !context.isInitialized() || !(klass instanceof PythonBuiltinClass) ||
229255
((PythonBuiltinClass) klass).getType().getSpecialMethodSlots() == null : String.format("%s.%s = %s", klass, getName(), value);
230-
klass.specialMethodSlots[ordinal()] = asSlotValue(value, context.getLanguage());
256+
klass.specialMethodSlots[ordinal()] = asSlotValue(this, value, context.getLanguage());
231257
if (klass instanceof PythonClass) {
232258
((PythonClass) klass).invalidateSlotsFinalAssumption();
233259
}
@@ -305,12 +331,12 @@ private static void initializeBuiltinTypeSlotsImpl(Python3Core core) {
305331
continue;
306332
}
307333
Object value = slot.getValue(klass);
308-
if (value instanceof PBuiltinFunction) {
334+
if (value instanceof PBuiltinFunction && slot.allowsBuiltinDescriptors) {
309335
BuiltinMethodDescriptor info = BuiltinMethodDescriptor.get((PBuiltinFunction) value);
310336
if (info != null) {
311337
typeSlots[slot.ordinal()] = info;
312338
}
313-
} else if (value instanceof BuiltinMethodDescriptor || PythonLanguage.canCache(value)) {
339+
} else if ((value instanceof BuiltinMethodDescriptor && slot.allowsBuiltinDescriptors) || PythonLanguage.canCache(value)) {
314340
typeSlots[slot.ordinal()] = value;
315341
}
316342
}
@@ -447,7 +473,7 @@ private static void setSlotsFromManaged(Object[] slots, PythonManagedClass sourc
447473
for (SpecialMethodSlot slot : VALUES) {
448474
final Object value = domLib.getOrDefault(source, slot.getName(), PNone.NO_VALUE);
449475
if (value != PNone.NO_VALUE) {
450-
slots[slot.ordinal()] = asSlotValue(value, language);
476+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
451477
}
452478
}
453479
} else {
@@ -456,7 +482,7 @@ private static void setSlotsFromManaged(Object[] slots, PythonManagedClass sourc
456482
for (SpecialMethodSlot slot : VALUES) {
457483
final Object value = hlib.getItem(storage, slot.getName());
458484
if (value != null) {
459-
slots[slot.ordinal()] = asSlotValue(value, language);
485+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
460486
}
461487
}
462488
}
@@ -467,7 +493,7 @@ private static void setSlotsFromGeneric(Object[] slots, PythonAbstractClass base
467493
for (SpecialMethodSlot slot : VALUES) {
468494
Object value = readAttNode.execute(base, slot.getName());
469495
if (value != PNone.NO_VALUE) {
470-
slots[slot.ordinal()] = asSlotValue(value, language);
496+
slots[slot.ordinal()] = asSlotValue(slot, value, language);
471497
}
472498
}
473499
}
@@ -520,7 +546,7 @@ private static void fixupSpecialMethodSlotInternal(PythonManagedClass klass, Spe
520546
// If the newly written value is not NO_VALUE, then should either override the slot with
521547
// the new value or leave it unchanged if it inherited the value from some other class
522548
assert currentNewValue != PNone.NO_VALUE;
523-
assert asSlotValue(currentNewValue, context.getLanguage()) == currentOldValue || currentNewValue == newValue;
549+
assert asSlotValue(slot, currentNewValue, context.getLanguage()) == currentOldValue || currentNewValue == newValue;
524550
}
525551
// Else if the newly written value was NO_VALUE, then we either remove the slot or we pull
526552
// its value from some other class in the MRO
@@ -547,8 +573,8 @@ private static void fixupSpecialMethodInSubClasses(java.util.Set<PythonAbstractC
547573
}
548574
}
549575

550-
private static Object asSlotValue(Object value, PythonLanguage language) {
551-
if (value instanceof PBuiltinFunction) {
576+
private static Object asSlotValue(SpecialMethodSlot slot, Object value, PythonLanguage language) {
577+
if (value instanceof PBuiltinFunction && slot.allowsBuiltinDescriptors) {
552578
BuiltinMethodDescriptor info = BuiltinMethodDescriptor.get((PBuiltinFunction) value);
553579
if (info != null) {
554580
language.registerBuiltinDescriptorCallTarget(info, ((PBuiltinFunction) value).getCallTarget());
@@ -639,8 +665,6 @@ public static SpecialMethodSlot findSpecialSlot(String name) {
639665
return Delete;
640666
case __DICT__:
641667
return Dict;
642-
case __PREPARE__:
643-
return Prepare;
644668
case __GT__:
645669
return Gt;
646670
case __GE__:

‎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
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/CallNode.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.oracle.graal.python.nodes.argument.CreateArgumentsNode;
5656
import com.oracle.graal.python.nodes.argument.positional.PositionalArgumentsNode;
5757
import com.oracle.graal.python.nodes.attributes.LookupInheritedAttributeNode;
58+
import com.oracle.graal.python.nodes.attributes.LookupInheritedSlotNode;
5859
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
5960
import com.oracle.graal.python.nodes.truffle.PythonTypes;
6061
import com.oracle.truffle.api.dsl.Cached;
@@ -124,7 +125,7 @@ protected Object builtinFunctionCall(VirtualFrame frame, PBuiltinFunction callab
124125
@Specialization
125126
protected Object doType(VirtualFrame frame, PythonBuiltinClassType callableObject, Object[] arguments, PKeyword[] keywords,
126127
@Shared("raise") @Cached PRaiseNode raise,
127-
@Shared("lookupCall") @Cached("create(__CALL__)") LookupInheritedAttributeNode callAttrGetterNode,
128+
@Shared("lookupCall") @Cached("create(Call)") LookupInheritedSlotNode callAttrGetterNode,
128129
@Shared("callCall") @Cached CallVarargsMethodNode callCallNode) {
129130
Object call = callAttrGetterNode.execute(callableObject);
130131
return callCall(frame, callableObject, arguments, keywords, raise, callCallNode, call);
@@ -133,7 +134,7 @@ protected Object doType(VirtualFrame frame, PythonBuiltinClassType callableObjec
133134
@Specialization(guards = "isPythonClass(callableObject)", replaces = "doType")
134135
protected Object doPythonClass(VirtualFrame frame, Object callableObject, Object[] arguments, PKeyword[] keywords,
135136
@Shared("raise") @Cached PRaiseNode raise,
136-
@Shared("lookupCall") @Cached("create(__CALL__)") LookupInheritedAttributeNode callAttrGetterNode,
137+
@Shared("lookupCall") @Cached("create(Call)") LookupInheritedSlotNode callAttrGetterNode,
137138
@Shared("callCall") @Cached CallVarargsMethodNode callCallNode) {
138139
Object call = callAttrGetterNode.execute(callableObject);
139140
return callCall(frame, callableObject, arguments, keywords, raise, callCallNode, call);
@@ -142,7 +143,7 @@ protected Object doPythonClass(VirtualFrame frame, Object callableObject, Object
142143
@Specialization(guards = "!isCallable(callableObject)", replaces = {"doType", "doPythonClass"})
143144
protected Object doObjectAndType(VirtualFrame frame, Object callableObject, Object[] arguments, PKeyword[] keywords,
144145
@Shared("raise") @Cached PRaiseNode raise,
145-
@Shared("lookupCall") @Cached("create(__CALL__)") LookupInheritedAttributeNode callAttrGetterNode,
146+
@Shared("lookupCall") @Cached("create(Call)") LookupInheritedSlotNode callAttrGetterNode,
146147
@Shared("callCall") @Cached CallVarargsMethodNode callCallNode) {
147148
Object call = callAttrGetterNode.execute(callableObject);
148149
return callCall(frame, callableObject, arguments, keywords, raise, callCallNode, call);
@@ -191,15 +192,15 @@ protected Object builtinMethodCallBuiltinDirect(VirtualFrame frame, PBuiltinMeth
191192
@Specialization(guards = "!isFunction(callable.getFunction())")
192193
protected Object methodCall(VirtualFrame frame, PMethod callable, Object[] arguments, PKeyword[] keywords,
193194
@Shared("raise") @Cached PRaiseNode raise,
194-
@Shared("lookupCall") @Cached("create(__CALL__)") LookupInheritedAttributeNode callAttrGetterNode,
195+
@Shared("lookupCall") @Cached("create(Call)") LookupInheritedSlotNode callAttrGetterNode,
195196
@Shared("callCall") @Cached CallVarargsMethodNode callCallNode) {
196197
return doObjectAndType(frame, callable, arguments, keywords, raise, callAttrGetterNode, callCallNode);
197198
}
198199

199200
@Specialization(guards = "!isFunction(callable.getFunction())")
200201
protected Object builtinMethodCall(VirtualFrame frame, PBuiltinMethod callable, Object[] arguments, PKeyword[] keywords,
201202
@Shared("raise") @Cached PRaiseNode raise,
202-
@Shared("lookupCall") @Cached("create(__CALL__)") LookupInheritedAttributeNode callAttrGetterNode,
203+
@Shared("lookupCall") @Cached("create(Call)") LookupInheritedSlotNode callAttrGetterNode,
203204
@Shared("callVarargs") @Cached CallVarargsMethodNode callCallNode) {
204205
return doObjectAndType(frame, callable, arguments, keywords, raise, callAttrGetterNode, callCallNode);
205206
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/special/LookupAndCallBinaryNode.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,11 @@
4747
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4848
import com.oracle.graal.python.builtins.objects.PNone;
4949
import com.oracle.graal.python.builtins.objects.PNotImplemented;
50+
import com.oracle.graal.python.builtins.objects.function.BuiltinMethodDescriptor.BinaryBuiltinDescriptor;
5051
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
5152
import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
5253
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
54+
import com.oracle.graal.python.builtins.objects.type.SpecialMethodSlot;
5355
import com.oracle.graal.python.builtins.objects.type.TypeNodes;
5456
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
5557
import com.oracle.graal.python.nodes.ErrorMessages;
@@ -174,6 +176,15 @@ private CallBinaryMethodNode ensureReverseDispatch() {
174176
}
175177

176178
protected final PythonBinaryBuiltinNode getBinaryBuiltin(PythonBuiltinClassType clazz) {
179+
if (SpecialMethodSlot.canBeSpecial(name)) {
180+
SpecialMethodSlot slot = SpecialMethodSlot.findSpecialSlot(name);
181+
if (slot != null) {
182+
Object attribute = slot.getValue(clazz);
183+
if (attribute instanceof BinaryBuiltinDescriptor) {
184+
return ((BinaryBuiltinDescriptor) attribute).createNode();
185+
}
186+
}
187+
}
177188
Object attribute = LookupAttributeInMRONode.Dynamic.getUncached().execute(clazz, name);
178189
if (attribute instanceof PBuiltinFunction) {
179190
PBuiltinFunction builtinFunction = (PBuiltinFunction) attribute;

0 commit comments

Comments
 (0)