Skip to content

Commit 2531059

Browse files
steve-ssteve-s
committed
Change isMapping{Type} messages to follow PyMapping_Check semantics
1 parent 028e735 commit 2531059

File tree

3 files changed

+47
-46
lines changed

3 files changed

+47
-46
lines changed

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

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.AttributeError;
4444
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
4545
import static com.oracle.graal.python.nodes.SpecialMethodNames.FILENO;
46+
import static com.oracle.graal.python.nodes.SpecialMethodNames.ITEMS;
47+
import static com.oracle.graal.python.nodes.SpecialMethodNames.KEYS;
48+
import static com.oracle.graal.python.nodes.SpecialMethodNames.VALUES;
4649
import static com.oracle.graal.python.nodes.SpecialMethodNames.__BOOL__;
4750
import static com.oracle.graal.python.nodes.SpecialMethodNames.__CALL__;
4851
import static com.oracle.graal.python.nodes.SpecialMethodNames.__DELETE__;
@@ -135,16 +138,16 @@
135138
import com.oracle.graal.python.runtime.exception.PException;
136139
import com.oracle.graal.python.util.OverflowException;
137140
import com.oracle.truffle.api.CompilerDirectives;
138-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
139141
import com.oracle.truffle.api.TruffleLanguage;
142+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
140143
import com.oracle.truffle.api.dsl.Cached;
141-
import com.oracle.truffle.api.dsl.Cached.Exclusive;
142-
import com.oracle.truffle.api.dsl.Cached.Shared;
143144
import com.oracle.truffle.api.dsl.CachedContext;
144145
import com.oracle.truffle.api.dsl.GenerateUncached;
145146
import com.oracle.truffle.api.dsl.ImportStatic;
146147
import com.oracle.truffle.api.dsl.ReportPolymorphism;
147148
import com.oracle.truffle.api.dsl.Specialization;
149+
import com.oracle.truffle.api.dsl.Cached.Exclusive;
150+
import com.oracle.truffle.api.dsl.Cached.Shared;
148151
import com.oracle.truffle.api.interop.ArityException;
149152
import com.oracle.truffle.api.interop.InteropLibrary;
150153
import com.oracle.truffle.api.interop.InvalidArrayIndexException;
@@ -191,6 +194,21 @@ public final void clearNativeWrapper(ConditionProfile hasHandleValidAssumptionPr
191194
nativeWrapper = null;
192195
}
193196

