From 0d2b817dd3682d0ea1fa8a58d09220fed2a6c746 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 13 Apr 2020 03:14:39 +0900 Subject: [PATCH 1/2] single_or_tuple_any uses TryFromObject instead of PyValue --- vm/src/builtins.rs | 10 +++++--- vm/src/frame.rs | 26 ++++----------------- vm/src/function.rs | 54 +++++++++++++++++++++++++++----------------- vm/src/obj/objstr.rs | 4 ++-- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index af9a02af68..d4899e0a5e 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -363,10 +363,14 @@ fn builtin_input(prompt: OptionalArg, vm: &VirtualMachine) -> PyRes } } -fn builtin_isinstance(obj: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { +pub fn builtin_isinstance( + obj: PyObjectRef, + typ: PyObjectRef, + vm: &VirtualMachine, +) -> PyResult { single_or_tuple_any( typ, - |cls: PyClassRef| vm.isinstance(&obj, &cls), + |cls: &PyClassRef| vm.isinstance(&obj, cls), |o| { format!( "isinstance() arg 2 must be a type or tuple of types, not {}", @@ -384,7 +388,7 @@ fn builtin_issubclass( ) -> PyResult { single_or_tuple_any( typ, - |cls: PyClassRef| vm.issubclass(&subclass, &cls), + |cls: &PyClassRef| vm.issubclass(&subclass, cls), |o| { format!( "issubclass() arg 2 must be a class or tuple of classes, not {}", diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 63ba01f09a..f374507369 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -5,9 +5,10 @@ use std::sync::Mutex; use indexmap::IndexMap; use itertools::Itertools; +use crate::builtins::builtin_isinstance; use crate::bytecode; use crate::exceptions::{self, ExceptionCtor, PyBaseExceptionRef}; -use crate::function::{single_or_tuple_any, PyFuncArgs}; +use crate::function::PyFuncArgs; use crate::obj::objasyncgenerator::PyAsyncGenWrappedValue; use crate::obj::objbool; use crate::obj::objcode::PyCodeRef; @@ -1353,25 +1354,6 @@ impl ExecutingFrame<'_> { !a.is(&b) } - fn exc_match( - &self, - vm: &VirtualMachine, - exc: PyObjectRef, - exc_type: PyObjectRef, - ) -> PyResult { - single_or_tuple_any( - exc_type, - |cls: PyClassRef| vm.isinstance(&exc, &cls), - |o| { - format!( - "isinstance() arg 2 must be a type or tuple of types, not {}", - o.class() - ) - }, - vm, - ) - } - #[cfg_attr(feature = "flame-it", flame("Frame"))] fn execute_compare( &mut self, @@ -1391,7 +1373,9 @@ impl ExecutingFrame<'_> { bytecode::ComparisonOperator::IsNot => vm.new_bool(self._is_not(a, b)), bytecode::ComparisonOperator::In => vm.new_bool(self._in(vm, a, b)?), bytecode::ComparisonOperator::NotIn => vm.new_bool(self._not_in(vm, a, b)?), - bytecode::ComparisonOperator::ExceptionMatch => vm.new_bool(self.exc_match(vm, a, b)?), + bytecode::ComparisonOperator::ExceptionMatch => { + vm.new_bool(builtin_isinstance(a, b, vm)?) + } }; self.push_value(value); diff --git a/vm/src/function.rs b/vm/src/function.rs index 75d03b339f..842d27a98f 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -7,7 +7,7 @@ use result_like::impl_option_like; use smallbox::{smallbox, space::S1, SmallBox}; use crate::exceptions::PyBaseExceptionRef; -use crate::obj::objtuple::PyTuple; +use crate::obj::objtuple::PyTupleRef; use crate::obj::objtype::{isinstance, PyClassRef}; use crate::pyobject::{ IntoPyObject, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, @@ -564,43 +564,55 @@ into_py_native_func_tuple!((a, A), (b, B), (c, C), (d, D), (e, E)); /// test that any of the values contained within the tuples satisfies the predicate. Type parameter /// T specifies the type that is expected, if the input value is not of that type or a tuple of /// values of that type, then a TypeError is raised. -pub fn single_or_tuple_any) -> PyResult>( +pub fn single_or_tuple_any( obj: PyObjectRef, predicate: F, - message: fn(&PyObjectRef) -> String, + message: M, vm: &VirtualMachine, -) -> PyResult { +) -> PyResult +where + T: TryFromObject, + F: Fn(&T) -> PyResult, + M: Fn(&PyObjectRef) -> String, +{ // TODO: figure out some way to have recursive calls without... this - use std::marker::PhantomData; - struct Checker<'vm, T: PyValue, F: Fn(PyRef) -> PyResult> { + struct Checker + where + F: Fn(&T) -> PyResult, + M: Fn(&PyObjectRef) -> String, + { predicate: F, - message: fn(&PyObjectRef) -> String, - vm: &'vm VirtualMachine, - t: PhantomData, - } - impl) -> PyResult> Checker<'_, T, F> { - fn check(&self, obj: PyObjectRef) -> PyResult { - match_class!(match obj { - obj @ T => (self.predicate)(obj), - tuple @ PyTuple => { + message: M, + t: std::marker::PhantomData, + } + impl Checker + where + T: TryFromObject, + F: Fn(&T) -> PyResult, + M: Fn(&PyObjectRef) -> String, + { + fn check(&self, obj: &PyObjectRef, vm: &VirtualMachine) -> PyResult { + match T::try_from_object(vm, obj.clone()) { + Ok(single) => (self.predicate)(&single), + Err(_) => { + let tuple = PyTupleRef::try_from_object(vm, obj.clone()) + .map_err(|_| vm.new_type_error((self.message)(&obj)))?; for obj in tuple.as_slice().iter() { - if self.check(obj.clone())? { + if self.check(&obj, vm)? { return Ok(true); } } Ok(false) } - obj => Err(self.vm.new_type_error((self.message)(&obj))), - }) + } } } let checker = Checker { predicate, message, - vm, - t: PhantomData, + t: std::marker::PhantomData, }; - checker.check(obj) + checker.check(&obj, vm) } #[cfg(test)] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 429a98eafd..00ed17e2ce 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -530,7 +530,7 @@ impl PyString { let value = &self.value[range]; single_or_tuple_any( suffix, - |s: PyStringRef| Ok(value.ends_with(&s.value)), + |s: &PyStringRef| Ok(value.ends_with(&s.value)), |o| { format!( "endswith first arg must be str or a tuple of str, not {}", @@ -557,7 +557,7 @@ impl PyString { let value = &self.value[range]; single_or_tuple_any( prefix, - |s: PyStringRef| Ok(value.starts_with(&s.value)), + |s: &PyStringRef| Ok(value.starts_with(&s.value)), |o| { format!( "startswith first arg must be str or a tuple of str, not {}", From 4c9486fb973e9428e6c70d3267a6323408e95db2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 13 Apr 2020 03:33:50 +0900 Subject: [PATCH 2/2] Fix bytes.{start|end}swith --- Lib/test/test_bytes.py | 4 -- vm/src/obj/objbytearray.rs | 36 +++++++++----- vm/src/obj/objbyteinner.rs | 58 +++++----------------- vm/src/obj/objbytes.rs | 36 ++++++++++---- vm/src/obj/objstr.rs | 99 +++++++++++--------------------------- vm/src/obj/pystr.rs | 79 +++++++++++++++++++++++++++++- 6 files changed, 169 insertions(+), 143 deletions(-) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 1bbdef59e7..d497609395 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -548,8 +548,6 @@ def test_count(self): self.assertEqual(b.count(i, 1, 3), 1) self.assertEqual(b.count(p, 7, 9), 1) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_startswith(self): b = self.type2test(b'hello') self.assertFalse(self.type2test().startswith(b"anything")) @@ -564,8 +562,6 @@ def test_startswith(self): self.assertIn('bytes', exc) self.assertIn('tuple', exc) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_endswith(self): b = self.type2test(b'hello') self.assertFalse(bytearray().endswith(b"anything")) diff --git a/vm/src/obj/objbytearray.rs b/vm/src/obj/objbytearray.rs index 9a2fc0cdf5..c79aa38f68 100644 --- a/vm/src/obj/objbytearray.rs +++ b/vm/src/obj/objbytearray.rs @@ -12,8 +12,8 @@ use super::objint::PyIntRef; use super::objiter; use super::objslice::PySliceRef; use super::objstr::{PyString, PyStringRef}; -use super::objtuple::PyTupleRef; use super::objtype::PyClassRef; +use super::pystr::PyCommonString; use crate::cformat::CFormatString; use crate::function::{OptionalArg, OptionalOption}; use crate::obj::objstr::do_cformat_string; @@ -303,25 +303,39 @@ impl PyByteArray { #[pymethod(name = "endswith")] fn endswith( &self, - suffix: Either, - start: OptionalArg, - end: OptionalArg, + suffix: PyObjectRef, + start: OptionalArg>, + end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - self.borrow_value() - .startsendswith(suffix, start, end, true, vm) + self.borrow_value().elements[..].py_startsendswith( + suffix, + start, + end, + "endswith", + "bytes", + |s, x: &PyByteInner| s.ends_with(&x.elements[..]), + vm, + ) } #[pymethod(name = "startswith")] fn startswith( &self, - prefix: Either, - start: OptionalArg, - end: OptionalArg, + prefix: PyObjectRef, + start: OptionalArg>, + end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - self.borrow_value() - .startsendswith(prefix, start, end, false, vm) + self.borrow_value().elements[..].py_startsendswith( + prefix, + start, + end, + "startswith", + "bytes", + |s, x: &PyByteInner| s.starts_with(&x.elements[..]), + vm, + ) } #[pymethod(name = "find")] diff --git a/vm/src/obj/objbyteinner.rs b/vm/src/obj/objbyteinner.rs index 753bbf4e35..1a4c1d305c 100644 --- a/vm/src/obj/objbyteinner.rs +++ b/vm/src/obj/objbyteinner.rs @@ -11,11 +11,10 @@ use super::objint::{self, PyInt, PyIntRef}; use super::objlist::PyList; use super::objmemory::PyMemoryView; use super::objnone::PyNoneRef; -use super::objsequence::{is_valid_slice_arg, PySliceableSequence}; +use super::objsequence::PySliceableSequence; use super::objslice::PySliceRef; -use super::objstr::{self, adjust_indices, PyString, PyStringRef, StringRange}; -use super::objtuple::PyTupleRef; -use super::pystr::PyCommonString; +use super::objstr::{self, PyString, PyStringRef}; +use super::pystr::{self, PyCommonString, StringRange}; use crate::function::{OptionalArg, OptionalOption}; use crate::pyhash; use crate::pyobject::{ @@ -172,7 +171,7 @@ impl ByteInnerFindOptions { Either::A(v) => v.elements.to_vec(), Either::B(int) => vec![int.as_bigint().byte_or(vm)?], }; - let range = adjust_indices(self.start, self.end, len); + let range = pystr::adjust_indices(self.start, self.end, len); Ok((sub, range)) } } @@ -856,47 +855,6 @@ impl PyByteInner { Ok(refs) } - #[inline] - pub fn startsendswith( - &self, - arg: Either, - start: OptionalArg, - end: OptionalArg, - endswith: bool, // true for endswith, false for startswith - vm: &VirtualMachine, - ) -> PyResult { - let suff = match arg { - Either::A(byte) => byte.elements, - Either::B(tuple) => { - let mut flatten = vec![]; - for v in tuple.as_slice() { - flatten.extend(PyByteInner::try_from_object(vm, v.clone())?.elements) - } - flatten - } - }; - - if suff.is_empty() { - return Ok(true); - } - let range = self.elements.get_slice_range( - &is_valid_slice_arg(start, vm)?, - &is_valid_slice_arg(end, vm)?, - ); - - if range.end - range.start < suff.len() { - return Ok(false); - } - - let offset = if endswith { - (range.end - suff.len())..range.end - } else { - 0..suff.len() - }; - - Ok(suff.as_slice() == &self.elements.do_slice(range)[offset]) - } - #[inline] pub fn find( &self, @@ -1342,6 +1300,14 @@ pub fn bytes_zfill(bytes: &[u8], width: usize) -> Vec { const ASCII_WHITESPACES: [u8; 6] = [0x20, 0x09, 0x0a, 0x0c, 0x0d, 0x0b]; impl PyCommonString<'_, u8> for [u8] { + fn get_slice(&self, range: std::ops::Range) -> &Self { + &self[range] + } + + fn len(&self) -> usize { + Self::len(self) + } + fn py_split_whitespace(&self, maxsplit: isize, convert: F) -> Vec where F: Fn(&Self) -> PyObjectRef, diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index 1e7675f23d..ffd18f5455 100644 --- a/vm/src/obj/objbytes.rs +++ b/vm/src/obj/objbytes.rs @@ -1,6 +1,7 @@ use crossbeam_utils::atomic::AtomicCell; use std::mem::size_of; use std::ops::Deref; +use std::str::FromStr; use super::objbyteinner::{ ByteInnerExpandtabsOptions, ByteInnerFindOptions, ByteInnerNewOptions, ByteInnerPaddingOptions, @@ -10,8 +11,8 @@ use super::objint::PyIntRef; use super::objiter; use super::objslice::PySliceRef; use super::objstr::{PyString, PyStringRef}; -use super::objtuple::PyTupleRef; use super::objtype::PyClassRef; +use super::pystr::PyCommonString; use crate::cformat::CFormatString; use crate::function::{OptionalArg, OptionalOption}; use crate::obj::objstr::do_cformat_string; @@ -23,7 +24,6 @@ use crate::pyobject::{ ThreadSafe, TryFromObject, TypeProtocol, }; use crate::vm::VirtualMachine; -use std::str::FromStr; /// "bytes(iterable_of_ints) -> bytes\n\ /// bytes(string, encoding[, errors]) -> bytes\n\ @@ -275,23 +275,39 @@ impl PyBytes { #[pymethod(name = "endswith")] fn endswith( &self, - suffix: Either, - start: OptionalArg, - end: OptionalArg, + suffix: PyObjectRef, + start: OptionalArg>, + end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - self.inner.startsendswith(suffix, start, end, true, vm) + self.inner.elements[..].py_startsendswith( + suffix, + start, + end, + "endswith", + "bytes", + |s, x: &PyByteInner| s.ends_with(&x.elements[..]), + vm, + ) } #[pymethod(name = "startswith")] fn startswith( &self, - prefix: Either, - start: OptionalArg, - end: OptionalArg, + prefix: PyObjectRef, + start: OptionalArg>, + end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - self.inner.startsendswith(prefix, start, end, false, vm) + self.inner.elements[..].py_startsendswith( + prefix, + start, + end, + "startswith", + "bytes", + |s, x: &PyByteInner| s.starts_with(&x.elements[..]), + vm, + ) } #[pymethod(name = "find")] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 00ed17e2ce..a53603cc31 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -21,13 +21,13 @@ use super::objsequence::PySliceableSequence; use super::objslice::PySliceRef; use super::objtuple; use super::objtype::{self, PyClassRef}; -use super::pystr::PyCommonString; +use super::pystr::{adjust_indices, PyCommonString, StringRange}; use crate::cformat::{ CFormatPart, CFormatPreconversor, CFormatQuantity, CFormatSpec, CFormatString, CFormatType, CNumberType, }; use crate::format::{FormatParseError, FormatPart, FormatPreconversor, FormatSpec, FormatString}; -use crate::function::{single_or_tuple_any, OptionalArg, PyFuncArgs}; +use crate::function::{OptionalArg, PyFuncArgs}; use crate::pyhash; use crate::pyobject::{ Either, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyContext, PyIterable, @@ -525,23 +525,15 @@ impl PyString { end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - let range = adjust_indices(start, end, self.value.len()); - if range.is_normal() { - let value = &self.value[range]; - single_or_tuple_any( - suffix, - |s: &PyStringRef| Ok(value.ends_with(&s.value)), - |o| { - format!( - "endswith first arg must be str or a tuple of str, not {}", - o.class(), - ) - }, - vm, - ) - } else { - Ok(false) - } + self.value.as_str().py_startsendswith( + suffix, + start, + end, + "endswith", + "str", + |s, x: &PyStringRef| s.ends_with(x.as_str()), + vm, + ) } #[pymethod] @@ -552,23 +544,15 @@ impl PyString { end: OptionalArg>, vm: &VirtualMachine, ) -> PyResult { - let range = adjust_indices(start, end, self.value.len()); - if range.is_normal() { - let value = &self.value[range]; - single_or_tuple_any( - prefix, - |s: &PyStringRef| Ok(value.starts_with(&s.value)), - |o| { - format!( - "startswith first arg must be str or a tuple of str, not {}", - o.class(), - ) - }, - vm, - ) - } else { - Ok(false) - } + self.value.as_str().py_startsendswith( + prefix, + start, + end, + "startswith", + "str", + |s, x: &PyStringRef| s.starts_with(x.as_str()), + vm, + ) } #[pymethod] @@ -1724,41 +1708,6 @@ impl PySliceableSequence for String { } } -pub trait StringRange { - fn is_normal(&self) -> bool; -} - -impl StringRange for std::ops::Range { - fn is_normal(&self) -> bool { - self.start <= self.end - } -} - -// help get optional string indices -pub fn adjust_indices( - start: OptionalArg>, - end: OptionalArg>, - len: usize, -) -> std::ops::Range { - let mut start = start.flat_option().unwrap_or(0); - let mut end = end.flat_option().unwrap_or(len as isize); - if end > len as isize { - end = len as isize; - } else if end < 0 { - end += len as isize; - if end < 0 { - end = 0; - } - } - if start < 0 { - start += len as isize; - if start < 0 { - start = 0; - } - } - start as usize..end as usize -} - // According to python following categories aren't printable: // * Cc (Other, Control) // * Cf (Other, Format) @@ -1851,6 +1800,14 @@ mod tests { } impl PyCommonString<'_, char> for str { + fn get_slice(&self, range: std::ops::Range) -> &Self { + &self[range] + } + + fn len(&self) -> usize { + Self::len(self) + } + fn py_split_whitespace(&self, maxsplit: isize, convert: F) -> Vec where F: Fn(&Self) -> PyObjectRef, diff --git a/vm/src/obj/pystr.rs b/vm/src/obj/pystr.rs index 54be550a15..ec2677854b 100644 --- a/vm/src/obj/pystr.rs +++ b/vm/src/obj/pystr.rs @@ -1,10 +1,49 @@ -use crate::pyobject::PyObjectRef; +use crate::function::{single_or_tuple_any, OptionalOption}; +use crate::pyobject::{PyObjectRef, PyResult, TryFromObject, TypeProtocol}; use crate::vm::VirtualMachine; +// help get optional string indices +pub fn adjust_indices( + start: OptionalOption, + end: OptionalOption, + len: usize, +) -> std::ops::Range { + let mut start = start.flat_option().unwrap_or(0); + let mut end = end.flat_option().unwrap_or(len as isize); + if end > len as isize { + end = len as isize; + } else if end < 0 { + end += len as isize; + if end < 0 { + end = 0; + } + } + if start < 0 { + start += len as isize; + if start < 0 { + start = 0; + } + } + start as usize..end as usize +} + +pub trait StringRange { + fn is_normal(&self) -> bool; +} + +impl StringRange for std::ops::Range { + fn is_normal(&self) -> bool { + self.start <= self.end + } +} + pub trait PyCommonString<'a, E> where Self: 'a, { + fn get_slice(&self, range: std::ops::Range) -> &Self; + fn len(&self) -> usize; + fn py_split( &self, sep: Option<&Self>, @@ -35,4 +74,42 @@ where fn py_rsplit_whitespace(&self, maxsplit: isize, convert: F) -> Vec where F: Fn(&Self) -> PyObjectRef; + + #[allow(clippy::too_many_arguments)] + #[inline] + fn py_startsendswith( + &self, + affix: PyObjectRef, + start: OptionalOption, + end: OptionalOption, + func_name: &str, + py_type_name: &str, + func: F, + vm: &VirtualMachine, + ) -> PyResult + where + T: TryFromObject, + F: Fn(&Self, &T) -> bool, + { + let range = adjust_indices(start, end, self.len()); + if range.is_normal() { + let value = self.get_slice(range); + single_or_tuple_any( + affix, + |s: &T| Ok(func(value, s)), + |o| { + format!( + "{} first arg must be {} or a tuple of {}, not {}", + func_name, + py_type_name, + py_type_name, + o.class(), + ) + }, + vm, + ) + } else { + Ok(false) + } + } }