From b8f276f8d76a3c02ccaca19dc638bc545abb5cf7 Mon Sep 17 00:00:00 2001 From: dldks321 <80047618+dldks321@users.noreply.github.com> Date: Fri, 24 Jun 2022 01:14:50 +0900 Subject: [PATCH] Fix OverflowError at collections.rs (#3815) --- Lib/test/seq_tests.py | 2 -- .../snippets/stdlib_collections_deque.py | 4 +++ vm/src/builtins/tuple.rs | 29 +++++++------------ vm/src/stdlib/collections.rs | 25 +++++++++------- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py index dd0bc5435..e73f25be9 100644 --- a/Lib/test/seq_tests.py +++ b/Lib/test/seq_tests.py @@ -362,8 +362,6 @@ class CommonTest(unittest.TestCase): self.assertRaises(BadExc, a.count, BadCmp()) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_index(self): u = self.type2test([0, 1]) self.assertEqual(u.index(0), 0) diff --git a/extra_tests/snippets/stdlib_collections_deque.py b/extra_tests/snippets/stdlib_collections_deque.py index 56655bccc..00331ac88 100644 --- a/extra_tests/snippets/stdlib_collections_deque.py +++ b/extra_tests/snippets/stdlib_collections_deque.py @@ -1,3 +1,4 @@ +from testutils import assert_raises from collections import deque from typing import Deque @@ -93,3 +94,6 @@ class D(deque): assert repr(D()) == "D([])" assert repr(D([1, 2, 3])) == "D([1, 2, 3])" + + +assert_raises(ValueError, lambda: deque().index(10,0,10000000000000000000000000)) \ No newline at end of file diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index 6078c07b9..35cffab07 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -1,4 +1,4 @@ -use super::{PositionIterInternal, PyGenericAlias, PyType, PyTypeRef}; +use super::{PositionIterInternal, PyGenericAlias, PyIntRef, PyType, PyTypeRef}; use crate::common::{hash::PyHash, lock::PyMutex}; use crate::{ class::PyClassImpl, @@ -8,8 +8,8 @@ use crate::{ protocol::{PyIterReturn, PyMappingMethods, PySequenceMethods}, recursion::ReprGuard, sequence::SequenceExt, + sliceable::pyint_saturate_index, sliceable::{SequenceIndex, SliceableSequenceOp}, - stdlib::sys, types::{ AsMapping, AsSequence, Comparable, Constructor, Hashable, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unconstructible, @@ -281,24 +281,17 @@ impl PyTuple { fn index( &self, needle: PyObjectRef, - start: OptionalArg, - stop: OptionalArg, + start: OptionalArg, + stop: OptionalArg, vm: &VirtualMachine, ) -> PyResult { - let mut start = start.into_option().unwrap_or(0); - if start < 0 { - start += self.len() as isize; - if start < 0 { - start = 0; - } - } - let mut stop = stop.into_option().unwrap_or(sys::MAXSIZE); - if stop < 0 { - stop += self.len() as isize; - if stop < 0 { - stop = 0; - } - } + let len = self.len(); + let saturate = |obj: PyObjectRef, len| -> PyResult<_> { + obj.try_into_value(vm) + .map(|int: PyIntRef| pyint_saturate_index(int, len)) + }; + let start = start.map_or(Ok(0), |i| saturate(i, len))?; + let stop = stop.map_or(Ok(len), |i| saturate(i, len))?; for (index, element) in self .elements .iter() diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs index da77d399d..c3b877c60 100644 --- a/vm/src/stdlib/collections.rs +++ b/vm/src/stdlib/collections.rs @@ -5,7 +5,7 @@ mod _collections { use crate::{ builtins::{ IterStatus::{Active, Exhausted}, - PositionIterInternal, PyGenericAlias, PyInt, PyTypeRef, + PositionIterInternal, PyGenericAlias, PyInt, PyIntRef, PyTypeRef, }, common::lock::{PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard}, function::{FuncArgs, KwArgs, OptionalArg, PyComparisonValue}, @@ -14,7 +14,7 @@ mod _collections { recursion::ReprGuard, sequence::MutObjectSequenceOp, sliceable, - sliceable::saturate_index, + sliceable::pyint_saturate_index, types::{ AsSequence, Comparable, Constructor, Hashable, Initializer, IterNext, IterNextIterable, Iterable, PyComparisonOp, Unhashable, @@ -157,26 +157,29 @@ mod _collections { #[pymethod] fn index( &self, - obj: PyObjectRef, - start: OptionalArg, - stop: OptionalArg, + needle: PyObjectRef, + start: OptionalArg, + stop: OptionalArg, vm: &VirtualMachine, ) -> PyResult { let start_state = self.state.load(); let len = self.len(); - let start = start.map(|i| saturate_index(i, len)).unwrap_or(0); - let stop = stop - .map(|i| saturate_index(i, len)) - .unwrap_or(isize::MAX as usize); - let index = self.mut_index_range(vm, &obj, start..stop)?; + let saturate = |obj: PyObjectRef, len| -> PyResult<_> { + obj.try_into_value(vm) + .map(|int: PyIntRef| pyint_saturate_index(int, len)) + }; + let start = start.map_or(Ok(0), |i| saturate(i, len))?; + let stop = stop.map_or(Ok(len), |i| saturate(i, len))?; + 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() { Ok(index) } else { Err(vm.new_value_error( - obj.repr(vm) + needle + .repr(vm) .map(|repr| format!("{} is not in deque", repr)) .unwrap_or_else(|_| String::new()), ))