From 8d38bf8ded5c3a8b2652e8d4adf323ee53b9baff Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 19 Mar 2023 10:31:23 +0900 Subject: [PATCH 1/2] clean up mmap --- stdlib/src/mmap.rs | 138 ++++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 64 deletions(-) diff --git a/stdlib/src/mmap.rs b/stdlib/src/mmap.rs index b0a72562e..801040693 100644 --- a/stdlib/src/mmap.rs +++ b/stdlib/src/mmap.rs @@ -16,7 +16,7 @@ mod mmap { BufferDescriptor, BufferMethods, PyBuffer, PyMappingMethods, PySequenceMethods, }, sliceable::{SaturatedSlice, SequenceIndex, SequenceIndexOp}, - types::{AsBuffer, AsMapping, AsSequence, Constructor}, + types::{AsBuffer, AsMapping, AsSequence, Constructor, Representable}, AsObject, FromArgs, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromBorrowedObject, VirtualMachine, }; @@ -433,7 +433,7 @@ mod mmap { ass_subscript: atomic_func!(|mapping, needle, value, vm| { let zelf = PyMmap::mapping_downcast(mapping); if let Some(value) = value { - PyMmap::_setitem(zelf.to_owned(), needle, value, vm) + PyMmap::_setitem(zelf, needle, value, vm) } else { Err(vm .new_type_error("mmap object doesn't support item deletion".to_owned())) @@ -456,7 +456,7 @@ mod mmap { ass_item: atomic_func!(|seq, i, value, vm| { let zelf = PyMmap::sequence_downcast(seq); if let Some(value) = value { - PyMmap::setitem_by_index(zelf.to_owned(), i, value, vm) + PyMmap::setitem_by_index(zelf, i, value, vm) } else { Err(vm .new_type_error("mmap object doesn't support item deletion".to_owned())) @@ -468,7 +468,10 @@ mod mmap { } } - #[pyclass(with(Constructor, AsMapping, AsSequence, AsBuffer), flags(BASETYPE))] + #[pyclass( + with(Constructor, AsMapping, AsSequence, AsBuffer, Representable), + flags(BASETYPE) + )] impl PyMmap { fn as_bytes_mut(&self) -> BorrowedValueMut<[u8]> { PyMutexGuard::map(self.mmap.lock(), |m| { @@ -556,32 +559,6 @@ mod mmap { self.closed.load() } - #[pymethod(magic)] - fn repr(zelf: PyRef) -> PyResult { - let mmap = zelf.mmap.lock(); - - if mmap.is_none() { - return Ok("".to_owned()); - } - - let access_str = match zelf.access { - AccessMode::Default => "ACCESS_DEFAULT", - AccessMode::Read => "ACCESS_READ", - AccessMode::Write => "ACCESS_WRITE", - AccessMode::Copy => "ACCESS_COPY", - }; - - let repr = format!( - "", - access_str, - zelf.len(), - zelf.pos(), - zelf.offset - ); - - Ok(repr) - } - #[pymethod] fn close(&self, vm: &VirtualMachine) -> PyResult<()> { if self.closed() { @@ -738,9 +715,12 @@ mod mmap { fn read(&self, n: OptionalArg, vm: &VirtualMachine) -> PyResult { let num_bytes = n .map(|obj| { - let name = obj.class().name().to_string(); + let class = obj.class().to_owned(); obj.try_into_value::>(vm).map_err(|_| { - vm.new_type_error(format!("read argument must be int or None, not {name}",)) + vm.new_type_error(format!( + "read argument must be int or None, not {}", + class.name() + )) }) }) .transpose()? @@ -818,7 +798,7 @@ mod mmap { Ok(result) } - //TODO: supports resize + // TODO: supports resize #[pymethod] fn resize(&self, _newsize: PyIntRef, vm: &VirtualMachine) -> PyResult<()> { self.check_resizeable(vm)?; @@ -924,6 +904,34 @@ mod mmap { Ok(()) } + #[pymethod(magic)] + fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { + self._getitem(&needle, vm) + } + + #[pymethod(magic)] + fn setitem( + zelf: &Py, + needle: PyObjectRef, + value: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult<()> { + Self::_setitem(zelf, &needle, value, vm) + } + + #[pymethod(magic)] + fn enter(zelf: &Py, vm: &VirtualMachine) -> PyResult> { + let _m = zelf.check_valid(vm)?; + Ok(zelf.to_owned()) + } + + #[pymethod(magic)] + fn exit(zelf: &Py, _args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { + zelf.close(vm) + } + } + + impl PyMmap { fn getitem_by_index(&self, i: isize, vm: &VirtualMachine) -> PyResult { let i = i .wrapped_at(self.len()) @@ -984,13 +992,8 @@ mod mmap { } } - #[pymethod(magic)] - fn getitem(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult { - self._getitem(&needle, vm) - } - fn _setitem( - zelf: PyRef, + zelf: &Py, needle: &PyObject, value: PyObjectRef, vm: &VirtualMachine, @@ -1002,18 +1005,18 @@ mod mmap { } fn setitem_by_index( - zelf: PyRef, + &self, i: isize, value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { let i: usize = i - .wrapped_at(zelf.len()) + .wrapped_at(self.len()) .ok_or_else(|| vm.new_index_error("mmap index out of range".to_owned()))?; let b = value_from_object(vm, &value)?; - zelf.try_writable(vm, |mmap| { + self.try_writable(vm, |mmap| { mmap[i] = b; })?; @@ -1021,12 +1024,12 @@ mod mmap { } fn setitem_by_slice( - zelf: PyRef, + &self, slice: &SaturatedSlice, value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - let (range, step, slice_len) = slice.adjust_indices(zelf.len()); + let (range, step, slice_len) = slice.adjust_indices(self.len()); let bytes = bytes_from_object(vm, &value)?; @@ -1038,7 +1041,7 @@ mod mmap { // do nothing Ok(()) } else if step == 1 { - zelf.try_writable(vm, |mmap| { + self.try_writable(vm, |mmap| { (&mut mmap[range]) .write(&bytes) .map_err(|e| vm.new_os_error(e.to_string()))?; @@ -1048,14 +1051,14 @@ mod mmap { let mut bi = 0; // bytes index if step.is_negative() { for i in range.rev().step_by(step.unsigned_abs()) { - zelf.try_writable(vm, |mmap| { + self.try_writable(vm, |mmap| { mmap[i] = bytes[bi]; })?; bi += 1; } } else { for i in range.step_by(step.unsigned_abs()) { - zelf.try_writable(vm, |mmap| { + self.try_writable(vm, |mmap| { mmap[i] = bytes[bi]; })?; bi += 1; @@ -1064,26 +1067,33 @@ mod mmap { Ok(()) } } + } - #[pymethod(magic)] - fn setitem( - zelf: PyRef, - needle: PyObjectRef, - value: PyObjectRef, - vm: &VirtualMachine, - ) -> PyResult<()> { - Self::_setitem(zelf, &needle, value, vm) - } + impl Representable for PyMmap { + #[inline] + fn repr_str(zelf: &Py, _vm: &VirtualMachine) -> PyResult { + let mmap = zelf.mmap.lock(); - #[pymethod(magic)] - fn enter(zelf: PyRef, vm: &VirtualMachine) -> PyResult> { - let _m = zelf.check_valid(vm)?; - Ok(zelf.to_owned()) - } + if mmap.is_none() { + return Ok("".to_owned()); + } - #[pymethod(magic)] - fn exit(zelf: PyRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - zelf.close(vm) + let access_str = match zelf.access { + AccessMode::Default => "ACCESS_DEFAULT", + AccessMode::Read => "ACCESS_READ", + AccessMode::Write => "ACCESS_WRITE", + AccessMode::Copy => "ACCESS_COPY", + }; + + let repr = format!( + "", + access_str, + zelf.len(), + zelf.pos(), + zelf.offset + ); + + Ok(repr) } } } From ccc8f7ed37cf763ee0558f3a27c9be5c020bb0d3 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 19 Mar 2023 10:31:32 +0900 Subject: [PATCH 2/2] clean up name() usage --- vm/src/builtins/str.rs | 5 +---- vm/src/cformat.rs | 4 ++-- vm/src/object/core.rs | 15 +++++++-------- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/vm/src/builtins/str.rs b/vm/src/builtins/str.rs index 32b86f0e6..ed3efbb5e 100644 --- a/vm/src/builtins/str.rs +++ b/vm/src/builtins/str.rs @@ -1653,10 +1653,7 @@ mod tests { let translated = text.translate(translated, vm).unwrap(); assert_eq!(translated, "🎅xda".to_owned()); let translated = text.translate(vm.ctx.new_int(3).into(), vm); - assert_eq!( - translated.unwrap_err().class().name().deref(), - "TypeError".to_owned() - ); + assert_eq!("TypeError", &*translated.unwrap_err().class().name(),); }) } } diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 4deb874d8..a0e4c8c2f 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -84,12 +84,12 @@ fn spec_format_bytes( } }, CFormatType::Float(_) => { - let type_name = obj.class().name().to_string(); + let class = obj.class().to_owned(); let value = ArgIntoFloat::try_from_object(vm, obj).map_err(|e| { if e.fast_isinstance(vm.ctx.exceptions.type_error) { // formatfloat in bytesobject.c generates its own specific exception // text in this case, mirror it here. - vm.new_type_error(format!("float argument required, not {type_name}")) + vm.new_type_error(format!("float argument required, not {}", class.name())) } else { e } diff --git a/vm/src/object/core.rs b/vm/src/object/core.rs index 1de24bdec..f7cce8851 100644 --- a/vm/src/object/core.rs +++ b/vm/src/object/core.rs @@ -611,14 +611,13 @@ impl PyObject { None }; let cls_is_weakref = typ.is(vm.ctx.types.weakref_type); - self.weak_ref_list() - .map(|wrl| wrl.add(self, typ, cls_is_weakref, callback, dict)) - .ok_or_else(|| { - vm.new_type_error(format!( - "cannot create weak reference to '{}' object", - self.class().name() - )) - }) + let wrl = self.weak_ref_list().ok_or_else(|| { + vm.new_type_error(format!( + "cannot create weak reference to '{}' object", + self.class().name() + )) + })?; + Ok(wrl.add(self, typ, cls_is_weakref, callback, dict)) } pub fn downgrade(