Skip to content

Commit e436dda

Browse files
steve-ssteve-s
committed
Rename TpSlots fields to be consistent with CPython and add "combined" prefix to computed artificial slots
1 parent 554028f commit e436dda

File tree

12 files changed

+58
-58
lines changed

12 files changed

+58
-58
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/builtin/objects/TpSlotsTests.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ public void testBuilderBasic() {
6363
TpSlots slots = builder.build();
6464
verifySlots(slots, def -> true);
6565

66-
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.sq_mp_length());
67-
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.mp_sq_length());
68-
checkSlotValue(TpSlotMeta.TP_GET_ATTRO, slots.tp_get_attro_attr());
66+
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.combined_sq_mp_length());
67+
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.combined_mp_sq_length());
68+
checkSlotValue(TpSlotMeta.TP_GET_ATTRO, slots.combined_tp_getattro_getattr());
6969
}
7070

7171
@Test
@@ -77,9 +77,9 @@ public void testBuilderOptimizations1() {
7777
TpSlots slots = builder.build();
7878
verifySlots(slots, def -> def == TpSlotMeta.MP_LENGTH || def == TpSlotMeta.TP_GET_ATTR);
7979

80-
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.sq_mp_length());
81-
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.mp_sq_length());
82-
checkSlotValue(TpSlotMeta.TP_GET_ATTR, slots.tp_get_attro_attr());
80+
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.combined_sq_mp_length());
81+
checkSlotValue(TpSlotMeta.MP_LENGTH, slots.combined_mp_sq_length());
82+
checkSlotValue(TpSlotMeta.TP_GET_ATTR, slots.combined_tp_getattro_getattr());
8383
}
8484

8585
@Test
@@ -90,9 +90,9 @@ public void testBuilderOptimizations2() {
9090
TpSlots slots = builder.build();
9191
verifySlots(slots, def -> def == TpSlotMeta.SQ_LENGTH);
9292

93-
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.sq_mp_length());
94-
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.mp_sq_length());
95-
Assert.assertNull(slots.tp_get_attro_attr());
93+
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.combined_sq_mp_length());
94+
checkSlotValue(TpSlotMeta.SQ_LENGTH, slots.combined_mp_sq_length());
95+
Assert.assertNull(slots.combined_tp_getattro_getattr());
9696
}
9797

