From d513aea7d21f00b2132b1f7d548900e774381eb8 Mon Sep 17 00:00:00 2001
From: Jeong Yunwon <jeong@youknowone.org>
Date: Thu, 21 Jul 2022 06:27:21 +0900
Subject: [PATCH] MutObjectSequenceOp find using Iteartor trait
to isolate control flow code from next() operation
---
vm/src/builtins/list.rs | 4 +-
vm/src/sequence.rs | 113 +++++++++++++++++++++--------------
vm/src/stdlib/collections.rs | 4 +-
3 files changed, 72 insertions(+), 49 deletions(-)
diff --git a/vm/src/builtins/list.rs b/vm/src/builtins/list.rs
index f2944942ee..d03d51b161 100644
--- a/vm/src/builtins/list.rs
+++ b/vm/src/builtins/list.rs
@@ -271,7 +271,7 @@ impl PyList {
) -> PyResult<usize> {
let (start, stop) = range.saturate(self.len(), vm)?;
let index = self.mut_index_range(vm, &needle, start..stop)?;
- if let Some(index) = index.into() {
+ if let Some(index) = index {
Ok(index)
} else {
Err(vm.new_value_error(format!("'{}' is not in list", needle.str(vm)?)))
@@ -298,7 +298,7 @@ impl PyList {
fn remove(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
let index = self.mut_index(vm, &needle)?;
- if let Some(index) = index.into() {
+ if let Some(index) = index {
// defer delete out of borrow
let is_inside_range = index < self.borrow_vec().len();
Ok(is_inside_range.then(|| self.borrow_vec_mut().remove(index)))
diff --git a/vm/src/sequence.rs b/vm/src/sequence.rs
index 2ed07b62c7..4297d1ffed 100644
--- a/vm/src/sequence.rs
+++ b/vm/src/sequence.rs
@@ -5,17 +5,18 @@ use crate::{
use optional::Optioned;
use std::ops::Range;
-pub trait MutObjectSequenceOp<'a> {
- type Guard;
+pub trait MutObjectSequenceOp<'a>: Sized {
+ type Guard: Sized;
fn do_get(index: usize, guard: &Self::Guard) -> Option<&PyObjectRef>;
fn do_lock(&'a self) -> Self::Guard;
fn mut_count(&'a self, vm: &VirtualMachine, needle: &PyObject) -> PyResult<usize> {
+ let mut find_iter = self._mut_find(vm, needle, 0..isize::MAX as usize);
let mut count = 0;
- self._mut_iter_equal_skeleton::<_, false>(vm, needle, 0..isize::MAX as usize, || {
- count += 1
- })?;
+ while (find_iter.next().transpose()?).is_some() {
+ count += 1;
+ }
Ok(count)
}
@@ -24,11 +25,11 @@ pub trait MutObjectSequenceOp<'a> {
vm: &VirtualMachine,
needle: &PyObject,
range: Range<usize>,
- ) -> PyResult<Optioned<usize>> {
- self._mut_iter_equal_skeleton::<_, true>(vm, needle, range, || {})
+ ) -> PyResult<Option<usize>> {
+ self._mut_find(vm, needle, range).next().transpose()
}
- fn mut_index(&'a self, vm: &VirtualMachine, needle: &PyObject) -> PyResult<Optioned<usize>> {
+ fn mut_index(&'a self, vm: &VirtualMachine, needle: &PyObject) -> PyResult<Option<usize>> {
self.mut_index_range(vm, needle, 0..isize::MAX as usize)
}
@@ -36,56 +37,78 @@ pub trait MutObjectSequenceOp<'a> {
self.mut_index(vm, needle).map(|x| x.is_some())
}
- fn _mut_iter_equal_skeleton<F, const SHORT: bool>(
+ fn _mut_find<'b, 'vm>(
&'a self,
- vm: &VirtualMachine,
- needle: &PyObject,
+ vm: &'vm VirtualMachine,
+ needle: &'b PyObject,
range: Range<usize>,
- mut f: F,
- ) -> PyResult<Optioned<usize>>
- where
- F: FnMut(),
- {
- let mut borrower = None;
- let mut i = range.start;
-
- let index = loop {
- if i >= range.end {
- break Optioned::<usize>::none();
- }
- let guard = if let Some(x) = borrower.take() {
- x
- } else {
- self.do_lock()
- };
+ ) -> MutObjectSequenceFindIter<'a, 'b, 'vm, Self> {
+ MutObjectSequenceFindIter {
+ seq: self,
+ needle,
+ pos: range.start,
+ end: range.end,
+ guard: None,
+ vm,
+ }
+ }
+}
+
+pub struct MutObjectSequenceFindIter<'a, 'b, 'vm, S: MutObjectSequenceOp<'a>> {
+ // mutable fields
+ pos: usize,
+ guard: Option<S::Guard>,
+ // immutable fields
+ seq: &'a S,
+ needle: &'b PyObject,
+ end: usize,
+ vm: &'vm VirtualMachine,
+}
- let elem = if let Some(x) = Self::do_get(i, &guard) {
+impl<'a, 'b, 'vm, S: MutObjectSequenceOp<'a>> MutObjectSequenceFindIter<'a, 'b, 'vm, S> {
+ #[inline]
+ fn next_impl(&mut self) -> PyResult<Optioned<usize>> {
+ loop {
+ if self.pos >= self.end {
+ return Ok(Optioned::none());
+ }
+ let guard = self.guard.take().unwrap_or_else(|| self.seq.do_lock());
+ let elem = if let Some(x) = S::do_get(self.pos, &guard) {
x
} else {
- break Optioned::<usize>::none();
+ return Ok(Optioned::none());
};
- if elem.is(needle) {
- f();
- if SHORT {
- break Optioned::<usize>::some(i);
- }
- borrower = Some(guard);
+ let is_equal = if elem.is(self.needle) {
+ self.guard = Some(guard);
+ true
} else {
let elem = elem.clone();
drop(guard);
+ elem.rich_compare_bool(self.needle, PyComparisonOp::Eq, self.vm)?
+ };
- if elem.rich_compare_bool(needle, PyComparisonOp::Eq, vm)? {
- f();
- if SHORT {
- break Optioned::<usize>::some(i);
- }
- }
+ if is_equal {
+ break;
}
- i += 1;
- };
- Ok(index)
+ self.pos += 1;
+ }
+
+ let i = self.pos;
+ self.pos += 1;
+ Ok(Optioned::some(i))
+ }
+}
+
+impl<'a, 'b, 'vm, S: MutObjectSequenceOp<'a>> Iterator
+ for MutObjectSequenceFindIter<'a, 'b, 'vm, S>
+{
+ type Item = PyResult<usize>;
+
+ #[inline]
+ fn next(&mut self) -> Option<Self::Item> {
+ self.next_impl().map(Into::into).transpose()
}
}
diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs
index a3cbdab996..32abb10e5a 100644
--- a/vm/src/stdlib/collections.rs
+++ b/vm/src/stdlib/collections.rs
@@ -167,7 +167,7 @@ mod _collections {
let index = self.mut_index_range(vm, &needle, start..stop)?;
if start_state != self.state.load() {
Err(vm.new_runtime_error("deque mutated during iteration".to_owned()))
- } else if let Some(index) = index.into() {
+ } else if let Some(index) = index {
Ok(index)
} else {
Err(vm.new_value_error(
@@ -228,7 +228,7 @@ mod _collections {
if start_state != self.state.load() {
Err(vm.new_index_error("deque mutated during remove().".to_owned()))
- } else if let Some(index) = index.into() {
+ } else if let Some(index) = index {
let mut deque = self.borrow_deque_mut();
self.state.fetch_add(1);
Ok(deque.remove(index).unwrap())