From 913b8fadc5fc45306764c0ede472e1bc3b606d10 Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Mon, 12 Jun 2023 13:42:09 -0500 Subject: [PATCH 1/4] Don't remove const for reverse iteration --- Cython/Compiler/ExprNodes.py | 27 --------------------------- Cython/Includes/libcpp/map.pxd | 8 ++++++-- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index ad4701b7bb6..275a6233da5 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -3268,32 +3268,6 @@ def free_temps(self, code): ExprNode.free_temps(self, code) -def remove_const(item_type): - """ - Removes the constness of a given type and its underlying templates - if any. - - This is to solve the compilation error when the temporary variable used to - store the result of an iterator cannot be changed due to its constness. - For example, the value_type of std::map, which will also be the type of - the temporarry variable, is std::pair. This means the first - component of the variable cannot be reused to store the result of each - iteration, which leads to a compilation error. - """ - if item_type.is_const: - item_type = item_type.cv_base_type - if item_type.is_typedef: - item_type = remove_const(item_type.typedef_base_type) - if item_type.is_cpp_class and item_type.templates: - templates = [remove_const(t) if t.is_const else t for t in item_type.templates] - template_type = item_type.template_type - item_type = PyrexTypes.CppClassType( - template_type.name, template_type.scope, - template_type.cname, template_type.base_classes, - templates, template_type) - return item_type - - class NextNode(AtomicExprNode): # Used as part of for statement implementation. # Implements result = next(iterator) @@ -3336,7 +3310,6 @@ def infer_type(self, env, iterator_type=None): def analyse_types(self, env): self.type = self.infer_type(env, self.iterator.type) - self.type = remove_const(self.type) self.is_temp = 1 return self diff --git a/Cython/Includes/libcpp/map.pxd b/Cython/Includes/libcpp/map.pxd index d81af66e09a..2c2c5c82adf 100644 --- a/Cython/Includes/libcpp/map.pxd +++ b/Cython/Includes/libcpp/map.pxd @@ -50,7 +50,9 @@ cdef extern from "" namespace "std" nogil: cppclass reverse_iterator: reverse_iterator() except + reverse_iterator(reverse_iterator&) except + - value_type& operator*() + # correct would be value_type& but this does not work + # well with cython's code gen + pair[T, U]& operator*() reverse_iterator operator++() reverse_iterator operator--() reverse_iterator operator++(int) @@ -63,7 +65,9 @@ cdef extern from "" namespace "std" nogil: const_reverse_iterator() except + const_reverse_iterator(reverse_iterator&) except + operator=(reverse_iterator&) except + - const value_type& operator*() + # correct would be const value_type& but this does not work + # well with cython's code gen + const pair[T, U]& operator*() const_reverse_iterator operator++() const_reverse_iterator operator--() const_reverse_iterator operator++(int) From cb804f989eaa9938e72d0336d82bb7aa0003455f Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Mon, 12 Jun 2023 13:57:58 -0500 Subject: [PATCH 2/4] Add test from gh5478 --- tests/run/cpp_iterators.pyx | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/run/cpp_iterators.pyx b/tests/run/cpp_iterators.pyx index 81048d0b36b..424168fa825 100644 --- a/tests/run/cpp_iterators.pyx +++ b/tests/run/cpp_iterators.pyx @@ -7,6 +7,7 @@ from libcpp.map cimport map as stdmap from libcpp.set cimport set as stdset from libcpp.string cimport string from libcpp.vector cimport vector +from libcpp.memory cimport shared_ptr, make_shared from cython.operator cimport dereference as deref cdef extern from "cpp_iterators_simple.h": @@ -272,6 +273,27 @@ def test_iteration_over_attribute_of_call(): for i in get_object_with_iterable_attribute().vec: print(i) +cdef extern from *: + # TODO: support make_shared[const int] + shared_ptr[const int] make_shared_const_int "std::make_shared"(int) + +def test_iteration_over_shared_const_ptr_set(py_v): + """ + >>> test_iteration_over_shared_const_ptr_set[2, 4, 6]) + 6 + 4 + 2 + """ + cdef stdset[shared_ptr[const int]] s + cdef int i + for e in py_v: + i = e + s.insert(make_shared_const_int(i)) + + cdef shared_ptr[const int] a + for a in s: + print(deref(a)) + def test_iteration_over_reversed_list(py_v): """ >>> test_iteration_over_reversed_list([2, 4, 6]) From a9bfacdcf5358e9d5a1d3c8ab0dd2eff6f18018a Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Mon, 12 Jun 2023 14:07:37 -0500 Subject: [PATCH 3/4] Fix multimap too --- Cython/Includes/libcpp/map.pxd | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Cython/Includes/libcpp/map.pxd b/Cython/Includes/libcpp/map.pxd index 2c2c5c82adf..eb739509ac1 100644 --- a/Cython/Includes/libcpp/map.pxd +++ b/Cython/Includes/libcpp/map.pxd @@ -177,7 +177,9 @@ cdef extern from "" namespace "std" nogil: cppclass reverse_iterator: reverse_iterator() except + reverse_iterator(reverse_iterator&) except + - value_type& operator*() + # correct would be value_type& but this does not work + # well with cython's code gen + pair[T, U]& operator*() reverse_iterator operator++() reverse_iterator operator--() reverse_iterator operator++(int) @@ -190,7 +192,9 @@ cdef extern from "" namespace "std" nogil: const_reverse_iterator() except + const_reverse_iterator(reverse_iterator&) except + operator=(reverse_iterator&) except + - const value_type& operator*() + # correct would be const value_type& but this does not work + # well with cython's code gen + const pair[T, U]& operator*() const_reverse_iterator operator++() const_reverse_iterator operator--() const_reverse_iterator operator++(int) From 0528cd937e6d4606eb0902ee8d8db672ee7f88fe Mon Sep 17 00:00:00 2001 From: Isuru Fernando Date: Mon, 12 Jun 2023 17:50:41 -0500 Subject: [PATCH 4/4] Fix test --- tests/run/cpp_iterators.pyx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/run/cpp_iterators.pyx b/tests/run/cpp_iterators.pyx index 424168fa825..57d2716bea5 100644 --- a/tests/run/cpp_iterators.pyx +++ b/tests/run/cpp_iterators.pyx @@ -277,18 +277,17 @@ cdef extern from *: # TODO: support make_shared[const int] shared_ptr[const int] make_shared_const_int "std::make_shared"(int) -def test_iteration_over_shared_const_ptr_set(py_v): +def test_iteration_over_shared_const_ptr_vector(py_v): """ - >>> test_iteration_over_shared_const_ptr_set[2, 4, 6]) - 6 - 4 + >>> test_iteration_over_shared_const_ptr_vector([2, 4, 6]) 2 + 4 + 6 """ - cdef stdset[shared_ptr[const int]] s + cdef vector[shared_ptr[const int]] s cdef int i - for e in py_v: - i = e - s.insert(make_shared_const_int(i)) + for i in py_v: + s.push_back(make_shared_const_int(i)) cdef shared_ptr[const int] a for a in s: