Skip to content

Commit 83726da

Browse files
tomasstupkatomasstupka
committed
[GR-23216] Make test_dictviews pass - reversed().
PullRequest: graalpython/1023
2 parents ef9afca + ccd1b62 commit 83726da

25 files changed

+428
-103
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
3939

40+
import unittest, sys
4041

4142
def assert_raises(err, fn, *args, **kwargs):
4243
raised = False
@@ -710,7 +711,13 @@ def test_wrapped_string_get():
710711
a = 'test'
711712
dict = locals()
712713
assert dict['a']
713-
714+
715+
@unittest.skipIf(sys.implementation.name == 'cpython' and sys.version_info[0:2] < (3, 8), "skipping for cPython versions < 3.8")
716+
def test_reverse_locals():
717+
a = 'atest'
718+
b = 'btest'
719+
dict = locals()
720+
assert list(reversed(dict)) == ['b', 'a']
714721

715722
def test_concat():
716723
r = {**{}}

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_dict.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,15 @@
3333
*graalpython.lib-python.3.test.test_dict.DictTest.test_mutating_iteration
3434
*graalpython.lib-python.3.test.test_dict.DictTest.test_mutating_lookup
3535
*graalpython.lib-python.3.test.test_dict.DictTest.test_object_set_item_single_instance_non_str_key
36+
*graalpython.lib-python.3.test.test_dict.DictTest.test_pop
3637
*graalpython.lib-python.3.test.test_dict.DictTest.test_reentrant_insertion
3738
*graalpython.lib-python.3.test.test_dict.DictTest.test_repr
39+
*graalpython.lib-python.3.test.test_dict.DictTest.test_repr_deep
3840
*graalpython.lib-python.3.test.test_dict.DictTest.test_resize1
3941
*graalpython.lib-python.3.test.test_dict.DictTest.test_resize2
42+
*graalpython.lib-python.3.test.test_dict.DictTest.test_reverse_iterator_for_empty_dict
43+
*graalpython.lib-python.3.test.test_dict.DictTest.test_reverse_iterator_for_shared_shared_dicts
44+
*graalpython.lib-python.3.test.test_dict.DictTest.test_reversed
4045
*graalpython.lib-python.3.test.test_dict.DictTest.test_setdefault
4146
*graalpython.lib-python.3.test.test_dict.DictTest.test_setitem_atomic_at_resize
4247
*graalpython.lib-python.3.test.test_dict.DictTest.test_splittable_del

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_dictviews.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_compare_error
33
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_constructors_not_callable
44
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_copy
5+
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_deeply_nested_repr
56
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_dict_items
67
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_dict_keys
78
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_dict_mixed_keys_items
@@ -10,3 +11,4 @@
1011
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_items_set_operations
1112
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_keys_set_operations
1213
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_pickle
14+
*graalpython.lib-python.3.test.test_dictviews.DictSetTest.test_recursive_repr

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltinClassType.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,14 @@ public enum PythonBuiltinClassType implements LazyPythonClass {
6969
PCell("cell"),
7070
PComplex("complex", BuiltinNames.BUILTINS),
7171
PDict("dict", BuiltinNames.BUILTINS),
72-
PDictKeysView(BuiltinNames.DICT_KEYS),
73-
PDictItemsIterator(BuiltinNames.DICT_ITEMITERATOR),
72+
PDictItemIterator(BuiltinNames.DICT_ITEMITERATOR),
73+
PDictReverseItemIterator(BuiltinNames.DICT_REVERSE_ITEMITERATOR),
7474
PDictItemsView(BuiltinNames.DICT_ITEMS),
75-
PDictKeysIterator(BuiltinNames.DICT_KEYITERATOR),
76-
PDictValuesIterator(BuiltinNames.DICT_VALUEITERATOR),
75+
PDictKeyIterator(BuiltinNames.DICT_KEYITERATOR),
76+
PDictReverseKeyIterator(BuiltinNames.DICT_REVERSE_KEYITERATOR),
77+
PDictKeysView(BuiltinNames.DICT_KEYS),
78+
PDictValueIterator(BuiltinNames.DICT_VALUEITERATOR),
79+
PDictReverseValueIterator(BuiltinNames.DICT_REVERSE_VALUEITERATOR),
7780
PDictValuesView(BuiltinNames.DICT_VALUES),
7881
PEllipsis("ellipsis"),
7982
PEnumerate("enumerate", BuiltinNames.BUILTINS),

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2686,7 +2686,7 @@ public Object dictKeys(Object args, Object kwargs) {
26862686
}
26872687
}
26882688

