From 790211b46027597621f27e1cd9177095ca471fcc Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 11 Jun 2026 17:10:33 -0400 Subject: [PATCH 1/2] fix(stl_bind): correct __delitem__ for negative-step slices and re-enable contiguous erase fast path The slice __delitem__ binding advanced the erase index by step - 1 for all steps. That correction is only valid for positive steps, where erasing shifts later elements down by one. For negative steps the visited indices are strictly decreasing and erasing never shifts them, so the extra -1 deleted the wrong elements (e.g. del v[::-2] on [0,1,2,3] yielded [1,2] instead of [0,2]) and del v[::-1] walked off the front of the vector (v.begin() - 1, observed SIGBUS). Switch to the signed slice::compute overload so negative steps stay signed, advance by step for negative steps and step - 1 for positive ones, and drop the && false that had disabled the O(n) contiguous fast path since 2016. Assisted-by: ClaudeCode:claude-fable-5 --- include/pybind11/stl_bind.h | 16 +++++++++++----- tests/test_stl_binders.py | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 8202300c70..4722065676 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -286,18 +286,24 @@ void vector_modifiers( cl.def( "__delitem__", [](Vector &v, const slice &slice) { - size_t start = 0, stop = 0, step = 0, slicelength = 0; + ssize_t start = 0, stop = 0, step = 0, slicelength = 0; - if (!slice.compute(v.size(), &start, &stop, &step, &slicelength)) { + if (!slice.compute((ssize_t) v.size(), &start, &stop, &step, &slicelength)) { throw error_already_set(); } - if (step == 1 && false) { + if (step == 1) { v.erase(v.begin() + (DiffType) start, v.begin() + DiffType(start + slicelength)); } else { - for (size_t i = 0; i < slicelength; ++i) { + // For a positive step, erasing an element shifts the remaining + // (later) elements down by one, so the next index to erase is + // ``start + step - 1``. For a negative step the visited indices + // are strictly decreasing, so erasing never shifts them and the + // next index is simply ``start + step``. + ssize_t offset = step > 0 ? step - 1 : step; + for (ssize_t i = 0; i < slicelength; ++i) { v.erase(v.begin() + DiffType(start)); - start += step - 1; + start += offset; } } }, diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 518f2df2b6..d147ba9288 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -67,6 +67,29 @@ def test_vector_int(): assert len(v_int2) == 0 +def test_vector_delitem_slice(): + slice_cases = [ + slice(1, 4), + slice(None, None, 2), + slice(1, None, 2), + slice(None, None, -1), + slice(None, None, -2), + slice(3, 1, -1), + slice(2, 2), + slice(None), + slice(5, 0, -2), + slice(-3, -1), + slice(None, None, -3), + ] + for n in range(8): + for s in slice_cases: + ref = list(range(n)) + got = m.VectorInt(range(n)) + del ref[s] + del got[s] + assert list(got) == ref, f"n={n} slice={s}" + + # Older PyPy's failed here, related to the PyPy's buffer protocol. def test_vector_buffer(): b = bytearray([1, 2, 3, 4]) From e5c1c6d02044f7d43351a18501819b3583ba9129 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 12 Jun 2026 15:40:36 -0400 Subject: [PATCH 2/2] =?UTF-8?q?refactor:=20address=20review=20=E2=80=94=20?= =?UTF-8?q?static=5Fcast=20and=20parametrized=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use static_cast instead of a C-style cast for the slice.compute() size argument, and convert the __delitem__ slice test to pytest.mark.parametrize over the slice cases. Assisted-by: ClaudeCode:claude-fable-5 --- include/pybind11/stl_bind.h | 3 ++- tests/test_stl_binders.py | 20 +++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 4722065676..5d47aacb16 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -288,7 +288,8 @@ void vector_modifiers( [](Vector &v, const slice &slice) { ssize_t start = 0, stop = 0, step = 0, slicelength = 0; - if (!slice.compute((ssize_t) v.size(), &start, &stop, &step, &slicelength)) { + if (!slice.compute( + static_cast(v.size()), &start, &stop, &step, &slicelength)) { throw error_already_set(); } diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index d147ba9288..7836a5a55b 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -67,8 +67,9 @@ def test_vector_int(): assert len(v_int2) == 0 -def test_vector_delitem_slice(): - slice_cases = [ +@pytest.mark.parametrize( + "s", + [ slice(1, 4), slice(None, None, 2), slice(1, None, 2), @@ -80,14 +81,15 @@ def test_vector_delitem_slice(): slice(5, 0, -2), slice(-3, -1), slice(None, None, -3), - ] + ], +) +def test_vector_delitem_slice(s): for n in range(8): - for s in slice_cases: - ref = list(range(n)) - got = m.VectorInt(range(n)) - del ref[s] - del got[s] - assert list(got) == ref, f"n={n} slice={s}" + ref = list(range(n)) + got = m.VectorInt(range(n)) + del ref[s] + del got[s] + assert list(got) == ref, f"n={n}" # Older PyPy's failed here, related to the PyPy's buffer protocol.