From 66d2e95f57595c034ff02dbab235cb08305e64ae Mon Sep 17 00:00:00 2001 From: Jeongseop Lim Date: Tue, 20 Oct 2026 02:42:34 +0900 Subject: [PATCH] Return the dict reverse iterator types from __reversed__ The __reversed__ methods of dict, dict_keys, dict_items and dict_values wrapped the reverse storage iterator with the forward factories, so reversed(d) was a dict_keyiterator and type(reversed(d)) is type(iter(d)) held. The PDictReverse*Iterator types were still defined and registered with IteratorBuiltins, but nothing created them since afc1f123e6 dropped the reverse factories. Restore createDictReverse{Key,Item,Value}Iterator in their former shape and use them at the four sites, as CPython builds these iterators from the PyDictRevIter*_Type types. Signed-off-by: Jeongseop Lim --- .../src/tests/test_dict.py | 13 +++++++++++++ .../builtins/objects/dict/DictBuiltins.java | 2 +- .../builtins/objects/dict/DictValuesBuiltins.java | 4 ++-- .../builtins/objects/dict/DictViewBuiltins.java | 4 ++-- .../graal/python/runtime/object/PFactory.java | 15 +++++++++++++++ 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_dict.py b/graalpython/com.oracle.graal.python.test/src/tests/test_dict.py index 63ce7afee6..96b1a538f3 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_dict.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/test_dict.py @@ -2007,3 +2007,16 @@ def test_bulk_value_update_preserves_iterators(): d.update({k: -v for k, v in d.items()}) assert list(it) == expected[1:] assert list(reverse) == expected[-2::-1] + + +def test_reversed_iterator_types(): + d = {1: 'a', 2: 'b'} + assert type(reversed(d)).__name__ == 'dict_reversekeyiterator' + assert type(reversed(d.keys())).__name__ == 'dict_reversekeyiterator' + assert type(reversed(d.values())).__name__ == 'dict_reversevalueiterator' + assert type(reversed(d.items())).__name__ == 'dict_reverseitemiterator' + assert type(reversed(d)) is not type(iter(d)) + assert_raises(TypeError, type(reversed(d))) + assert_raises(TypeError, type(reversed(d.values()))) + assert_raises(TypeError, type(reversed(d.items()))) + assert list(reversed(d.items())) == [(2, 'b'), (1, 'a')] diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictBuiltins.java index af5b42dcb5..2ecf206665 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictBuiltins.java @@ -436,7 +436,7 @@ static Object run(Object self, @Cached HashingStorageGetReverseIterator getReverseIterator, @Bind PythonLanguage language) { var storage = getStorageNode.execute(inliningTarget, self); - return PFactory.createDictKeyIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); + return PFactory.createDictReverseKeyIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); } } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictValuesBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictValuesBuiltins.java index 4e27468861..c4c8c9dc40 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictValuesBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictValuesBuiltins.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2025, Oracle and/or its affiliates. + * Copyright (c) 2017, 2026, Oracle and/or its affiliates. * Copyright (c) 2013, Regents of the University of California * * All rights reserved. @@ -109,7 +109,7 @@ static Object doPDictValuesView(PDictValuesView self, @Cached HashingStorageGetReverseIterator getReverseIter, @Bind PythonLanguage language) { HashingStorage storage = self.getWrappedStorage(); - return PFactory.createDictValueIterator(language, getReverseIter.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); + return PFactory.createDictReverseValueIterator(language, getReverseIter.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); } } } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictViewBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictViewBuiltins.java index a203480f5d..2f4ee1db81 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictViewBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictViewBuiltins.java @@ -193,7 +193,7 @@ static Object getReversedKeysViewIter(PDictKeysView self, @Shared @Cached HashingStorageGetReverseIterator getReverseIterator, @Bind PythonLanguage language) { HashingStorage storage = self.getWrappedStorage(); - return PFactory.createDictKeyIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); + return PFactory.createDictReverseKeyIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); } @Specialization @@ -203,7 +203,7 @@ static Object getReversedItemsViewIter(PDictItemsView self, @Shared @Cached HashingStorageGetReverseIterator getReverseIterator, @Bind PythonLanguage language) { HashingStorage storage = self.getWrappedStorage(); - return PFactory.createDictItemIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); + return PFactory.createDictReverseItemIterator(language, getReverseIterator.execute(inliningTarget, storage), storage, lenNode.execute(inliningTarget, storage)); } } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PFactory.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PFactory.java index 989b5a5c52..9dedd345b2 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PFactory.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PFactory.java @@ -1037,14 +1037,29 @@ public static PDictItemIterator createDictItemIterator(PythonLanguage language, initialSize); } + public static PDictItemIterator createDictReverseItemIterator(PythonLanguage language, HashingStorageNodes.HashingStorageIterator iterator, HashingStorage hashingStorage, int initialSize) { + return new PDictItemIterator(PythonBuiltinClassType.PDictReverseItemIterator, PythonBuiltinClassType.PDictReverseItemIterator.getInstanceShape(language), iterator, hashingStorage, + initialSize); + } + public static PDictKeyIterator createDictKeyIterator(PythonLanguage language, HashingStorageNodes.HashingStorageIterator iterator, HashingStorage hashingStorage, int initialSize) { return new PDictKeyIterator(PythonBuiltinClassType.PDictKeyIterator, PythonBuiltinClassType.PDictKeyIterator.getInstanceShape(language), iterator, hashingStorage, initialSize); } + public static PDictKeyIterator createDictReverseKeyIterator(PythonLanguage language, HashingStorageNodes.HashingStorageIterator iterator, HashingStorage hashingStorage, int initialSize) { + return new PDictKeyIterator(PythonBuiltinClassType.PDictReverseKeyIterator, PythonBuiltinClassType.PDictReverseKeyIterator.getInstanceShape(language), iterator, hashingStorage, + initialSize); + } + public static PDictValueIterator createDictValueIterator(PythonLanguage language, HashingStorageNodes.HashingStorageIterator iterator, HashingStorage hashingStorage, int initialSize) { return new PDictValueIterator(PythonBuiltinClassType.PDictValueIterator, PythonBuiltinClassType.PDictValueIterator.getInstanceShape(language), iterator, hashingStorage, initialSize); } + public static PDictValueIterator createDictReverseValueIterator(PythonLanguage language, HashingStorageNodes.HashingStorageIterator iterator, HashingStorage hashingStorage, int initialSize) { + return new PDictValueIterator(PythonBuiltinClassType.PDictReverseValueIterator, PythonBuiltinClassType.PDictReverseValueIterator.getInstanceShape(language), iterator, hashingStorage, + initialSize); + } + public static Object createSentinelIterator(PythonLanguage language, Object callable, Object sentinel) { return new PSentinelIterator(PythonBuiltinClassType.PSentinelIterator, PythonBuiltinClassType.PSentinelIterator.getInstanceShape(language), callable, sentinel); }