From 6d8c987c454686161f2ad5314b9febfab2522b43 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 30 Sep 2020 10:16:14 +0900 Subject: [PATCH 1/6] vm._len and objsequence.rs -> sliceable.rs --- vm/src/builtins.rs | 9 +- vm/src/bytesinner.rs | 6 +- vm/src/lib.rs | 1 + vm/src/obj/mod.rs | 1 - vm/src/obj/objbytearray.rs | 2 +- vm/src/obj/objbytes.rs | 2 +- vm/src/obj/objiter.rs | 5 +- vm/src/obj/objlist.rs | 6 +- vm/src/obj/objstr.rs | 2 +- vm/src/obj/objtuple.rs | 2 +- vm/src/{obj/objsequence.rs => sliceable.rs} | 126 +++++++------------- vm/src/stdlib/array.rs | 2 +- vm/src/stdlib/collections.rs | 10 +- vm/src/vm.rs | 35 +++++- 14 files changed, 101 insertions(+), 108 deletions(-) rename vm/src/{obj/objsequence.rs => sliceable.rs} (83%) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index bc48d5301..4c6c2fac6 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -28,7 +28,6 @@ mod decl { use crate::obj::objint::{self, PyIntRef}; use crate::obj::objiter; use crate::obj::objlist::{PyList, SortOptions}; - use crate::obj::objsequence; use crate::obj::objstr::{PyStr, PyStrRef}; use crate::obj::objtype::{self, PyTypeRef}; use crate::pyobject::{ @@ -37,6 +36,7 @@ mod decl { }; use crate::readline::{Readline, ReadlineResult}; use crate::scope::Scope; + use crate::sliceable; use crate::slots::PyComparisonOp; #[cfg(feature = "rustpython-parser")] use crate::stdlib::ast; @@ -444,7 +444,12 @@ mod decl { #[pyfunction] fn len(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { - objsequence::len(&obj, vm) + vm._len(&obj).unwrap_or_else(|| { + Err(vm.new_type_error(format!( + "object of type '{}' has no len()", + obj.lease_class().name + ))) + }) } #[pyfunction] diff --git a/vm/src/bytesinner.rs b/vm/src/bytesinner.rs index 7ca5843b7..d7acc2d9a 100644 --- a/vm/src/bytesinner.rs +++ b/vm/src/bytesinner.rs @@ -11,9 +11,6 @@ use crate::obj::objbytes::PyBytes; use crate::obj::objint::{self, PyInt, PyIntRef}; use crate::obj::objlist::PyList; use crate::obj::objmemory::PyMemoryView; -use crate::obj::objsequence::{ - get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex, -}; use crate::obj::objsingletons::PyNoneRef; use crate::obj::objslice::PySliceRef; use crate::obj::objstr::{self, PyStr, PyStrRef}; @@ -21,6 +18,9 @@ use crate::pyobject::{ BorrowValue, Either, PyComparisonValue, PyIterable, PyIterator, PyObjectRef, PyResult, TryFromObject, TypeProtocol, }; +use crate::sliceable::{ + get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex, +}; use crate::slots::PyComparisonOp; use crate::vm::VirtualMachine; use rustpython_common::hash; diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 4779c98f4..04e7eca40 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -65,6 +65,7 @@ mod pyobjectrc; pub mod readline; pub mod scope; mod sequence; +mod sliceable; pub mod slots; pub mod stdlib; mod sysmodule; diff --git a/vm/src/obj/mod.rs b/vm/src/obj/mod.rs index 7e6e1e629..198e792e1 100644 --- a/vm/src/obj/mod.rs +++ b/vm/src/obj/mod.rs @@ -29,7 +29,6 @@ pub mod objnamespace; pub mod objobject; pub mod objproperty; pub mod objrange; -pub mod objsequence; pub mod objset; pub mod objsingletons; pub mod objslice; diff --git a/vm/src/obj/objbytearray.rs b/vm/src/obj/objbytearray.rs index 123dc419b..acc40ebd7 100644 --- a/vm/src/obj/objbytearray.rs +++ b/vm/src/obj/objbytearray.rs @@ -5,7 +5,6 @@ use std::mem::size_of; use super::objint::PyIntRef; use super::objiter; -use super::objsequence::SequenceIndex; use super::objstr::PyStrRef; use super::objtype::PyTypeRef; use crate::anystr::{self, AnyStr}; @@ -22,6 +21,7 @@ use crate::pyobject::{ BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; +use crate::sliceable::SequenceIndex; use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable}; use crate::vm::VirtualMachine; diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index 6bcf030f7..abd47c500 100644 --- a/vm/src/obj/objbytes.rs +++ b/vm/src/obj/objbytes.rs @@ -5,7 +5,6 @@ use std::ops::Deref; use super::objint::PyIntRef; use super::objiter; -use super::objsequence::SequenceIndex; use super::objstr::PyStrRef; use super::objtype::PyTypeRef; use crate::anystr::{self, AnyStr}; @@ -20,6 +19,7 @@ use crate::pyobject::{ BorrowValue, Either, IntoPyObject, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; +use crate::sliceable::SequenceIndex; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::vm::VirtualMachine; use rustpython_common::hash::PyHash; diff --git a/vm/src/obj/objiter.rs b/vm/src/obj/objiter.rs index 066c2e24b..2e7e10fad 100644 --- a/vm/src/obj/objiter.rs +++ b/vm/src/obj/objiter.rs @@ -6,7 +6,6 @@ use crossbeam_utils::atomic::AtomicCell; use num_traits::Signed; use super::objint::{self, PyInt}; -use super::objsequence; use super::objtype::{self, PyTypeRef}; use crate::exceptions::PyBaseExceptionRef; use crate::pyobject::{ @@ -103,7 +102,7 @@ pub fn stop_iter_value(vm: &VirtualMachine, exc: &PyBaseExceptionRef) -> PyResul } pub fn length_hint(vm: &VirtualMachine, iter: PyObjectRef) -> PyResult> { - if let Some(len) = objsequence::opt_len(&iter, vm) { + if let Some(len) = vm._len(&iter) { match len { Ok(len) => return Ok(Some(len)), Err(e) => { @@ -203,7 +202,7 @@ impl PySequenceIterator { let hint = if self.reversed { pos + 1 } else { - let len = objsequence::opt_len(&self.obj, vm).unwrap_or_else(|| { + let len = vm._len(&self.obj).unwrap_or_else(|| { Err(vm.new_type_error("sequence has no __len__ method".to_owned())) })?; len as isize - pos diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index ee5fb86c6..86188bc7c 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -8,9 +8,6 @@ use num_traits::ToPrimitive; use super::objint::PyIntRef; use super::objiter; -use super::objsequence::{ - get_item, get_pos, get_saturated_pos, PySliceableSequenceMut, SequenceIndex, -}; use super::objslice::PySliceRef; use super::objtype::PyTypeRef; use crate::bytesinner; @@ -21,6 +18,9 @@ use crate::pyobject::{ PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; +use crate::sliceable::{ + get_item, get_pos, get_saturated_pos, PySliceableSequenceMut, SequenceIndex, +}; use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable}; use crate::vm::{ReprGuard, VirtualMachine}; diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 316b9846c..caeaba197 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -16,7 +16,6 @@ use super::objbytes::{PyBytes, PyBytesRef}; use super::objdict::PyDict; use super::objint::{PyInt, PyIntRef}; use super::objiter; -use super::objsequence::{PySliceableSequence, SequenceIndex}; use super::objtype::{self, PyTypeRef}; use crate::anystr::{self, adjust_indices, AnyStr, AnyStrContainer, AnyStrWrapper}; use crate::exceptions::IntoPyException; @@ -26,6 +25,7 @@ use crate::pyobject::{ BorrowValue, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef, TypeProtocol, }; +use crate::sliceable::{PySliceableSequence, SequenceIndex}; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::VirtualMachine; use rustpython_common::hash; diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 0a2c18c13..6c39cb3e7 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -2,7 +2,6 @@ use crossbeam_utils::atomic::AtomicCell; use std::fmt; use super::objiter; -use super::objsequence::get_item; use super::objtype::PyTypeRef; use crate::function::OptionalArg; use crate::pyobject::{ @@ -10,6 +9,7 @@ use crate::pyobject::{ PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; +use crate::sliceable::get_item; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::vm::{ReprGuard, VirtualMachine}; use rustpython_common::hash::PyHash; diff --git a/vm/src/obj/objsequence.rs b/vm/src/sliceable.rs similarity index 83% rename from vm/src/obj/objsequence.rs rename to vm/src/sliceable.rs index 9ef4d2c0f..a58ef0479 100644 --- a/vm/src/obj/objsequence.rs +++ b/vm/src/sliceable.rs @@ -1,12 +1,9 @@ -use std::ops::Range; - use num_bigint::BigInt; use num_traits::{One, Signed, ToPrimitive, Zero}; +use std::ops::Range; -use super::objint::{PyInt, PyIntRef}; -use super::objsingletons::PyNone; -use super::objslice::{PySlice, PySliceRef}; -use crate::function::OptionalArg; +use crate::obj::objint::PyInt; +use crate::obj::objslice::{PySlice, PySliceRef}; use crate::pyobject::{BorrowValue, Either, PyObjectRef, PyResult, TryFromObject, TypeProtocol}; use crate::vm::VirtualMachine; @@ -383,28 +380,28 @@ impl TryFromObject for SequenceIndex { /// Get the index into a sequence like type. Get it from a python integer /// object, accounting for negative index, and out of bounds issues. -pub fn get_sequence_index(vm: &VirtualMachine, index: &PyIntRef, length: usize) -> PyResult { - if let Some(value) = index.borrow_value().to_i64() { - if value < 0 { - let from_end: usize = -value as usize; - if from_end > length { - Err(vm.new_index_error("Index out of bounds!".to_owned())) - } else { - let index = length - from_end; - Ok(index) - } - } else { - let index = value as usize; - if index >= length { - Err(vm.new_index_error("Index out of bounds!".to_owned())) - } else { - Ok(index) - } - } - } else { - Err(vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned())) - } -} +// pub fn get_sequence_index(vm: &VirtualMachine, index: &PyIntRef, length: usize) -> PyResult { +// if let Some(value) = index.borrow_value().to_i64() { +// if value < 0 { +// let from_end: usize = -value as usize; +// if from_end > length { +// Err(vm.new_index_error("Index out of bounds!".to_owned())) +// } else { +// let index = length - from_end; +// Ok(index) +// } +// } else { +// let index = value as usize; +// if index >= length { +// Err(vm.new_index_error("Index out of bounds!".to_owned())) +// } else { +// Ok(index) +// } +// } +// } else { +// Err(vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned())) +// } +// } pub fn get_pos(p: isize, len: usize) -> Option { let neg = p.is_negative(); @@ -451,12 +448,12 @@ pub fn get_slice_pos(slice_pos: &BigInt, len: usize) -> usize { } } -pub fn get_slice_range(start: &Option, stop: &Option, len: usize) -> Range { - let start = start.as_ref().map_or(0, |x| get_slice_pos(x, len)); - let stop = stop.as_ref().map_or(len, |x| get_slice_pos(x, len)); +// pub fn get_slice_range(start: &Option, stop: &Option, len: usize) -> Range { +// let start = start.as_ref().map_or(0, |x| get_slice_pos(x, len)); +// let stop = stop.as_ref().map_or(len, |x| get_slice_pos(x, len)); - start..stop -} +// start..stop +// } pub fn get_item( vm: &VirtualMachine, @@ -483,50 +480,19 @@ pub fn get_item( } //Check if given arg could be used with PySliceableSequence.get_slice_range() -pub fn is_valid_slice_arg( - arg: OptionalArg, - vm: &VirtualMachine, -) -> PyResult> { - if let OptionalArg::Present(value) = arg { - match_class!(match value { - i @ PyInt => Ok(Some(i.borrow_value().clone())), - _obj @ PyNone => Ok(None), - _ => Err(vm.new_type_error( - "slice indices must be integers or None or have an __index__ method".to_owned() - )), // TODO: check for an __index__ method - }) - } else { - Ok(None) - } -} - -pub fn opt_len(obj: &PyObjectRef, vm: &VirtualMachine) -> Option> { - vm.get_method(obj.clone(), "__len__").map(|len| { - let len = vm.invoke(&len?, vec![])?; - let len = len - .payload_if_subclass::(vm) - .ok_or_else(|| { - vm.new_type_error(format!( - "'{}' object cannot be interpreted as an integer", - len.lease_class().name - )) - })? - .borrow_value(); - if len.is_negative() { - return Err(vm.new_value_error("__len__() should return >= 0".to_owned())); - } - let len = len.to_isize().ok_or_else(|| { - vm.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned()) - })?; - Ok(len as usize) - }) -} - -pub fn len(obj: &PyObjectRef, vm: &VirtualMachine) -> PyResult { - opt_len(obj, vm).unwrap_or_else(|| { - Err(vm.new_type_error(format!( - "object of type '{}' has no len()", - obj.lease_class().name - ))) - }) -} +// pub fn is_valid_slice_arg( +// arg: OptionalArg, +// vm: &VirtualMachine, +// ) -> PyResult> { +// if let OptionalArg::Present(value) = arg { +// match_class!(match value { +// i @ PyInt => Ok(Some(i.borrow_value().clone())), +// _obj @ PyNone => Ok(None), +// _ => Err(vm.new_type_error( +// "slice indices must be integers or None or have an __index__ method".to_owned() +// )), // TODO: check for an __index__ method +// }) +// } else { +// Ok(None) +// } +// } diff --git a/vm/src/stdlib/array.rs b/vm/src/stdlib/array.rs index d3d36d198..50cdc345f 100644 --- a/vm/src/stdlib/array.rs +++ b/vm/src/stdlib/array.rs @@ -7,7 +7,6 @@ use crate::obj::objbytes::PyBytesRef; use crate::obj::objfloat::try_float; use crate::obj::objiter; use crate::obj::objlist::PyList; -use crate::obj::objsequence::{get_saturated_pos, PySliceableSequence, PySliceableSequenceMut}; use crate::obj::objslice::PySliceRef; use crate::obj::objstr::PyStrRef; use crate::obj::objtype::PyTypeRef; @@ -15,6 +14,7 @@ use crate::pyobject::{ BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; +use crate::sliceable::{get_saturated_pos, PySliceableSequence, PySliceableSequenceMut}; use crate::slots::{Comparable, PyComparisonOp}; use crate::VirtualMachine; use crossbeam_utils::atomic::AtomicCell; diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 9cfca2c55..6b19f0bc8 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -4,12 +4,12 @@ pub(crate) use _collections::make_module; mod _collections { use crate::common::lock::{PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard}; use crate::function::OptionalArg; - use crate::obj::{objiter, objsequence, objtype::PyTypeRef}; + use crate::obj::{objiter, objtype::PyTypeRef}; use crate::pyobject::{PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult, PyValue}; - use crate::sequence; use crate::slots::{Comparable, PyComparisonOp}; use crate::vm::ReprGuard; use crate::VirtualMachine; + use crate::{sequence, sliceable}; use itertools::Itertools; use std::collections::VecDeque; @@ -248,7 +248,7 @@ mod _collections { #[pymethod(magic)] fn getitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult { let deque = self.borrow_deque(); - objsequence::get_pos(idx, deque.len()) + sliceable::get_pos(idx, deque.len()) .and_then(|i| deque.get(i).cloned()) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) } @@ -256,7 +256,7 @@ mod _collections { #[pymethod(magic)] fn setitem(&self, idx: isize, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.borrow_deque_mut(); - objsequence::get_pos(idx, deque.len()) + sliceable::get_pos(idx, deque.len()) .and_then(|i| deque.get_mut(i)) .map(|x| *x = value) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) @@ -265,7 +265,7 @@ mod _collections { #[pymethod(magic)] fn delitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.borrow_deque_mut(); - objsequence::get_pos(idx, deque.len()) + sliceable::get_pos(idx, deque.len()) .and_then(|i| deque.remove(i).map(drop)) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index dc6289548..f60a8c866 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -11,11 +11,7 @@ use std::fmt; use arr_macro::arr; use crossbeam_utils::atomic::AtomicCell; -#[cfg(feature = "rustpython-compiler")] -use rustpython_compiler::{ - compile::{self, CompileOpts}, - error::CompileError, -}; +use num_traits::{Signed, ToPrimitive}; use crate::builtins::{self, to_ascii}; use crate::bytecode; @@ -28,7 +24,7 @@ use crate::import; use crate::obj::objbool; use crate::obj::objcode::{PyCode, PyCodeRef}; use crate::obj::objdict::PyDictRef; -use crate::obj::objint::PyIntRef; +use crate::obj::objint::{PyInt, PyIntRef}; use crate::obj::objiter; use crate::obj::objlist::PyList; use crate::obj::objmodule::{self, PyModule}; @@ -44,6 +40,11 @@ use crate::scope::Scope; use crate::slots::PyComparisonOp; use crate::stdlib; use crate::sysmodule; +#[cfg(feature = "rustpython-compiler")] +use rustpython_compiler::{ + compile::{self, CompileOpts}, + error::CompileError, +}; // use objects::objects; @@ -1494,6 +1495,28 @@ impl VirtualMachine { hash(&obj, self) } + pub fn _len(&self, obj: &PyObjectRef) -> Option> { + self.get_method(obj.clone(), "__len__").map(|len| { + let len = self.invoke(&len?, vec![])?; + let len = len + .payload_if_subclass::(self) + .ok_or_else(|| { + self.new_type_error(format!( + "'{}' object cannot be interpreted as an integer", + len.lease_class().name + )) + })? + .borrow_value(); + if len.is_negative() { + return Err(self.new_value_error("__len__() should return >= 0".to_owned())); + } + let len = len.to_isize().ok_or_else(|| { + self.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned()) + })?; + Ok(len as usize) + }) + } + // https://docs.python.org/3/reference/expressions.html#membership-test-operations fn _membership_iter_search(&self, haystack: PyObjectRef, needle: PyObjectRef) -> PyResult { let iter = objiter::get_iter(self, &haystack)?; From c3ead5137b12f99f521873f9753ec62f677c28ea Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 30 Sep 2020 10:32:41 +0900 Subject: [PATCH 2/6] objcoroinner.rs -> coroutine.rs --- vm/src/{obj/objcoroinner.rs => coroutine.rs} | 2 +- vm/src/frame.rs | 2 +- vm/src/lib.rs | 1 + vm/src/obj/mod.rs | 1 - vm/src/obj/objasyncgenerator.rs | 2 +- vm/src/obj/objcoroutine.rs | 2 +- vm/src/obj/objgenerator.rs | 2 +- 7 files changed, 6 insertions(+), 6 deletions(-) rename vm/src/{obj/objcoroinner.rs => coroutine.rs} (99%) diff --git a/vm/src/obj/objcoroinner.rs b/vm/src/coroutine.rs similarity index 99% rename from vm/src/obj/objcoroinner.rs rename to vm/src/coroutine.rs index 12444697c..00f11f265 100644 --- a/vm/src/obj/objcoroinner.rs +++ b/vm/src/coroutine.rs @@ -1,6 +1,6 @@ -use super::objtype::{self, PyTypeRef}; use crate::exceptions::{self, PyBaseExceptionRef}; use crate::frame::{ExecutionResult, FrameRef}; +use crate::obj::objtype::{self, PyTypeRef}; use crate::pyobject::{PyObjectRef, PyResult}; use crate::vm::VirtualMachine; diff --git a/vm/src/frame.rs b/vm/src/frame.rs index c17adcd95..cafc092b6 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -7,11 +7,11 @@ use itertools::Itertools; use crate::builtins::builtin_isinstance; use crate::bytecode; use crate::common::lock::PyMutex; +use crate::coroutine::Coro; use crate::exceptions::{self, ExceptionCtor, PyBaseExceptionRef}; use crate::function::PyFuncArgs; use crate::obj::objasyncgenerator::PyAsyncGenWrappedValue; use crate::obj::objcode::PyCodeRef; -use crate::obj::objcoroinner::Coro; use crate::obj::objcoroutine::PyCoroutine; use crate::obj::objdict::{PyDict, PyDictRef}; use crate::obj::objgenerator::PyGenerator; diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 04e7eca40..deea3f8fd 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -48,6 +48,7 @@ mod builtins; mod bytesinner; pub mod byteslike; pub mod cformat; +mod coroutine; mod dictdatatype; #[cfg(feature = "rustpython-compiler")] pub mod eval; diff --git a/vm/src/obj/mod.rs b/vm/src/obj/mod.rs index 198e792e1..a5bab4445 100644 --- a/vm/src/obj/mod.rs +++ b/vm/src/obj/mod.rs @@ -8,7 +8,6 @@ pub mod objbytes; pub mod objclassmethod; pub mod objcode; pub mod objcomplex; -pub mod objcoroinner; pub mod objcoroutine; pub mod objdict; pub mod objenumerate; diff --git a/vm/src/obj/objasyncgenerator.rs b/vm/src/obj/objasyncgenerator.rs index dc425794d..8c4a1798b 100644 --- a/vm/src/obj/objasyncgenerator.rs +++ b/vm/src/obj/objasyncgenerator.rs @@ -1,6 +1,6 @@ use super::objcode::PyCodeRef; -use super::objcoroinner::{Coro, Variant}; use super::objtype::{self, PyTypeRef}; +use crate::coroutine::{Coro, Variant}; use crate::exceptions::PyBaseExceptionRef; use crate::frame::FrameRef; use crate::function::OptionalArg; diff --git a/vm/src/obj/objcoroutine.rs b/vm/src/obj/objcoroutine.rs index c1b32aacf..2e4ab03ff 100644 --- a/vm/src/obj/objcoroutine.rs +++ b/vm/src/obj/objcoroutine.rs @@ -1,7 +1,7 @@ use super::objcode::PyCodeRef; -use super::objcoroinner::{Coro, Variant}; use super::objstr::PyStrRef; use super::objtype::PyTypeRef; +use crate::coroutine::{Coro, Variant}; use crate::frame::FrameRef; use crate::function::OptionalArg; use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; diff --git a/vm/src/obj/objgenerator.rs b/vm/src/obj/objgenerator.rs index 5cbe0aea5..b7138e94d 100644 --- a/vm/src/obj/objgenerator.rs +++ b/vm/src/obj/objgenerator.rs @@ -3,8 +3,8 @@ */ use super::objcode::PyCodeRef; -use super::objcoroinner::{Coro, Variant}; use super::objtype::PyTypeRef; +use crate::coroutine::{Coro, Variant}; use crate::frame::FrameRef; use crate::function::OptionalArg; use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; From 6fda0d355e3bd4eeb4592aff9f1e74ff44477598 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 1 Oct 2020 15:55:01 +0900 Subject: [PATCH 3/6] get_pos --- vm/src/obj/objlist.rs | 6 +++--- vm/src/sliceable.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 86188bc7c..28ce33a19 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -19,7 +19,7 @@ use crate::pyobject::{ }; use crate::sequence::{self, SimpleSeq}; use crate::sliceable::{ - get_item, get_pos, get_saturated_pos, PySliceableSequenceMut, SequenceIndex, + get_item, get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex, }; use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable}; use crate::vm::{ReprGuard, VirtualMachine}; @@ -228,7 +228,7 @@ impl PyList { fn setindex(&self, index: isize, mut value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut elements = self.borrow_value_mut(); - if let Some(pos_index) = get_pos(index, elements.len()) { + if let Some(pos_index) = elements.get_pos(index) { std::mem::swap(&mut elements[pos_index], &mut value); Ok(()) } else { @@ -361,7 +361,7 @@ impl PyList { fn delindex(&self, index: isize, vm: &VirtualMachine) -> PyResult<()> { let removed = { let mut elements = self.borrow_value_mut(); - if let Some(pos_index) = get_pos(index, elements.len()) { + if let Some(pos_index) = elements.get_pos(index) { // defer delete out of borrow Ok(elements.remove(pos_index)) } else { diff --git a/vm/src/sliceable.rs b/vm/src/sliceable.rs index a58ef0479..c0087b7e2 100644 --- a/vm/src/sliceable.rs +++ b/vm/src/sliceable.rs @@ -465,7 +465,8 @@ pub fn get_item( let value = i.borrow_value().to_isize().ok_or_else(|| { vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned()) })?; - let pos_index = get_pos(value, elements.len()) + let pos_index = elements + .get_pos(value) .ok_or_else(|| vm.new_index_error("Index out of bounds!".to_owned()))?; Ok(Either::A(elements[pos_index].clone())) } else { From 699c801354de2b13b6fdbe747014d2a158d41fd6 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 1 Oct 2020 17:26:17 +0900 Subject: [PATCH 4/6] Fix sequence index utilities not to use get_ prefix --- vm/src/bytesinner.rs | 14 +++---- vm/src/obj/objlist.rs | 10 ++--- vm/src/sliceable.rs | 74 ++++++++++++++++++------------------ vm/src/stdlib/array.rs | 4 +- vm/src/stdlib/collections.rs | 6 +-- 5 files changed, 52 insertions(+), 56 deletions(-) diff --git a/vm/src/bytesinner.rs b/vm/src/bytesinner.rs index d7acc2d9a..79727da97 100644 --- a/vm/src/bytesinner.rs +++ b/vm/src/bytesinner.rs @@ -18,9 +18,7 @@ use crate::pyobject::{ BorrowValue, Either, PyComparisonValue, PyIterable, PyIterator, PyObjectRef, PyResult, TryFromObject, TypeProtocol, }; -use crate::sliceable::{ - get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex, -}; +use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex}; use crate::slots::PyComparisonOp; use crate::vm::VirtualMachine; use rustpython_common::hash; @@ -300,7 +298,7 @@ impl PyBytesInner { pub fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult { match needle { SequenceIndex::Int(int) => { - if let Some(idx) = self.elements.get_pos(int) { + if let Some(idx) = self.elements.wrap_index(int) { Ok(vm.ctx.new_int(self.elements[idx])) } else { Err(vm.new_index_error("index out of range".to_owned())) @@ -318,7 +316,7 @@ impl PyBytesInner { object: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - if let Some(idx) = self.elements.get_pos(int) { + if let Some(idx) = self.elements.wrap_index(int) { let value = Self::value_try_from_object(vm, object)?; self.elements[idx] = value; Ok(()) @@ -374,7 +372,7 @@ impl PyBytesInner { pub fn delitem(&mut self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult<()> { match needle { SequenceIndex::Int(int) => { - if let Some(idx) = self.elements.get_pos(int) { + if let Some(idx) = self.elements.wrap_index(int) { self.elements.remove(idx); Ok(()) } else { @@ -390,7 +388,7 @@ impl PyBytesInner { } pub fn pop(&mut self, index: isize, vm: &VirtualMachine) -> PyResult { - if let Some(index) = self.elements.get_pos(index) { + if let Some(index) = self.elements.wrap_index(index) { Ok(self.elements.remove(index)) } else { Err(vm.new_index_error("index out of range".to_owned())) @@ -404,7 +402,7 @@ impl PyBytesInner { vm: &VirtualMachine, ) -> PyResult<()> { let value = Self::value_try_from_object(vm, object)?; - let index = get_saturated_pos(index, self.len()); + let index = self.elements.saturate_index(index); self.elements.insert(index, value); Ok(()) } diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index 28ce33a19..eb2d14774 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -18,9 +18,7 @@ use crate::pyobject::{ PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; -use crate::sliceable::{ - get_item, get_saturated_pos, PySliceableSequence, PySliceableSequenceMut, SequenceIndex, -}; +use crate::sliceable::{get_item, PySliceableSequence, PySliceableSequenceMut, SequenceIndex}; use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable}; use crate::vm::{ReprGuard, VirtualMachine}; @@ -120,7 +118,7 @@ impl PyList { #[pymethod] pub(crate) fn insert(&self, position: isize, element: PyObjectRef) { let mut elements = self.borrow_value_mut(); - let position = get_saturated_pos(position, elements.len()); + let position = elements.saturate_index(position); elements.insert(position, element); } @@ -228,7 +226,7 @@ impl PyList { fn setindex(&self, index: isize, mut value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut elements = self.borrow_value_mut(); - if let Some(pos_index) = elements.get_pos(index) { + if let Some(pos_index) = elements.wrap_index(index) { std::mem::swap(&mut elements[pos_index], &mut value); Ok(()) } else { @@ -361,7 +359,7 @@ impl PyList { fn delindex(&self, index: isize, vm: &VirtualMachine) -> PyResult<()> { let removed = { let mut elements = self.borrow_value_mut(); - if let Some(pos_index) = elements.get_pos(index) { + if let Some(pos_index) = elements.wrap_index(index) { // defer delete out of borrow Ok(elements.remove(pos_index)) } else { diff --git a/vm/src/sliceable.rs b/vm/src/sliceable.rs index c0087b7e2..27ca6c87e 100644 --- a/vm/src/sliceable.rs +++ b/vm/src/sliceable.rs @@ -34,7 +34,7 @@ pub trait PySliceableSequenceMut { return Err(vm.new_value_error("slice step cannot be zero".to_owned())); } if step == BigInt::one() { - let range = self.as_slice().get_slice_range(&start, &stop); + let range = self.as_slice().saturate_range(&start, &stop); let range = if range.end < range.start { range.start..range.start } else { @@ -67,7 +67,7 @@ pub trait PySliceableSequenceMut { (start, stop, step, false) }; - let range = self.as_slice().get_slice_range(&start, &stop); + let range = self.as_slice().saturate_range(&start, &stop); let range = if range.end < range.start { range.start..range.start } else { @@ -137,7 +137,7 @@ pub trait PySliceableSequenceMut { } if step == BigInt::one() { - let range = self.as_slice().get_slice_range(&start, &stop); + let range = self.as_slice().saturate_range(&start, &stop); if range.start < range.end { self.do_delete_range(range); } @@ -167,7 +167,7 @@ pub trait PySliceableSequenceMut { (start, stop, step, false) }; - let range = self.as_slice().get_slice_range(&start, &stop); + let range = self.as_slice().saturate_range(&start, &stop); if range.start >= range.end { return Ok(()); } @@ -251,19 +251,35 @@ pub trait PySliceableSequence { fn len(&self) -> usize; fn is_empty(&self) -> bool; - fn get_pos(&self, p: isize) -> Option { - get_pos(p, self.len()) + fn wrap_index(&self, p: isize) -> Option { + wrap_index(p, self.len()) } - fn get_slice_pos(&self, slice_pos: &BigInt) -> usize { - get_slice_pos(slice_pos, self.len()) + fn saturate_index(&self, p: isize) -> usize { + saturate_index(p, self.len()) } - fn get_slice_range(&self, start: &Option, stop: &Option) -> Range { - let start = start.as_ref().map(|x| self.get_slice_pos(x)).unwrap_or(0); + fn saturate_big_index(&self, slice_pos: &BigInt) -> usize { + let len = self.len(); + if let Some(pos) = slice_pos.to_isize() { + self.saturate_index(pos) + } else if slice_pos.is_negative() { + // slice past start bound, round to start + 0 + } else { + // slice past end bound, round to end + len + } + } + + fn saturate_range(&self, start: &Option, stop: &Option) -> Range { + let start = start + .as_ref() + .map(|x| self.saturate_big_index(x)) + .unwrap_or(0); let stop = stop .as_ref() - .map(|x| self.get_slice_pos(x)) + .map(|x| self.saturate_big_index(x)) .unwrap_or_else(|| self.len()); start..stop @@ -276,7 +292,7 @@ pub trait PySliceableSequence { if step.is_zero() { Err(vm.new_value_error("slice step cannot be zero".to_owned())) } else if step.is_positive() { - let range = self.get_slice_range(&start, &stop); + let range = self.saturate_range(&start, &stop); if range.start < range.end { #[allow(clippy::range_plus_one)] match step.to_i32() { @@ -304,7 +320,7 @@ pub trait PySliceableSequence { x + 1 } }); - let range = self.get_slice_range(&stop, &start); + let range = self.saturate_range(&stop, &start); if range.start < range.end { match (-step).to_i32() { Some(1) => Ok(self.do_slice_reverse(range)), @@ -403,7 +419,8 @@ impl TryFromObject for SequenceIndex { // } // } -pub fn get_pos(p: isize, len: usize) -> Option { +// Use PySliceableSequence::wrap_index for implementors +pub(crate) fn wrap_index(p: isize, len: usize) -> Option { let neg = p.is_negative(); let p = p.abs().to_usize()?; if neg { @@ -416,7 +433,7 @@ pub fn get_pos(p: isize, len: usize) -> Option { } // return pos is in range [0, len] inclusive -pub fn get_saturated_pos(p: isize, len: usize) -> usize { +pub(crate) fn saturate_index(p: isize, len: usize) -> usize { let mut p = p; let len = len.to_isize().unwrap(); if p < 0 { @@ -431,26 +448,9 @@ pub fn get_saturated_pos(p: isize, len: usize) -> usize { p as usize } -pub fn get_slice_pos(slice_pos: &BigInt, len: usize) -> usize { - if let Some(pos) = slice_pos.to_isize() { - if let Some(index) = get_pos(pos, len) { - // within bounds - return index; - } - } - - if slice_pos.is_negative() { - // slice past start bound, round to start - 0 - } else { - // slice past end bound, round to end - len - } -} - -// pub fn get_slice_range(start: &Option, stop: &Option, len: usize) -> Range { -// let start = start.as_ref().map_or(0, |x| get_slice_pos(x, len)); -// let stop = stop.as_ref().map_or(len, |x| get_slice_pos(x, len)); +// pub fn saturate_range(start: &Option, stop: &Option, len: usize) -> Range { +// let start = start.as_ref().map_or(0, |x| saturate_big_index(x, len)); +// let stop = stop.as_ref().map_or(len, |x| saturate_big_index(x, len)); // start..stop // } @@ -466,7 +466,7 @@ pub fn get_item( vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned()) })?; let pos_index = elements - .get_pos(value) + .wrap_index(value) .ok_or_else(|| vm.new_index_error("Index out of bounds!".to_owned()))?; Ok(Either::A(elements[pos_index].clone())) } else { @@ -480,7 +480,7 @@ pub fn get_item( } } -//Check if given arg could be used with PySliceableSequence.get_slice_range() +//Check if given arg could be used with PySliceableSequence.saturate_range() // pub fn is_valid_slice_arg( // arg: OptionalArg, // vm: &VirtualMachine, diff --git a/vm/src/stdlib/array.rs b/vm/src/stdlib/array.rs index 50cdc345f..9811e1704 100644 --- a/vm/src/stdlib/array.rs +++ b/vm/src/stdlib/array.rs @@ -14,7 +14,7 @@ use crate::pyobject::{ BorrowValue, Either, IdProtocol, IntoPyObject, PyClassImpl, PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; -use crate::sliceable::{get_saturated_pos, PySliceableSequence, PySliceableSequenceMut}; +use crate::sliceable::{saturate_index, PySliceableSequence, PySliceableSequenceMut}; use crate::slots::{Comparable, PyComparisonOp}; use crate::VirtualMachine; use crossbeam_utils::atomic::AtomicCell; @@ -581,7 +581,7 @@ impl PyArray { #[pymethod] fn insert(&self, i: isize, x: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let i = get_saturated_pos(i, self.len()); + let i = saturate_index(i, self.len()); self.borrow_value_mut().insert(i, x, vm) } diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index 6b19f0bc8..f75aafa2c 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -248,7 +248,7 @@ mod _collections { #[pymethod(magic)] fn getitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult { let deque = self.borrow_deque(); - sliceable::get_pos(idx, deque.len()) + sliceable::wrap_index(idx, deque.len()) .and_then(|i| deque.get(i).cloned()) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) } @@ -256,7 +256,7 @@ mod _collections { #[pymethod(magic)] fn setitem(&self, idx: isize, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.borrow_deque_mut(); - sliceable::get_pos(idx, deque.len()) + sliceable::wrap_index(idx, deque.len()) .and_then(|i| deque.get_mut(i)) .map(|x| *x = value) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) @@ -265,7 +265,7 @@ mod _collections { #[pymethod(magic)] fn delitem(&self, idx: isize, vm: &VirtualMachine) -> PyResult<()> { let mut deque = self.borrow_deque_mut(); - sliceable::get_pos(idx, deque.len()) + sliceable::wrap_index(idx, deque.len()) .and_then(|i| deque.remove(i).map(drop)) .ok_or_else(|| vm.new_index_error("deque index out of range".to_owned())) } From dd5f9e384d97dd84747e8f3010e5995e9798573f Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 1 Oct 2020 18:23:23 +0900 Subject: [PATCH 5/6] Refactor get_item for sequences --- vm/src/obj/objlist.rs | 13 ++++---- vm/src/obj/objstr.rs | 38 +++++++++------------- vm/src/obj/objtuple.rs | 13 ++++---- vm/src/sliceable.rs | 74 +++++++++++++++++++++++++----------------- 4 files changed, 72 insertions(+), 66 deletions(-) diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs index eb2d14774..bce11b407 100644 --- a/vm/src/obj/objlist.rs +++ b/vm/src/obj/objlist.rs @@ -18,7 +18,7 @@ use crate::pyobject::{ PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; -use crate::sliceable::{get_item, PySliceableSequence, PySliceableSequenceMut, SequenceIndex}; +use crate::sliceable::{PySliceableSequence, PySliceableSequenceMut, SequenceIndex}; use crate::slots::{Comparable, Hashable, PyComparisonOp, Unhashable}; use crate::vm::{ReprGuard, VirtualMachine}; @@ -190,12 +190,11 @@ impl PyList { #[pymethod(name = "__getitem__")] fn getitem(zelf: PyRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Ok( - match get_item(vm, zelf.as_object(), &zelf.borrow_value(), needle)? { - Either::A(obj) => obj, - Either::B(vec) => vm.ctx.new_list(vec), - }, - ) + let result = match zelf.borrow_value().get_item(vm, needle, "list")? { + Either::A(obj) => obj, + Either::B(vec) => vm.ctx.new_list(vec), + }; + Ok(result) } #[pymethod(name = "__iter__")] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index caeaba197..e1ae4a3c2 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -22,10 +22,11 @@ use crate::exceptions::IntoPyException; use crate::format::{FormatSpec, FormatString, FromTemplate}; use crate::function::{OptionalArg, OptionalOption, PyFuncArgs}; use crate::pyobject::{ - BorrowValue, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyComparisonValue, PyContext, - PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef, TypeProtocol, + BorrowValue, Either, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyComparisonValue, + PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef, + TypeProtocol, }; -use crate::sliceable::{PySliceableSequence, SequenceIndex}; +use crate::sliceable::PySliceableSequence; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::VirtualMachine; use rustpython_common::hash; @@ -228,26 +229,12 @@ impl PyStr { } #[pymethod(name = "__getitem__")] - fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult { - match needle { - SequenceIndex::Int(pos) => { - let index: usize = if pos.is_negative() { - (self.value.chars().count() as isize + pos) as usize - } else { - pos.abs() as usize - }; - - if let Some(character) = self.value.chars().nth(index) { - Ok(vm.ctx.new_str(character.to_string())) - } else { - Err(vm.new_index_error("string index out of range".to_owned())) - } - } - SequenceIndex::Slice(slice) => { - let string = self.get_slice_items(vm, &slice)?; - Ok(vm.ctx.new_str(string)) - } - } + fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let s = match self.get_item(vm, needle, "string")? { + Either::A(ch) => ch.to_string(), + Either::B(s) => s, + }; + Ok(vm.ctx.new_str(s)) } pub(crate) fn hash(&self, vm: &VirtualMachine) -> hash::PyHash { @@ -1172,8 +1159,13 @@ pub fn borrow_value(obj: &PyObjectRef) -> &str { } impl PySliceableSequence for PyStr { + type Item = char; type Sliced = String; + fn do_get(&self, index: usize) -> Self::Item { + self.value.chars().nth(index).unwrap() + } + fn do_slice(&self, range: Range) -> Self::Sliced { self.value .chars() diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 6c39cb3e7..a1e8e35b6 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -9,7 +9,7 @@ use crate::pyobject::{ PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; -use crate::sliceable::get_item; +use crate::sliceable::PySliceableSequence; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::vm::{ReprGuard, VirtualMachine}; use rustpython_common::hash::PyHash; @@ -184,12 +184,11 @@ impl PyTuple { #[pymethod(name = "__getitem__")] fn getitem(zelf: PyRef, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - Ok( - match get_item(vm, zelf.as_object(), &zelf.elements, needle)? { - Either::A(obj) => obj, - Either::B(vec) => vm.ctx.new_tuple(vec), - }, - ) + let result = match zelf.elements.as_ref().get_item(vm, needle, "tuple")? { + Either::A(obj) => obj, + Either::B(vec) => vm.ctx.new_tuple(vec), + }; + Ok(result) } #[pymethod(name = "index")] diff --git a/vm/src/sliceable.rs b/vm/src/sliceable.rs index 27ca6c87e..db8074603 100644 --- a/vm/src/sliceable.rs +++ b/vm/src/sliceable.rs @@ -240,8 +240,10 @@ impl PySliceableSequenceMut for Vec { } pub trait PySliceableSequence { + type Item; type Sliced; + fn do_get(&self, index: usize) -> Self::Item; fn do_slice(&self, range: Range) -> Self::Sliced; fn do_slice_reverse(&self, range: Range) -> Self::Sliced; fn do_stepped_slice(&self, range: Range, step: usize) -> Self::Sliced; @@ -332,25 +334,53 @@ pub trait PySliceableSequence { } } } + + fn get_item( + &self, + vm: &VirtualMachine, + needle: PyObjectRef, + owner_type: &'static str, + ) -> PyResult> { + let needle = SequenceIndex::try_from_object_for(vm, needle, owner_type)?; + match needle { + SequenceIndex::Int(value) => { + let pos_index = self.wrap_index(value).ok_or_else(|| { + vm.new_index_error(format!("{} index out of range", owner_type)) + })?; + Ok(Either::A(self.do_get(pos_index))) + } + SequenceIndex::Slice(slice) => Ok(Either::B(self.get_slice_items(vm, &slice)?)), + } + } } impl PySliceableSequence for [T] { + type Item = T; type Sliced = Vec; + #[inline] + fn do_get(&self, index: usize) -> Self::Item { + self[index].clone() + } + + #[inline] fn do_slice(&self, range: Range) -> Self::Sliced { self[range].to_vec() } + #[inline] fn do_slice_reverse(&self, range: Range) -> Self::Sliced { let mut slice = self[range].to_vec(); slice.reverse(); slice } + #[inline] fn do_stepped_slice(&self, range: Range, step: usize) -> Self::Sliced { self[range].iter().step_by(step).cloned().collect() } + #[inline] fn do_stepped_slice_reverse(&self, range: Range, step: usize) -> Self::Sliced { self[range].iter().rev().step_by(step).cloned().collect() } @@ -376,8 +406,12 @@ pub enum SequenceIndex { Slice(PySliceRef), } -impl TryFromObject for SequenceIndex { - fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { +impl SequenceIndex { + fn try_from_object_for( + vm: &VirtualMachine, + obj: PyObjectRef, + owner_type: &'static str, + ) -> PyResult { match_class!(match obj { i @ PyInt => i .borrow_value() @@ -387,13 +421,20 @@ impl TryFromObject for SequenceIndex { .new_index_error("cannot fit 'int' into an index-sized integer".to_owned())), s @ PySlice => Ok(SequenceIndex::Slice(s)), obj => Err(vm.new_type_error(format!( - "sequence indices be integers or slices, not {}", - obj.class(), + "{} indices must be integers or slices, not {}", + owner_type, + obj.lease_class().name, ))), }) } } +impl TryFromObject for SequenceIndex { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + Self::try_from_object_for(vm, obj, "sequence") + } +} + /// Get the index into a sequence like type. Get it from a python integer /// object, accounting for negative index, and out of bounds issues. // pub fn get_sequence_index(vm: &VirtualMachine, index: &PyIntRef, length: usize) -> PyResult { @@ -455,31 +496,6 @@ pub(crate) fn saturate_index(p: isize, len: usize) -> usize { // start..stop // } -pub fn get_item( - vm: &VirtualMachine, - sequence: &PyObjectRef, - elements: &[PyObjectRef], - subscript: PyObjectRef, -) -> PyResult>> { - if let Some(i) = subscript.payload::() { - let value = i.borrow_value().to_isize().ok_or_else(|| { - vm.new_index_error("cannot fit 'int' into an index-sized integer".to_owned()) - })?; - let pos_index = elements - .wrap_index(value) - .ok_or_else(|| vm.new_index_error("Index out of bounds!".to_owned()))?; - Ok(Either::A(elements[pos_index].clone())) - } else { - let slice = subscript.payload::().ok_or_else(|| { - vm.new_type_error(format!( - "{} indices must be integers or slices", - sequence.lease_class().name - )) - })?; - Ok(Either::B(elements.get_slice_items(vm, slice)?)) - } -} - //Check if given arg could be used with PySliceableSequence.saturate_range() // pub fn is_valid_slice_arg( // arg: OptionalArg, From 39058e2519a976799929b35b1355b680f6174f26 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 1 Oct 2020 20:17:10 +0900 Subject: [PATCH 6/6] getitem for bytes/bytearray --- Lib/test/test_bytes.py | 4 ---- vm/src/bytesinner.rs | 24 +++++++++++------------- vm/src/obj/objbytearray.rs | 4 ++-- vm/src/obj/objbytes.rs | 5 ++--- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index ff40a715d..2c6717b12 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -934,8 +934,6 @@ class BaseBytesTest: class BytesTest(BaseBytesTest, unittest.TestCase): type2test = bytes - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_getitem_error(self): b = b'python' msg = "byte indices must be integers or slices" @@ -1133,8 +1131,6 @@ class BytesTest(BaseBytesTest, unittest.TestCase): class ByteArrayTest(BaseBytesTest, unittest.TestCase): type2test = bytearray - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_getitem_error(self): b = bytearray(b'python') msg = "bytearray indices must be integers or slices" diff --git a/vm/src/bytesinner.rs b/vm/src/bytesinner.rs index 79727da97..cfbb2538d 100644 --- a/vm/src/bytesinner.rs +++ b/vm/src/bytesinner.rs @@ -295,19 +295,17 @@ impl PyBytesInner { }) } - pub fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult { - match needle { - SequenceIndex::Int(int) => { - if let Some(idx) = self.elements.wrap_index(int) { - Ok(vm.ctx.new_int(self.elements[idx])) - } else { - Err(vm.new_index_error("index out of range".to_owned())) - } - } - SequenceIndex::Slice(slice) => { - Ok(vm.ctx.new_bytes(self.elements.get_slice_items(vm, &slice)?)) - } - } + pub fn getitem( + &self, + name: &'static str, + needle: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + let obj = match self.elements.get_item(vm, needle, name)? { + Either::A(byte) => vm.new_pyobj(byte), + Either::B(bytes) => vm.ctx.new_bytes(bytes), + }; + Ok(obj) } pub fn setindex( diff --git a/vm/src/obj/objbytearray.rs b/vm/src/obj/objbytearray.rs index acc40ebd7..5ce2e9573 100644 --- a/vm/src/obj/objbytearray.rs +++ b/vm/src/obj/objbytearray.rs @@ -152,8 +152,8 @@ impl PyByteArray { } #[pymethod(name = "__getitem__")] - fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult { - self.borrow_value().getitem(needle, vm) + fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.borrow_value().getitem("bytearray", needle, vm) } #[pymethod(name = "__setitem__")] diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index abd47c500..54444cf62 100644 --- a/vm/src/obj/objbytes.rs +++ b/vm/src/obj/objbytes.rs @@ -19,7 +19,6 @@ use crate::pyobject::{ BorrowValue, Either, IntoPyObject, PyClassImpl, PyComparisonValue, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, }; -use crate::sliceable::SequenceIndex; use crate::slots::{Comparable, Hashable, PyComparisonOp}; use crate::vm::VirtualMachine; use rustpython_common::hash::PyHash; @@ -139,8 +138,8 @@ impl PyBytes { } #[pymethod(name = "__getitem__")] - fn getitem(&self, needle: SequenceIndex, vm: &VirtualMachine) -> PyResult { - self.inner.getitem(needle, vm) + fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self.inner.getitem("byte", needle, vm) } #[pymethod(name = "isalnum")]