Skip to content

Commit d2bccd4

Browse files
youknowoneyouknowone
committed
str: count a stepped slice's characters with a ceiling division
`do_stepped_slice` and `do_stepped_slice_reverse` took the character count of a non-ASCII result as `(range.len() / step) + 1`, which overshoots by one whenever the span is an exact multiple of the step: `"aéc"[::3]` is one character and reported two, `"가나다라"[::2]` two and reported three. ASCII subjects were unaffected, since that arm collects into an `AsciiString` whose length comes from the data. The count is stored as the string's character length, so the result then claimed a character its buffer does not hold, and `reversed()` on it indexed past the end and panicked: s = "".join(["a", "é", "c"]) list(reversed(s[::3])) # index out of bounds: the len is 1 but the index is 1 Use `div_ceil`, which is the number of elements the underlying range yields. A differential over 44331 subscript and slice shapes -- every combination of start, stop and step over strings straddling the ASCII split, the surrogate range and the astral plane -- now matches CPython 3.14 exactly, where it differed on 3100 lines before. Assisted-by: Claude
1 parent d04318e commit d2bccd4

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

‎crates/vm/src/builtins/str.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,14 +1870,14 @@ impl SliceableSequenceOp for PyStr {
18701870
.collect::<AsciiString>()
18711871
.into(),
18721872
PyKindStr::Utf8(s) => {
1873-
let char_len = (range.len() / step) + 1;
1873+
let char_len = range.len().div_ceil(step);
18741874
let mut out = String::with_capacity(2 * char_len);
18751875
out.extend(s.chars().skip(range.start).take(range.len()).step_by(step));
18761876
// SAFETY: char_len is accurate
18771877
unsafe { Self::new_with_char_len(out, char_len) }
18781878
}
18791879
PyKindStr::Wtf8(w) => {
1880-
let char_len = (range.len() / step) + 1;
1880+
let char_len = range.len().div_ceil(step);
18811881
let mut out = Wtf8Buf::with_capacity(2 * char_len);
18821882
out.extend(
18831883
w.code_points()
@@ -1900,7 +1900,7 @@ impl SliceableSequenceOp for PyStr {
19001900
.collect::<AsciiString>()
19011901
.into(),
19021902
PyKindStr::Utf8(s) => {
1903-
let char_len = (range.len() / step) + 1;
1903+
let char_len = range.len().div_ceil(step);
19041904
// not ascii, so the codepoints have to be at least 2 bytes each
19051905
let mut out = String::with_capacity(2 * char_len);
19061906
out.extend(
@@ -1914,7 +1914,7 @@ impl SliceableSequenceOp for PyStr {
19141914
unsafe { Self::new_with_char_len(out, char_len) }
19151915
}
19161916
PyKindStr::Wtf8(w) => {
1917-
let char_len = (range.len() / step) + 1;
1917+
let char_len = range.len().div_ceil(step);
19181918
// not ascii, so the codepoints have to be at least 2 bytes each
19191919
let mut out = Wtf8Buf::with_capacity(2 * char_len);
19201920
out.extend(

‎extra_tests/snippets/builtin_str_unicode_slice.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,32 @@ def expect_index_error(s, index):
5959
assert len(hebrew_text[30:10:-3]) == 7
6060
assert hebrew_text[30:10:-1] == "א ,םיִהֹלֱא אָרָּב ,"
6161
assert len(hebrew_text[30:10:-1]) == 20
62+
63+
64+
# A stepped slice whose span is an exact multiple of the step ends on the last
65+
# character it collects rather than one past it, so the character count is the
66+
# span divided by the step and not one more. The subject goes through a
67+
# variable because a constant subscript is folded at compile time and would
68+
# never reach the runtime slice at all.
69+
def stepped(s, step):
70+
return s[::step]
71+
72+
73+
for subject, step, expected in [
74+
("a\u00e9c", 3, "a"),
75+
("가나다라", 2, "가다"),
76+
("가나다라마바", 3, "가라"),
77+
("가나다라", -2, "라나"),
78+
("가나다라마바", -3, "바다"),
79+
("\U0001f600\U0001f601\U0001f602\U0001f603", 2, "\U0001f600\U0001f602"),
80+
]:
81+
sliced = stepped(subject, step)
82+
assert sliced == expected, (subject, step, sliced)
83+
assert len(sliced) == len(expected), (subject, step, len(sliced))
84+
# An overstated count makes the string claim characters its buffer does not
85+
# hold, which reversed() then reads past.
86+
assert list(reversed(sliced)) == list(expected)[::-1]
87+
88+
assert len(stepped(hebrew_text, 2)) == 30
89+
assert len(stepped(hebrew_text, 4)) == 15
90+
assert len(stepped(hebrew_text, -2)) == 30

0 commit comments

Comments
 (0)