197+
/**
198+
* Checks if the object is a Mapping as described in the
199+
* <a href="https://docs.python.org/3/reference/datamodel.html">Python Data Model</a>. Mappings
200+
* are treated differently to other containers in some interop messages.
201+
*/
202+
private static boolean isAbstractMapping(Object receiver, PythonObjectLibrary lib) {
203+
return lib.isSequence(receiver) && lib.lookupAttribute(receiver, KEYS, true) != PNone.NO_VALUE && //
204+
lib.lookupAttribute(receiver, ITEMS, true) != PNone.NO_VALUE && //
205+
lib.lookupAttribute(receiver, VALUES, true) != PNone.NO_VALUE;
206+
}
207+
208+
private boolean isAbstractMapping(PythonObjectLibrary thisLib) {
209+
return isAbstractMapping(this, thisLib);
210+
}
211+
194212
@ExportMessage
195213
public void writeMember(String key, Object value,
196214
@Exclusive @Cached PInteropSubscriptAssignNode setItemNode,
@@ -218,7 +236,7 @@ public void writeMember(String key, Object value,
218236
return;
219237
}
220238
}
221-
if (dataModelLibrary.isMapping(this)) {
239+
if (isAbstractMapping(dataModelLibrary)) {
222240
setItemNode.execute(this, key, value);
223241
} else {
224242
writeNode.execute(this, key, value);
@@ -277,7 +295,7 @@ public Object readMember(String key,
277295
@ExportMessage
278296
public boolean hasArrayElements(
279297
@CachedLibrary("this") PythonObjectLibrary dataModelLibrary) {
280-
return dataModelLibrary.isSequence(this) && !dataModelLibrary.isMapping(this);
298+
return dataModelLibrary.isSequence(this) && !isAbstractMapping(dataModelLibrary);
281299
}
282300

283301
@ExportMessage
@@ -289,7 +307,7 @@ public Object readArrayElement(long key,
289307
try {
290308
return toForeign.executeConvert(getItemNode.execute(this, key));
291309
} catch (PException e) {
292-
if (dataModelLibrary.isMapping(this)) {
310+
if (isAbstractMapping(dataModelLibrary)) {
293311
throw UnsupportedMessageException.create();
294312
} else {
295313
// TODO(fa) refine exception handling
@@ -504,8 +522,8 @@ public Object getMembers(boolean includeInternal,
504522
}
505523
if (includeInternal) {
506524
// we use the internal flag to also return dictionary keys for mappings
507-
if (dataModelLibrary.isMapping(this)) {
508-
PList mapKeys = castToList.executeWithGlobalState(keysNode.executeObject(this, SpecialMethodNames.KEYS));
525+
if (isAbstractMapping(dataModelLibrary)) {
526+
PList mapKeys = castToList.executeWithGlobalState(keysNode.executeObject(this, KEYS));
509527
int len = lenNode.execute(mapKeys);
510528
for (int i = 0; i < len; i++) {
511529
Object key = getItemNode.execute(mapKeys, i);
@@ -549,7 +567,7 @@ public void removeMember(String member,
549567
return;
550568
}
551569
}
552-
if (dataModelLibrary.isMapping(this) && getDelItemNode.execute(this, __DELITEM__) != PNone.NO_VALUE) {
570+
if (isAbstractMapping(dataModelLibrary) && getDelItemNode.execute(this, __DELITEM__) != PNone.NO_VALUE) {
553571
delItemNode.execute(this, member);
554572
} else {
555573
deleteAttributeNode.execute(this, member);
@@ -644,9 +662,9 @@ public boolean isMapping(@CachedLibrary("this") PythonObjectLibrary plib,
644662
public boolean isSequenceType(
645663
@CachedLibrary("this") PythonObjectLibrary lib,
646664
@Shared("hasGetItemNode") @Cached LookupAttributeInMRONode.Dynamic hasGetItemNode,
647-
@Shared("hasLenNode") @Cached LookupAttributeInMRONode.Dynamic hasLenNode,
665+
@Exclusive @Cached LookupAttributeInMRONode.Dynamic hasLenNode,
648666
@Shared("isLazyClass") @Cached("createBinaryProfile()") ConditionProfile isLazyClass,
649-
@Shared("lenProfile") @Cached("createBinaryProfile()") ConditionProfile lenProfile,
667+
@Exclusive @Cached("createBinaryProfile()") ConditionProfile lenProfile,
650668
@Shared("getItemProfile") @Cached("createBinaryProfile()") ConditionProfile getItemProfile) {
651669
if (isLazyClass.profile(lib.isLazyPythonClass(this))) {
652670
if (lenProfile.profile(hasLenNode.execute(this, __LEN__) != PNone.NO_VALUE)) {
@@ -660,18 +678,10 @@ public boolean isSequenceType(
660678
public boolean isMappingType(
661679
@CachedLibrary("this") PythonObjectLibrary lib,
662680
@Shared("hasGetItemNode") @Cached LookupAttributeInMRONode.Dynamic hasGetItemNode,
663-
@Shared("hasLenNode") @Cached LookupAttributeInMRONode.Dynamic hasLenNode,
664681
@Shared("isLazyClass") @Cached("createBinaryProfile()") ConditionProfile isLazyClass,
665-
@Shared("lenProfile") @Cached("createBinaryProfile()") ConditionProfile lenProfile,
666-
@Shared("getItemProfile") @Cached("createBinaryProfile()") ConditionProfile getItemProfile,
667-
@Exclusive @Cached LookupAttributeInMRONode.Dynamic hasKeysNode,
668-
@Exclusive @Cached LookupAttributeInMRONode.Dynamic hasItemsNode,
669-
@Exclusive @Cached LookupAttributeInMRONode.Dynamic hasValuesNode,
670-
@Exclusive @Cached("createBinaryProfile()") ConditionProfile profile) {
671-
if (isSequenceType(lib, hasGetItemNode, hasLenNode, isLazyClass, lenProfile, getItemProfile)) {
672-
return profile.profile(hasKeysNode.execute(this, SpecialMethodNames.KEYS) != PNone.NO_VALUE &&
673-
hasItemsNode.execute(this, SpecialMethodNames.ITEMS) != PNone.NO_VALUE &&
674-
hasValuesNode.execute(this, SpecialMethodNames.VALUES) != PNone.NO_VALUE);
682+
@Shared("getItemProfile") @Cached("createBinaryProfile()") ConditionProfile getItemProfile) {
683+
if (isLazyClass.profile(lib.isLazyPythonClass(this))) {
684+
return getItemProfile.profile(hasGetItemNode.execute(this, __GETITEM__) != PNone.NO_VALUE);
675685
}
676686
return false;
677687
}
@@ -1504,7 +1514,7 @@ int access(Object object, String fieldName,
15041514
info |= REMOVABLE;
15051515
info |= MODIFIABLE;
15061516
}
1507-
} else if (!isImmutable.execute(object) || dataModelLibrary.isMapping(object)) {
1517+
} else if (!isImmutable.execute(object) || isAbstractMapping(object, dataModelLibrary)) {
15081518
// If the member does not exist yet, it is insertable if this object is mutable,
15091519
// i.e., it's not a builtin object or it is a mapping.
15101520
info |= INSERTABLE;

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

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@
6262
import com.oracle.truffle.api.interop.InteropLibrary;
6363
import com.oracle.truffle.api.interop.UnsupportedMessageException;
6464
import com.oracle.truffle.api.library.GenerateLibrary;
65-
import com.oracle.truffle.api.library.GenerateLibrary.Abstract;
66-
import com.oracle.truffle.api.library.GenerateLibrary.DefaultExport;
6765
import com.oracle.truffle.api.library.Library;
6866
import com.oracle.truffle.api.library.LibraryFactory;
67+
import com.oracle.truffle.api.library.GenerateLibrary.Abstract;
68+
import com.oracle.truffle.api.library.GenerateLibrary.DefaultExport;
6969
import com.oracle.truffle.api.nodes.Node;
7070
import com.oracle.truffle.api.nodes.NodeCost;
7171
import com.oracle.truffle.api.profiles.ConditionProfile;
@@ -808,10 +808,10 @@ public int lengthWithFrame(Object receiver, ConditionProfile hasFrameProfile, Vi
808808
}
809809

810810
/**
811-
* Checks whether the receiver is a Python mapping. As described in the
812-
* <a href="https://docs.python.org/3/reference/datamodel.html">Python Data Model</a> and
813-
* <a href="https://docs.python.org/3/library/collections.abc.html">Abstract Base Classes for
814-
* Containers</a>
811+
* Checks whether the receiver is a Python mapping. This message is supposed to be an equivalent
812+
* of CPython's {@code PyCheck_Mapping}. Note that such object does not have to conform to the
813+
* definition of mapping as described in
814+
* <a href="https://docs.python.org/3/reference/datamodel.html">Python Data Model</a>.
815815
*
816816
* <br>
817817
* See {@link #isMappingType(Object)}
@@ -846,21 +846,14 @@ public boolean isSequenceType(Object receiver) {
846846
}
847847

848848
/**
849-
* Checks whether the receiver is a Python mapping type. As described in the
850-
* <a href="https://docs.python.org/3/reference/datamodel.html">Python Data Model</a> and
851-
* <a href="https://docs.python.org/3/library/collections.abc.html">Abstract Base Classes for
852-
* Containers</a>
849+
* Checks whether the receiver is a Python mapping. This message is supposed to be an equivalent
850+
* of CPython's {@code PyCheck_Mapping}. Note that such object does not have to conform to the
851+
* definition of mapping as described in
852+
* <a href="https://docs.python.org/3/reference/datamodel.html">Python Data Model</a>.
853853
*
854854
* <br>
855-
* Specifically the default implementation checks whether the receiver
856-
* {@link #isSequenceType(Object)} and for the implementation of the following special methods:
857-
* <b>
858-
* <ul>
859-
* <li>keys</li>
860-
* <li>items</li>
861-
* <li>values</li>
862-
* </ul>
863-
* </b>
855+
* Specifically the default implementation checks whether the receiver has the {@code __items__}
856+
* special method.
864857
*
865858
* @param receiver the receiver Object
866859
* @return True if a mapping type

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/formatting/FormatProcessor.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
*/
77
package com.oracle.graal.python.runtime.formatting;
88

9-
import static com.oracle.graal.python.nodes.SpecialMethodNames.__GETITEM__;
109
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INDEX__;
1110
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INT__;
1211
import static com.oracle.graal.python.runtime.exception.PythonErrorType.MemoryError;
@@ -18,7 +17,6 @@
1817
import java.math.MathContext;
1918

2019
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
21-
import com.oracle.graal.python.builtins.objects.PNone;
2220
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
2321
import com.oracle.graal.python.builtins.objects.floats.PFloat;
2422
import com.oracle.graal.python.builtins.objects.function.PKeyword;
@@ -232,8 +230,8 @@ protected boolean isString(Object args1, Object lazyClass) {
232230
return PGuards.isString(args1) || isSubtype(lazyClass, PythonBuiltinClassType.PString);
233231
}
234232

235-
protected boolean isMapping(Object args1) {
236-
return lookupAttribute(args1, __GETITEM__) != PNone.NO_VALUE;
233+
protected static boolean isMapping(Object args1) {
234+
return PythonObjectLibrary.getUncached().isMapping(args1);
237235
}
238236

239237
protected static boolean isSubtype(Object lazyClass, PythonBuiltinClassType clazz) {

0 commit comments

Comments
 (0)