Skip to content

Commit b6c9882

Browse files
authored
Fix bug in Sliceable and rename del_item* to delitem (RustPython#4333)
* fix bug in sliceable * fixup
1 parent d5b70b0 commit b6c9882

File tree

4 files changed

+17
-21
lines changed

4 files changed

+17
-21
lines changed

stdlib/src/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,15 +337,15 @@ mod array {
337337
fn delitem_by_index(&mut self, i: isize, vm: &VirtualMachine) -> PyResult<()> {
338338
match self {
339339
$(ArrayContentType::$n(v) => {
340-
v.del_item_by_index(vm, i)
340+
v.delitem_by_index(vm, i)
341341
})*
342342
}
343343
}
344344

345345
fn delitem_by_slice(&mut self, slice: SaturatedSlice, vm: &VirtualMachine) -> PyResult<()> {
346346
match self {
347347
$(ArrayContentType::$n(v) => {
348-
v.del_item_by_slice(vm, slice)
348+
v.delitem_by_slice(vm, slice)
349349
})*
350350
}
351351
}

vm/src/builtins/bytearray.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,12 +226,10 @@ impl PyByteArray {
226226

227227
pub fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> {
228228
match SequenceIndex::try_from_borrowed_object(vm, needle, "bytearray")? {
229-
SequenceIndex::Int(i) => self.try_resizable(vm)?.elements.del_item_by_index(vm, i),
229+
SequenceIndex::Int(i) => self.try_resizable(vm)?.elements.delitem_by_index(vm, i),
230230
SequenceIndex::Slice(slice) => {
231231
// TODO: delete 0 elements don't need resizable
232-
self.try_resizable(vm)?
233-
.elements
234-
.del_item_by_slice(vm, slice)
232+
self.try_resizable(vm)?.elements.delitem_by_slice(vm, slice)
235233
}
236234
}
237235
}
@@ -813,7 +811,7 @@ impl AsSequence for PyByteArray {
813811
if let Some(value) = value {
814812
zelf._setitem_by_index(i, value, vm)
815813
} else {
816-
zelf.borrow_buf_mut().del_item_by_index(vm, i)
814+
zelf.borrow_buf_mut().delitem_by_index(vm, i)
817815
}
818816
}),
819817
contains: atomic_func!(|seq, other, vm| {

vm/src/builtins/list.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ impl PyList {
309309

310310
fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> {
311311
match SequenceIndex::try_from_borrowed_object(vm, needle, "list")? {
312-
SequenceIndex::Int(i) => self.borrow_vec_mut().del_item_by_index(vm, i),
313-
SequenceIndex::Slice(slice) => self.borrow_vec_mut().del_item_by_slice(vm, slice),
312+
SequenceIndex::Int(i) => self.borrow_vec_mut().delitem_by_index(vm, i),
313+
SequenceIndex::Slice(slice) => self.borrow_vec_mut().delitem_by_slice(vm, slice),
314314
}
315315
}
316316

@@ -444,7 +444,7 @@ impl AsSequence for PyList {
444444
if let Some(value) = value {
445445
zelf.borrow_vec_mut().setitem_by_index(vm, i, value)
446446
} else {
447-
zelf.borrow_vec_mut().del_item_by_index(vm, i)
447+
zelf.borrow_vec_mut().delitem_by_index(vm, i)
448448
}
449449
}),
450450
contains: atomic_func!(|seq, target, vm| {

vm/src/sliceable.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ where
8686
}
8787
}
8888

89-
fn del_item_by_index(&mut self, vm: &VirtualMachine, index: isize) -> PyResult<()> {
89+
fn delitem_by_index(&mut self, vm: &VirtualMachine, index: isize) -> PyResult<()> {
9090
let pos = self
9191
.as_ref()
9292
.wrap_index(index)
@@ -95,11 +95,11 @@ where
9595
Ok(())
9696
}
9797

98-
fn del_item_by_slice(&mut self, _vm: &VirtualMachine, slice: SaturatedSlice) -> PyResult<()> {
98+
fn delitem_by_slice(&mut self, _vm: &VirtualMachine, slice: SaturatedSlice) -> PyResult<()> {
9999
let (range, step, slice_len) = slice.adjust_indices(self.as_ref().len());
100100
if slice_len == 0 {
101101
Ok(())
102-
} else if step == 1 {
102+
} else if step == 1 || step == -1 {
103103
self.do_set_range(range, &[]);
104104
Ok(())
105105
} else {
@@ -192,15 +192,13 @@ pub trait SliceableSequenceOp {
192192
let sliced = if slice_len == 0 {
193193
Self::empty()
194194
} else if step == 1 {
195-
if step.is_negative() {
196-
self.do_slice_reverse(range)
197-
} else {
198-
self.do_slice(range)
199-
}
200-
} else if step.is_negative() {
201-
self.do_stepped_slice_reverse(range, step.unsigned_abs())
202-
} else {
195+
self.do_slice(range)
196+
} else if step == -1 {
197+
self.do_slice_reverse(range)
198+
} else if step.is_positive() {
203199
self.do_stepped_slice(range, step.unsigned_abs())
200+
} else {
201+
self.do_stepped_slice_reverse(range, step.unsigned_abs())
204202
};
205203
Ok(sliced)
206204
}

0 commit comments

Comments
 (0)