From 06463f98eb804fca20c73270534cedda5ba07813 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 11 Apr 2020 11:40:14 +0900 Subject: [PATCH] Fix bytes.find --- Lib/test/string_tests.py | 7 ++++--- vm/src/obj/objbytearray.rs | 13 +++++++------ vm/src/obj/objbyteinner.rs | 32 +++++++------------------------- vm/src/obj/objbytes.rs | 9 +++++---- vm/src/obj/objstr.rs | 8 ++------ vm/src/obj/pystr.rs | 15 +++++++++++++++ 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 495044433..59766a3eb 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -157,7 +157,8 @@ class BaseTest: self.assertEqual(rem, 0, '%s != 0 for %s' % (rem, i)) self.assertEqual(r1, r2, '%s != %s for %s' % (r1, r2, i)) - @unittest.skip("TODO: RUSTPYTHON test_bytes") + # TODO: RUSTPYTHON + @unittest.expectedFailure def test_find(self): self.checkequal(0, 'abcdefghiabc', 'find', 'abc') self.checkequal(9, 'abcdefghiabc', 'find', 'abc', 1) @@ -215,7 +216,8 @@ class BaseTest: if loc != -1: self.assertEqual(i[loc:loc+len(j)], j) - @unittest.skip("TODO: RUSTPYTHON test_bytes") + # TODO: RUSTPYTHON + @unittest.expectedFailure def test_rfind(self): self.checkequal(9, 'abcdefghiabc', 'rfind', 'abc') self.checkequal(12, 'abcdefghiabc', 'rfind', '') @@ -294,7 +296,6 @@ class BaseTest: else: self.checkraises(TypeError, 'hello', 'index', 42) - @unittest.skip("TODO: RUSTPYTHON test_bytes") def test_rindex(self): self.checkequal(12, 'abcdefghiabc', 'rindex', '') self.checkequal(3, 'abcdefghiabc', 'rindex', 'def') diff --git a/vm/src/obj/objbytearray.rs b/vm/src/obj/objbytearray.rs index 1c7d52316..029be2d1c 100644 --- a/vm/src/obj/objbytearray.rs +++ b/vm/src/obj/objbytearray.rs @@ -1,6 +1,9 @@ //! Implementation of the python bytearray object. +use bstr::ByteSlice; use crossbeam_utils::atomic::AtomicCell; use std::convert::TryFrom; +use std::mem::size_of; +use std::str::FromStr; use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use super::objbyteinner::{ @@ -21,8 +24,6 @@ use crate::pyobject::{ PyValue, ThreadSafe, TryFromObject, TypeProtocol, }; use crate::vm::VirtualMachine; -use std::mem::size_of; -use std::str::FromStr; /// "bytearray(iterable_of_ints) -> bytearray\n\ /// bytearray(string, encoding[, errors]) -> bytearray\n\ @@ -327,25 +328,25 @@ impl PyByteArray { #[pymethod(name = "find")] fn find(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.borrow_value().find(options, false, vm)?; + let index = self.borrow_value().find(options, |h, n| h.find(n), vm)?; Ok(index.map_or(-1, |v| v as isize)) } #[pymethod(name = "index")] fn index(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.borrow_value().find(options, false, vm)?; + let index = self.borrow_value().find(options, |h, n| h.find(n), vm)?; index.ok_or_else(|| vm.new_value_error("substring not found".to_owned())) } #[pymethod(name = "rfind")] fn rfind(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.borrow_value().find(options, true, vm)?; + let index = self.borrow_value().find(options, |h, n| h.rfind(n), vm)?; Ok(index.map_or(-1, |v| v as isize)) } #[pymethod(name = "rindex")] fn rindex(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.borrow_value().find(options, true, vm)?; + let index = self.borrow_value().find(options, |h, n| h.rfind(n), vm)?; index.ok_or_else(|| vm.new_value_error("substring not found".to_owned())) } diff --git a/vm/src/obj/objbyteinner.rs b/vm/src/obj/objbyteinner.rs index dffd6c95e..1ce464f23 100644 --- a/vm/src/obj/objbyteinner.rs +++ b/vm/src/obj/objbyteinner.rs @@ -823,35 +823,17 @@ impl PyByteInner { } #[inline] - pub fn find( + pub fn find( &self, options: ByteInnerFindOptions, - reverse: bool, + find: F, vm: &VirtualMachine, - ) -> PyResult> { + ) -> PyResult> + where + F: Fn(&[u8], &[u8]) -> Option, + { let (needle, range) = options.get_value(self.elements.len(), vm)?; - if !range.is_normal() { - return Ok(None); - } - if needle.is_empty() { - return Ok(Some(if reverse { range.end } else { range.start })); - } - let haystack = &self.elements[range.clone()]; - let windows = haystack.windows(needle.len()); - if reverse { - for (i, w) in windows.rev().enumerate() { - if w == needle.as_slice() { - return Ok(Some(range.end - i - needle.len())); - } - } - } else { - for (i, w) in windows.enumerate() { - if w == needle.as_slice() { - return Ok(Some(range.start + i)); - } - } - } - Ok(None) + Ok(self.elements.py_find(&needle, range, find)) } pub fn maketrans(from: PyByteInner, to: PyByteInner, vm: &VirtualMachine) -> PyResult { diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index 83a26d557..bf76f49a2 100644 --- a/vm/src/obj/objbytes.rs +++ b/vm/src/obj/objbytes.rs @@ -1,3 +1,4 @@ +use bstr::ByteSlice; use crossbeam_utils::atomic::AtomicCell; use std::mem::size_of; use std::ops::Deref; @@ -300,25 +301,25 @@ impl PyBytes { #[pymethod(name = "find")] fn find(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.inner.find(options, false, vm)?; + let index = self.inner.find(options, |h, n| h.find(n), vm)?; Ok(index.map_or(-1, |v| v as isize)) } #[pymethod(name = "index")] fn index(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.inner.find(options, false, vm)?; + let index = self.inner.find(options, |h, n| h.find(n), vm)?; index.ok_or_else(|| vm.new_value_error("substring not found".to_owned())) } #[pymethod(name = "rfind")] fn rfind(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.inner.find(options, true, vm)?; + let index = self.inner.find(options, |h, n| h.rfind(n), vm)?; Ok(index.map_or(-1, |v| v as isize)) } #[pymethod(name = "rindex")] fn rindex(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - let index = self.inner.find(options, true, vm)?; + let index = self.inner.find(options, |h, n| h.rfind(n), vm)?; index.ok_or_else(|| vm.new_value_error("substring not found".to_owned())) } diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 936673797..aabf623ec 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -811,6 +811,7 @@ impl PyString { Ok(joined) } + #[inline] fn _find( &self, sub: PyStringRef, @@ -822,12 +823,7 @@ impl PyString { F: Fn(&str, &str) -> Option, { let range = adjust_indices(start, end, self.value.len()); - if range.is_normal() { - if let Some(index) = find(&self.value[range.clone()], &sub.value) { - return Some(range.start + index); - } - } - None + self.value.py_find(&sub.value, range, find) } #[pymethod] diff --git a/vm/src/obj/pystr.rs b/vm/src/obj/pystr.rs index c0c0d113e..46921da9b 100644 --- a/vm/src/obj/pystr.rs +++ b/vm/src/obj/pystr.rs @@ -186,6 +186,7 @@ pub trait PyCommonString { } } + #[inline] fn py_strip<'a, S, FC, FD>( &'a self, chars: OptionalOption, @@ -203,4 +204,18 @@ pub trait PyCommonString { None => func_default(self), } } + + #[inline] + fn py_find(&self, needle: &Self, range: std::ops::Range, find: F) -> Option + where + F: Fn(&Self, &Self) -> Option, + { + if range.is_normal() { + let start = range.start; + if let Some(index) = find(self.get_slice(range), &needle) { + return Some(start + index); + } + } + None + } }