2689-
@Builtin(name = DICT_KEYITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictKeysIterator, isPublic = false)
2689+
@Builtin(name = DICT_KEYITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictKeyIterator, isPublic = false)
26902690
@GenerateNodeFactory
26912691
public abstract static class DictKeysIteratorTypeNode extends PythonBuiltinNode {
26922692
@SuppressWarnings("unused")
@@ -2706,7 +2706,7 @@ public Object dictKeys(Object args, Object kwargs) {
27062706
}
27072707
}
27082708

2709-
@Builtin(name = DICT_VALUEITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictValuesIterator, isPublic = false)
2709+
@Builtin(name = DICT_VALUEITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictValueIterator, isPublic = false)
27102710
@GenerateNodeFactory
27112711
public abstract static class DictValuesIteratorTypeNode extends PythonBuiltinNode {
27122712
@SuppressWarnings("unused")
@@ -2726,7 +2726,7 @@ public Object dictKeys(Object args, Object kwargs) {
27262726
}
27272727
}
27282728

2729-
@Builtin(name = DICT_ITEMITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictItemsIterator, isPublic = false)
2729+
@Builtin(name = DICT_ITEMITERATOR, takesVarArgs = true, takesVarKeywordArgs = true, constructsClass = PythonBuiltinClassType.PDictItemIterator, isPublic = false)
27302730
@GenerateNodeFactory
27312731
public abstract static class DictItemsIteratorTypeNode extends PythonBuiltinNode {
27322732
@SuppressWarnings("unused")

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.oracle.graal.python.builtins.objects.code.CodeNodes.CreateCodeNode;
5252
import com.oracle.graal.python.builtins.objects.code.PCode;
5353
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
54+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
5455
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
5556
import com.oracle.graal.python.builtins.objects.common.HashingStorage.DictEntry;
5657
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
@@ -412,11 +413,13 @@ void handlePList(VirtualFrame frame, PList l, int version, DataOutputStream buff
412413
@Specialization(limit = "1")
413414
void handlePDict(VirtualFrame frame, PDict d, int version, DataOutputStream buffer,
414415
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
415-
@CachedLibrary("d.getDictStorage()") HashingStorageLibrary lib) {
416+
@Cached GetDictStorageNode getStore,
417+
@CachedLibrary("getStore.execute(d)") HashingStorageLibrary lib) {
416418
writeByte(TYPE_DICT, version, buffer);
417-
int len = lib.lengthWithFrame(d.getDictStorage(), hasFrame, frame);
419+
HashingStorage dictStorage = getStore.execute(d);
420+
int len = lib.lengthWithFrame(dictStorage, hasFrame, frame);
418421
writeInt(len, version, buffer);
419-
for (DictEntry entry : d.entries()) {
422+
for (DictEntry entry : lib.entries(dictStorage)) {
420423
getRecursiveNode().execute(frame, entry.key, version, buffer);
421424
getRecursiveNode().execute(frame, entry.value, version, buffer);
422425
}
@@ -443,33 +446,37 @@ private static String getSourceCode(PCode c) {
443446
@Specialization(limit = "1")
444447
void handlePSet(VirtualFrame frame, PSet s, int version, DataOutputStream buffer,
445448
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
446-
@CachedLibrary("s.getDictStorage()") HashingStorageLibrary lib) {
449+
@Cached GetDictStorageNode getStore,
450+
@CachedLibrary("getStore.execute(s)") HashingStorageLibrary lib) {
447451
writeByte(TYPE_SET, version, buffer);
448452
int len;
453+
HashingStorage dictStorage = getStore.execute(s);
449454
if (hasFrame.profile(frame != null)) {
450-
len = lib.lengthWithState(s.getDictStorage(), PArguments.getThreadState(frame));
455+
len = lib.lengthWithState(dictStorage, PArguments.getThreadState(frame));
451456
} else {
452-
len = lib.length(s.getDictStorage());
457+
len = lib.length(dictStorage);
453458
}
454459
writeInt(len, version, buffer);
455-
for (DictEntry entry : s.entries()) {
460+
for (DictEntry entry : lib.entries(dictStorage)) {
456461
getRecursiveNode().execute(frame, entry.key, version, buffer);
457462
}
458463
}
459464

460465
@Specialization(limit = "1")
461466
void handlePForzenSet(VirtualFrame frame, PFrozenSet s, int version, DataOutputStream buffer,
462467
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
463-
@CachedLibrary("s.getDictStorage()") HashingStorageLibrary lib) {
468+
@Cached GetDictStorageNode getStore,
469+
@CachedLibrary("getStore.execute(s)") HashingStorageLibrary lib) {
464470
writeByte(TYPE_FROZENSET, version, buffer);
465471
int len;
472+
HashingStorage dictStorage = getStore.execute(s);
466473
if (hasFrame.profile(frame != null)) {
467-
len = lib.lengthWithState(s.getDictStorage(), PArguments.getThreadState(frame));
474+
len = lib.lengthWithState(dictStorage, PArguments.getThreadState(frame));
468475
} else {
469-
len = lib.length(s.getDictStorage());
476+
len = lib.length(dictStorage);
470477
}
471478
writeInt(len, version, buffer);
472-
for (DictEntry entry : s.entries()) {
479+
for (DictEntry entry : lib.entries(dictStorage)) {
473480
getRecursiveNode().execute(frame, entry.key, version, buffer);
474481
}
475482
}

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

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import com.oracle.truffle.api.object.Shape;
7373
import com.oracle.truffle.api.profiles.BranchProfile;
7474
import com.oracle.truffle.api.profiles.ConditionProfile;
75+
import java.util.List;
7576

7677
/**
7778
* This storage keeps a reference to the MRO when used for a type dict. Writing to this storage will
@@ -358,22 +359,31 @@ public HashingStorageIterable<Object> keys(
358359
return new HashingStorageIterable<>(new KeysIterator(store, readNode));
359360
}
360361

361-
private static final class KeysIterator implements Iterator<Object> {
362-
private final Iterator<Object> keyIter;
362+
@ExportMessage
363+
public HashingStorageIterable<Object> reverseKeys(
364+
@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {
365+
return new HashingStorageIterable<>(new ReverseKeysIterator(store, readNode));
366+
}
367+
368+
private abstract static class AbstractKeysIterator implements Iterator<Object> {
363369
private final DynamicObject store;
364370
private final ReadAttributeFromDynamicObjectNode readNode;
365371
private Object next = null;
366372

367-
public KeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
368-
this.keyIter = store.getShape().getKeys().iterator();
373+
public AbstractKeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
369374
this.store = store;
370375
this.readNode = readNode;
371376
}
372377

378+
protected abstract boolean hasNextKey();
379+
380+
protected abstract Object nextKey();
381+
373382
@TruffleBoundary
383+
@Override
374384
public boolean hasNext() {
375-
while (next == null && keyIter.hasNext()) {
376-
Object key = keyIter.next();
385+
while (next == null && hasNextKey()) {
386+
Object key = nextKey();
377387
Object value = readNode.execute(store, key);
378388
if (value != PNone.NO_VALUE) {
379389
next = key;
@@ -382,6 +392,7 @@ public boolean hasNext() {
382392
return next != null;
383393
}
384394

395+
@Override
385396
public Object next() {
386397
hasNext(); // find the next value
387398
if (next != null) {
@@ -394,6 +405,47 @@ public Object next() {
394405
}
395406
}
396407

408+
private static final class KeysIterator extends AbstractKeysIterator {
409+
private final Iterator<Object> keyIter;
410+
411+
public KeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
412+
super(store, readNode);
413+
this.keyIter = store.getShape().getKeys().iterator();
414+
}
415+
416+
@Override
417+
protected boolean hasNextKey() {
418+
return keyIter.hasNext();
419+
}
420+
421+
@Override
422+
protected Object nextKey() {
423+
return keyIter.next();
424+
}
425+
}
426+
427+
private static final class ReverseKeysIterator extends AbstractKeysIterator {
428+
private final List<Object> keyList;
429+
private int index;
430+
431+
public ReverseKeysIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
432+
super(store, readNode);
433+
this.keyList = store.getShape().getKeyList();
434+
this.index = keyList.size() - 1;
435+
}
436+
437+
@Override
438+
protected boolean hasNextKey() {
439+
return index >= 0;
440+
}
441+
442+
@Override
443+
protected Object nextKey() {
444+
return keyList.get(index--);
445+
}
446+
447+
}
448+
397449
@ExportMessage
398450
public HashingStorageIterable<DictEntry> entries(
399451
@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,12 @@ public HashingStorageIterable<Object> keys() {
574574
return new HashingStorageIterable<>(new KeysIterator(map.getKeys().iterator()));
575575
}
576576

577+
@Override
578+
@ExportMessage
579+
public HashingStorageIterable<Object> reverseKeys() {
580+
return new HashingStorageIterable<>(new KeysIterator(map.reverseKeyIterator()));
581+
}
582+
577583
private static final class KeysIterator implements Iterator<Object> {
578584
private final Iterator<DictKey> keysIterator;
579585

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,12 @@ public HashingStorage copy() {
118118
}
119119

120120
private static final Iterator<Object> KEYS_ITERATOR = new Iterator<Object>() {
121+
@Override
121122
public boolean hasNext() {
122123
return false;
123124
}
124125

126+
@Override
125127
public Object next() {
126128
throw new NoSuchElementException();
127129
}
@@ -132,4 +134,11 @@ public Object next() {
132134
public HashingStorageIterable<Object> keys() {
133135
return new HashingStorageIterable<>(KEYS_ITERATOR);
134136
}
137+
138+
@ExportMessage
139+
@Override
140+
public HashingStorageIterable<Object> reverseKeys() {
141+
return new HashingStorageIterable<>(KEYS_ITERATOR);
142+
}
143+
135144
}

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,12 @@ HashingStorageIterable<Object> keys() {
276276
throw new AbstractMethodError("HashingStorage.keys");
277277
}
278278

279+
@ExportMessage
280+
public HashingStorageIterable<Object> reverseKeys() {
281+
CompilerDirectives.transferToInterpreterAndInvalidate();
282+
throw new AbstractMethodError("HashingStorage.reverseKeys");
283+
}
284+
279285
@GenerateUncached
280286
protected abstract static class AddToOtherInjectNode extends InjectIntoNode {
281287
@Specialization
@@ -542,14 +548,19 @@ public HashingStorage diff(HashingStorage other,
542548

543549
@ExportMessage
544550
public HashingStorageIterable<Object> values(@CachedLibrary("this") HashingStorageLibrary lib) {
545-
return new HashingStorageIterable<>(new ValuesIterator(this, lib));
551+
return new HashingStorageIterable<>(new ValuesIterator(this, lib.keys(this).iterator(), lib));
552+
}
553+
554+
@ExportMessage
555+
public HashingStorageIterable<Object> reverseValues(@CachedLibrary("this") HashingStorageLibrary lib) {
556+
return new HashingStorageIterable<>(new ValuesIterator(this, lib.reverseKeys(this).iterator(), lib));
546557
}
547558

548559
private static final class ValuesIterator implements Iterator<Object> {
549560
private final Iterator<DictEntry> entriesIterator;
550561

551-
ValuesIterator(HashingStorage self, HashingStorageLibrary lib) {
552-
this.entriesIterator = new EntriesIterator(self, lib);
562+
ValuesIterator(HashingStorage self, HashingStorageIterator<Object> keysIterator, HashingStorageLibrary lib) {
563+
this.entriesIterator = new EntriesIterator(self, keysIterator, lib);
553564
}
554565

555566
@Override
@@ -564,19 +575,24 @@ public Object next() {
564575
}
565576

566577
@ExportMessage
567-
public HashingStorageIterable<DictEntry> entries(@CachedLibrary("this") HashingStorageLibrary lib) {
568-
return new HashingStorageIterable<>(new EntriesIterator(this, lib));
578+
public final HashingStorageIterable<DictEntry> entries(@CachedLibrary("this") HashingStorageLibrary lib) {
579+
return new HashingStorageIterable<>(new EntriesIterator(this, lib.keys(this).iterator(), lib));
580+
}
581+
582+
@ExportMessage
583+
public final HashingStorageIterable<DictEntry> reverseEntries(@CachedLibrary("this") HashingStorageLibrary lib) {
584+
return new HashingStorageIterable<>(new EntriesIterator(this, lib.reverseKeys(this).iterator(), lib));
569585
}
570586

571587
private static final class EntriesIterator implements Iterator<DictEntry> {
572588
private final HashingStorageIterator<Object> keysIterator;
573589
private final HashingStorage self;
574590
private final HashingStorageLibrary lib;
575591

576-
EntriesIterator(HashingStorage self, HashingStorageLibrary lib) {
592+
EntriesIterator(HashingStorage self, HashingStorageIterator<Object> keysIterator, HashingStorageLibrary lib) {
577593
this.self = self;
578594
this.lib = lib;
579-
this.keysIterator = lib.keys(self).iterator();
595+
this.keysIterator = keysIterator;
580596
}
581597

582598
@Override

0 commit comments

Comments
 (0)