9898
private static void verifySlots(TpSlots slots, Function<TpSlotMeta, Boolean> checkNonNullValue) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ public boolean hasArrayElements(
288288
if (behavior != null) {
289289
return getValue.executeBoolean(inliningTarget, behavior, method, toBooleanNode, raiseNode, this);
290290
} else {
291-
return sequenceCheck.execute(inliningTarget, this) && getSlotsNode.execute(inliningTarget, this).sq_mp_length() != null;
291+
return sequenceCheck.execute(inliningTarget, this) && getSlotsNode.execute(inliningTarget, this).combined_sq_mp_length() != null;
292292
}
293293
} finally {
294294
gil.release(mustRelease);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/iterator/IteratorNodes.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -196,9 +196,9 @@ static int length(VirtualFrame frame, Node inliningTarget, Object iterable,
196196
}
197197
Object clazz = getClassNode.execute(inliningTarget, iterable);
198198
TpSlots slots = getSlotsNode.execute(inliningTarget, clazz);
199-
if (hasLenProfile.profile(inliningTarget, slots.sq_mp_length() != null)) {
199+
if (hasLenProfile.profile(inliningTarget, slots.combined_sq_mp_length() != null)) {
200200
try {
201-
return callSlotLenNode.execute(frame, inliningTarget, slots.sq_mp_length(), iterable);
201+
return callSlotLenNode.execute(frame, inliningTarget, slots.combined_sq_mp_length(), iterable);
202202
} catch (PException e) {
203203
// This is done (maybe aside other things) because of ProxyType.__len__ calls
204204
// len builtin, which may raise this for objects that do not have len slot. This

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@
119119
import com.oracle.truffle.nfi.api.SignatureLibrary;
120120

121121
/**
122-
* Wraps fields that hold slot values, instances of {@link TpSlot}, such as {@link #tp_get_attr()}.
122+
* Wraps fields that hold slot values, instances of {@link TpSlot}, such as {@link #tp_getattr()}.
123123
* This is GraalPython equivalent of the same fields in CPython's {@code PyTypeObject}.
124124
* <p>
125125
* Summary of the interactions:
@@ -165,22 +165,22 @@
165165
* - CPython does that in the type's tp_setattr(o) slot, se we are inconsistent here, which can be fixed later
166166
* </pre>
167167
* <p>
168-
* Note: fields such as {@link #sq_mp_length()} are optimization: the value is {@link #sq_length()}
169-
* if non-null otherwise {@link #mp_length()}.
168+
* Note: fields with "combined" prefix, such as {@link #combined_sq_mp_length()}, are optimization:
169+
* the value is {@link #sq_length()} if non-null otherwise {@link #mp_length()}.
170170
*
171-
* @param tp_get_attr Note: in CPython, this slot is considered deprecated and the preferred slot
172-
* should be {@link #tp_get_attro()}. We assume that no builtins define this slot.
171+
* @param tp_getattr Note: in CPython, this slot is considered deprecated and the preferred slot
172+
* should be {@link #tp_getattro()}. We assume that no builtins define this slot.
173173
*/
174174
public record TpSlots(TpSlot nb_bool, //
175175
TpSlot sq_length, //
176176
TpSlot mp_length, //
177-
TpSlot sq_mp_length, //
178-
TpSlot mp_sq_length, //
177+
TpSlot combined_sq_mp_length, //
178+
TpSlot combined_mp_sq_length, //
179179
TpSlot tp_descr_get, //
180180
TpSlot tp_descr_set, //
181-
TpSlot tp_get_attro, //
182-
TpSlot tp_get_attr, //
183-
TpSlot tp_get_attro_attr) {
181+
TpSlot tp_getattro, //
182+
TpSlot tp_getattr, //
183+
TpSlot combined_tp_getattro_getattr) {
184184

185185
private static final TruffleLogger LOGGER = PythonLanguage.getLogger(TpSlot.class);
186186

@@ -284,15 +284,15 @@ public enum TpSlotMeta {
284284
PExternalFunctionWrapper.DESCR_SET,
285285
DescrSetFunctionWrapper::new),
286286
TP_GET_ATTRO(
287-
TpSlots::tp_get_attro,
287+
TpSlots::tp_getattro,
288288
TpSlotGetAttrPython.class,
289289
TpSlotGetAttrBuiltin.class,
290290
CFields.PyTypeObject__tp_getattro,
291291
null,
292292
PExternalFunctionWrapper.BINARYFUNC,
293293
GetAttrWrapper::new),
294294
TP_GET_ATTR(
295-
TpSlots::tp_get_attr,
295+
TpSlots::tp_getattr,
296296
null,
297297
null,
298298
CFields.PyTypeObject__tp_getattr,

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,19 +172,19 @@ public Object getType() {
172172
}
173173

174174
/**
175-
* For managed types, this calls {@link TpSlots#tp_get_attro_attr()}, because their signature
176-
* difference is not visible on the managed level. For native slots, this calls
177-
* {@link TpSlots#tp_get_attro()} if available, otherwise {@link TpSlots#tp_get_attr()} - at
178-
* least one of those must be available, i.e., the caller is expected to check precondition
179-
* {@code tp_get_attro_attr() != null}.
175+
* For managed types, this calls {@link TpSlots#combined_tp_getattro_getattr()}, because their
176+
* signature difference is not visible on the managed level. For native slots, this calls
177+
* {@link TpSlots#tp_getattro()} if available, otherwise {@link TpSlots#tp_getattr()} - at least
178+
* one of those must be available, i.e., the caller is expected to check precondition
179+
* {@code combined_tp_getattro_getattr() != null}.
180180
*/
181181
@GenerateInline
182182
@GenerateCached(false)
183183
@GenerateUncached
184184
public abstract static class CallSlotGetAttrNode extends Node {
185185
public final Object execute(VirtualFrame frame, Node inliningTarget, TpSlots slots, Object self, TruffleString name) {
186-
assert slots.tp_get_attro_attr() != null;
187-
return executeImpl(frame, inliningTarget, slots, slots.tp_get_attro_attr(), self, name);
186+
assert slots.combined_tp_getattro_getattr() != null;
187+
return executeImpl(frame, inliningTarget, slots, slots.combined_tp_getattro_getattr(), self, name);
188188
}
189189

190190
abstract Object executeImpl(VirtualFrame frame, Node inliningTarget, TpSlots slots, TpSlot tp_get_attro_attr, Object self, TruffleString name);
@@ -211,8 +211,8 @@ static Object callNative(VirtualFrame frame, TpSlots slots, TpSlotNative slot, O
211211
@GenerateUncached
212212
public abstract static class CallSlotGetAttrONode extends Node {
213213
public final Object execute(VirtualFrame frame, Node inliningTarget, TpSlots slots, Object self, Object name) {
214-
assert slots.tp_get_attro_attr() != null;
215-
return executeImpl(frame, inliningTarget, slots, slots.tp_get_attro_attr(), self, name);
214+
assert slots.combined_tp_getattro_getattr() != null;
215+
return executeImpl(frame, inliningTarget, slots, slots.combined_tp_getattro_getattr(), self, name);
216216
}
217217

218218
abstract Object executeImpl(VirtualFrame frame, Node inliningTarget, TpSlots slots, TpSlot tp_get_attro_attr, Object self, Object name);
@@ -249,7 +249,7 @@ static Object callNative(VirtualFrame frame, TpSlots slots, TpSlotNative slot, O
249249
@Cached NativeToPythonTransferNode toPythonNode,
250250
@Cached PyObjectCheckFunctionResultNode checkResultNode) {
251251
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget, null);
252-
boolean isGetAttr = isGetAttrProfile.profile(inliningTarget, slots.tp_get_attr() == slot);
252+
boolean isGetAttr = isGetAttrProfile.profile(inliningTarget, slots.tp_getattr() == slot);
253253
Object nameArg;
254254
if (isGetAttr) {
255255
nameArg = asCharPointerNode.execute(name);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectGetMethod.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -103,7 +103,7 @@ public final Object executeCached(Frame frame, Object receiver, TruffleString na
103103

104104
protected static boolean isObjectGetAttribute(Node inliningTarget, GetCachedTpSlotsNode getSlotsNode, Object lazyClass) {
105105
TpSlots slots = getSlotsNode.execute(inliningTarget, lazyClass);
106-
return slots.tp_get_attro() == ObjectBuiltins.SLOTS.tp_get_attro();
106+
return slots.tp_getattro() == ObjectBuiltins.SLOTS.tp_getattro();
107107
}
108108

109109
// isObjectGetAttribute implies not foreign

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectIsTrueNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ static boolean doIt(VirtualFrame frame, Object object,
185185
return callBoolNode.execute(frame, inliningTarget, slots.nb_bool(), object);
186186
}
187187
lenLookupBranch.enter(inliningTarget);
188-
TpSlot lenSlot = slots.mp_sq_length();
188+
TpSlot lenSlot = slots.combined_mp_sq_length();
189189
if (hasLenProfile.profile(inliningTarget, lenSlot != null)) {
190190
return callLenNode.execute(frame, inliningTarget, lenSlot, object) != 0;
191191
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectLookupAttr.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,19 @@ protected static boolean hasNoGetAttr(Object lazyClass) {
113113

114114
protected static boolean getAttributeIs(Node inliningTarget, GetCachedTpSlotsNode getSlotsNode, Object lazyClass, TpSlot slot) {
115115
TpSlots slots = getSlotsNode.execute(inliningTarget, lazyClass);
116-
return slots.tp_get_attro() == slot;
116+
return slots.tp_getattro() == slot;
117117
}
118118

119119
protected static boolean isObjectGetAttribute(Node inliningTarget, GetCachedTpSlotsNode getSlotsNode, Object lazyClass) {
120-
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, ObjectBuiltins.SLOTS.tp_get_attro());
120+
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, ObjectBuiltins.SLOTS.tp_getattro());
121121
}
122122

123123
protected static boolean isModuleGetAttribute(Node inliningTarget, GetCachedTpSlotsNode getSlotsNode, Object lazyClass) {
124-
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, ModuleBuiltins.SLOTS.tp_get_attro());
124+
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, ModuleBuiltins.SLOTS.tp_getattro());
125125
}
126126

127127
protected static boolean isTypeGetAttribute(Node inliningTarget, GetCachedTpSlotsNode getSlotsNode, Object lazyClass) {
128-
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, TypeBuiltins.SLOTS.tp_get_attro());
128+
return getAttributeIs(inliningTarget, getSlotsNode, lazyClass, TypeBuiltins.SLOTS.tp_getattro());
129129
}
130130

131131
protected static boolean isBuiltinTypeType(Object type) {
@@ -150,7 +150,7 @@ static Object doBuiltinObject(VirtualFrame frame, Node inliningTarget, Object ob
150150
@Bind("lookupName.execute(type)") Object descr,
151151
@Shared @Cached(inline = false) ReadAttributeFromObjectNode readNode) {
152152
// It should not have __getattr__, because otherwise it would not have builtin
153-
// object#tp_get_attro, but slot wrapper dispatching to __getattribute__ or __getattr__
153+
// object#tp_getattro, but slot wrapper dispatching to __getattribute__ or __getattr__
154154
assert hasNoGetAttr(type);
155155
return readNode.execute(object, cachedName);
156156
}
@@ -276,12 +276,12 @@ static Object getDynamicAttr(Frame frame, Node inliningTarget, Object receiver,
276276
Object type = getClass.execute(inliningTarget, receiver);
277277
TpSlots slots = getSlotsNode.execute(inliningTarget, type);
278278
if (!codePointLengthNode.isAdoptable()) {
279-
// It pays to try this in the uncached case, avoiding a full call to tp_get_attr(o)
279+
// It pays to try this in the uncached case, avoiding a full call to tp_getattr(o)
280280
Object result = readAttributeQuickly(type, slots, receiver, name, codePointLengthNode, codePointAtIndexNode);
281281
if (result != null) {
282282
return result;
283283
}
284-
// Otherwise fallback to tp_get_attr(o)
284+
// Otherwise fallback to tp_getattr(o)
285285
}
286286
try {
287287
return callGetattribute.execute((VirtualFrame) frame, inliningTarget, slots, receiver, name);
@@ -319,7 +319,7 @@ public static PyObjectLookupAttr getUncached() {
319319
*/
320320
static Object readAttributeQuickly(Object type, TpSlots slots, Object receiver, TruffleString stringName, TruffleString.CodePointLengthNode codePointLengthNode,
321321
TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
322-
if (slots.tp_get_attro() == ObjectBuiltins.SLOTS.tp_get_attro() && type instanceof PythonManagedClass) {
322+
if (slots.tp_getattro() == ObjectBuiltins.SLOTS.tp_getattro() && type instanceof PythonManagedClass) {
323323
PythonAbstractClass[] bases = ((PythonManagedClass) type).getBaseClasses();
324324
if (bases.length == 1) {
325325
PythonAbstractClass base = bases[0];
@@ -337,7 +337,7 @@ static Object readAttributeQuickly(Object type, TpSlots slots, Object receiver,
337337
}
338338
}
339339
}
340-
} else if (slots.tp_get_attro() == ModuleBuiltins.SLOTS.tp_get_attro() && type == PythonBuiltinClassType.PythonModule) {
340+
} else if (slots.tp_getattro() == ModuleBuiltins.SLOTS.tp_getattro() && type == PythonBuiltinClassType.PythonModule) {
341341
// this is slightly simpler than the previous case, since we don't need to check
342342
// the type. There may be a module-level __getattr__, however. Since that would be
343343
// a call anyway, we return to the generic code in that case

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectLookupAttrO.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ static Object getDynamicAttr(Frame frame, Node inliningTarget, Object receiver,
8484
Object type = getClass.execute(inliningTarget, receiver);
8585
TpSlots slots = getSlotsNode.execute(inliningTarget, type);
8686
if (!codePointLengthNode.isAdoptable() && name instanceof TruffleString tsName) {
87-
// It pays to try this in the uncached case, avoiding a full call to tp_get_attr(o)
87+
// It pays to try this in the uncached case, avoiding a full call to tp_getattr(o)
8888
Object result = readAttributeQuickly(type, slots, receiver, tsName, codePointLengthNode, codePointAtIndexNode);
8989
if (result != null) {
9090
return result;
9191
}
92-
// Otherwise fallback to tp_get_attr(o)
92+
// Otherwise fallback to tp_getattr(o)
9393
}
9494
try {
9595
return callGetattribute.execute((VirtualFrame) frame, inliningTarget, slots, receiver, name);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectSetAttr.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ static void setFixedAttr(Frame frame, Node inliningTarget, Object self, @Suppres
108108
Object setattr = lookupSetattr.execute(frame, type, self);
109109
if (setattr == PNone.NO_VALUE) {
110110
TpSlots slots = getSlotsNode.execute(inliningTarget, type);
111-
if (slots.tp_get_attro_attr() == null) {
111+
if (slots.combined_tp_getattro_getattr() == null) {
112112
throw raise.get(inliningTarget).raise(TypeError, P_HAS_NO_ATTRS_S_TO_ASSIGN, self, name);
113113
} else {
114114
throw raise.get(inliningTarget).raise(TypeError, P_HAS_RO_ATTRS_S_TO_ASSIGN, self, name);
@@ -129,7 +129,7 @@ static void delFixedAttr(Frame frame, Node inliningTarget, Object self, @Suppres
129129
Object delattr = lookupDelattr.execute(frame, type, self);
130130
if (delattr == PNone.NO_VALUE) {
131131
TpSlots slots = getSlotsNode.execute(inliningTarget, type);
132-
if (slots.tp_get_attro_attr() == null) {
132+
if (slots.combined_tp_getattro_getattr() == null) {
133133
throw raise.get(inliningTarget).raise(TypeError, P_HAS_NO_ATTRS_S_TO_DELETE, self, name);
134134
} else {
135135
throw raise.get(inliningTarget).raise(TypeError, P_HAS_RO_ATTRS_S_TO_DELETE, self, name);

0 commit comments

Comments
 (0)