From 27b9a490ee34c8b4235d746aaae5faee1260cfc0 Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Sat, 4 Oct 2025 22:27:28 +0530 Subject: [PATCH 1/6] Destructive iteration tests [skip ci] --- tests/functional/test_keys_iter.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/functional/test_keys_iter.py b/tests/functional/test_keys_iter.py index 4c044919..72431739 100644 --- a/tests/functional/test_keys_iter.py +++ b/tests/functional/test_keys_iter.py @@ -65,3 +65,19 @@ def test_modify_with_active_iterators(sorted_dict, iterators, advance): ): sorted_dict.clear() sorted_dict_copy.clear() + + +@pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) +def test_destructive_forward_iteration(sorted_dict): + for key in sorted_dict: + del sorted_dict[key] + assert len(sorted_dict) == 0 + assert not [*sorted_dict] + + +@pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) +def test_destructive_reverse_iteration(sorted_dict): + for key in reversed(sorted_dict): + del sorted_dict[key] + assert len(sorted_dict) == 0 + assert not [*sorted_dict] From 6855e87b835550089036f2d98b3c6f20dd121e87 Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Sun, 5 Oct 2025 01:28:48 +0530 Subject: [PATCH 2/6] Not working [skip ci] --- src/pysorteddict/sorted_dict_view_type.cc | 31 ++++++++++++++++++++--- tests/functional/test_keys_iter.py | 7 ++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/pysorteddict/sorted_dict_view_type.cc b/src/pysorteddict/sorted_dict_view_type.cc index 1d8228c1..e2c61dd5 100644 --- a/src/pysorteddict/sorted_dict_view_type.cc +++ b/src/pysorteddict/sorted_dict_view_type.cc @@ -57,7 +57,11 @@ void SortedDictViewIterType::track(RevIterType it) } if (it != this->sd->map->rend()) { - ++it->second.known_referrers; + FwdIterType it_base = it.base(); + if (it_base != this->sd->map->end()) + { + ++it_base->second.known_referrers; + } } else { @@ -68,7 +72,7 @@ void SortedDictViewIterType::track(RevIterType it) } /** - * Do all the necessary bookkeeping required to stop tracking the given + * Do all the necessary bookkeeping required to stop tracking the given forward * iterator of the underlying sorted dictionary. * * The caller should ensure that this method is called immediately after the @@ -76,12 +80,31 @@ void SortedDictViewIterType::track(RevIterType it) * * @param it Previous value of the iterator member. */ -template -void SortedDictViewIterType::untrack(T it) +template<> +void SortedDictViewIterType::untrack(FwdIterType it) { --it->second.known_referrers; } +/** + * Do all the necessary bookkeeping required to stop tracking the given reverse + * iterator of the underlying sorted dictionary. + * + * The caller should ensure that this method is called immediately after the + * iterator member is updated. + * + * @param it Previous value of the iterator member. + */ +template<> +void SortedDictViewIterType::untrack(RevIterType it) +{ + FwdIterType it_base = it.base(); + if (it_base != this->sd->map->end()) + { + --it->second.known_referrers; + } +} + template void SortedDictViewIterType::Delete(PyObject* self) { diff --git a/tests/functional/test_keys_iter.py b/tests/functional/test_keys_iter.py index 72431739..f348aab8 100644 --- a/tests/functional/test_keys_iter.py +++ b/tests/functional/test_keys_iter.py @@ -77,7 +77,8 @@ def test_destructive_forward_iteration(sorted_dict): @pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) def test_destructive_reverse_iteration(sorted_dict): + prev_key = None for key in reversed(sorted_dict): - del sorted_dict[key] - assert len(sorted_dict) == 0 - assert not [*sorted_dict] + if prev_key: + del sorted_dict[key] + prev_key = key From 1831d3c743207fdc62c58627a2a0aa82f933235b Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Tue, 7 Oct 2025 19:50:28 +0530 Subject: [PATCH 3/6] Failing test demo --- tests/functional/test_keys_iter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/test_keys_iter.py b/tests/functional/test_keys_iter.py index f348aab8..ca97d64c 100644 --- a/tests/functional/test_keys_iter.py +++ b/tests/functional/test_keys_iter.py @@ -75,6 +75,7 @@ def test_destructive_forward_iteration(sorted_dict): assert not [*sorted_dict] +@pytest.mark.skip("bug fix needed") @pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) def test_destructive_reverse_iteration(sorted_dict): prev_key = None From 4b0971f1b73eb691b3a9c1b6c4de1a3afddfb54a Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Sun, 12 Oct 2025 21:39:50 +0530 Subject: [PATCH 4/6] Address review comment --- src/pysorteddict/sorted_dict_view_type.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pysorteddict/sorted_dict_view_type.cc b/src/pysorteddict/sorted_dict_view_type.cc index e2c61dd5..6aeeac21 100644 --- a/src/pysorteddict/sorted_dict_view_type.cc +++ b/src/pysorteddict/sorted_dict_view_type.cc @@ -101,7 +101,7 @@ void SortedDictViewIterType::untrack(RevIterType it) FwdIterType it_base = it.base(); if (it_base != this->sd->map->end()) { - --it->second.known_referrers; + --it_base->second.known_referrers; } } From fd65f3c093e32b48304edc8ca4b54f35f83a572f Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Sun, 12 Oct 2025 23:25:12 +0530 Subject: [PATCH 5/6] Destructive reverse iteration test --- tests/functional/test_keys_iter.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_keys_iter.py b/tests/functional/test_keys_iter.py index 729ef8d9..54b55028 100644 --- a/tests/functional/test_keys_iter.py +++ b/tests/functional/test_keys_iter.py @@ -75,11 +75,17 @@ def test_destructive_forward_iteration(sorted_dict): assert not [*sorted_dict] -@pytest.mark.skip("feature not implemented correctly") @pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) def test_destructive_reverse_iteration(sorted_dict): + sorted_dict_len = len(sorted_dict) prev_key = None for key in reversed(sorted_dict): + # A quirk of the implementation of reverse iterators: the current key + # cannot be deleted. if prev_key: - del sorted_dict[key] + del sorted_dict[prev_key] prev_key = key + if sorted_dict_len > 0: + del sorted_dict[prev_key] + assert len(sorted_dict) == 0 + assert not [*sorted_dict] From 221bb3285dfc596b34c84d13d9f33bd568e19dbf Mon Sep 17 00:00:00 2001 From: Vishal Pankaj Chandratreya <19171016+tfpf@users.noreply.github.com> Date: Mon, 13 Oct 2025 00:02:16 +0530 Subject: [PATCH 6/6] Fix test --- tests/functional/test_keys_iter.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_keys_iter.py b/tests/functional/test_keys_iter.py index 54b55028..9c6411f2 100644 --- a/tests/functional/test_keys_iter.py +++ b/tests/functional/test_keys_iter.py @@ -77,15 +77,14 @@ def test_destructive_forward_iteration(sorted_dict): @pytest.mark.parametrize("sorted_dict", [*range(10), 100, 1_000, 10_000, 100_000], indirect=True) def test_destructive_reverse_iteration(sorted_dict): - sorted_dict_len = len(sorted_dict) prev_key = None for key in reversed(sorted_dict): # A quirk of the implementation of reverse iterators: the current key # cannot be deleted. - if prev_key: + if prev_key is not None: del sorted_dict[prev_key] prev_key = key - if sorted_dict_len > 0: + if prev_key is not None: del sorted_dict[prev_key] assert len(sorted_dict) == 0 assert not [*sorted_dict]