From f9f665be62bf8768064cbecab54062fad39a3804 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 15 Feb 2023 01:43:21 +0900 Subject: [PATCH 1/3] PyObjectRef::downcast_exact returns PyRefExact --- vm/src/builtins/complex.rs | 2 +- vm/src/builtins/dict.rs | 11 ++++++----- vm/src/builtins/int.rs | 2 +- vm/src/builtins/set.rs | 4 ++-- vm/src/builtins/tuple.rs | 2 +- vm/src/bytesinner.rs | 4 +--- vm/src/object/core.rs | 6 +++--- vm/src/stdlib/builtins.rs | 6 +++++- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/vm/src/builtins/complex.rs b/vm/src/builtins/complex.rs index df81122fd2..3f86b48d19 100644 --- a/vm/src/builtins/complex.rs +++ b/vm/src/builtins/complex.rs @@ -128,7 +128,7 @@ impl Constructor for PyComplex { let val = if cls.is(vm.ctx.types.complex_type) && imag_missing { match val.downcast_exact::(vm) { Ok(c) => { - return Ok(c.into()); + return Ok(c.into_pyref().into()); } Err(val) => val, } diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index ba747bbf25..89f64df994 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -11,7 +11,6 @@ use crate::{ }, class::{PyClassDef, PyClassImpl}, common::ascii, - convert::ToPyObject, dictdatatype::{self, DictKey}, function::{ ArgIterable, FuncArgs, KwArgs, OptionalArg, PyArithmeticValue::*, PyComparisonValue, @@ -24,7 +23,8 @@ use crate::{ IterNextIterable, Iterable, PyComparisonOp, Unconstructible, Unhashable, }, vm::VirtualMachine, - AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, + AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, + TryFromObject, }; use once_cell::sync::Lazy; use rustpython_common::lock::PyMutex; @@ -65,8 +65,9 @@ impl PyDict { // Used in update and ior. pub(crate) fn merge_object(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let other = match other.downcast_exact(vm) { - Ok(dict_other) => return self.merge_dict(dict_other, vm), + let casted: Result, _> = other.downcast_exact(vm); + let other = match casted { + Ok(dict_other) => return self.merge_dict(dict_other.into_pyref(), vm), Err(other) => other, }; let dict = &self.entries; @@ -227,7 +228,7 @@ impl PyDict { for key in iterable.iter(vm)? { pydict.setitem(key?, value.clone(), vm)?; } - Ok(pydict.to_pyobject(vm)) + Ok(pydict.into_pyref().into()) } Err(pyobj) => { for key in iterable.iter(vm)? { diff --git a/vm/src/builtins/int.rs b/vm/src/builtins/int.rs index d7fd2f8de0..a9b6e846ff 100644 --- a/vm/src/builtins/int.rs +++ b/vm/src/builtins/int.rs @@ -231,7 +231,7 @@ impl Constructor for PyInt { let val = if cls.is(vm.ctx.types.int_type) { match val.downcast_exact::(vm) { Ok(i) => { - return Ok(i.to_pyobject(vm)); + return Ok(i.into_pyref().into()); } Err(val) => val, } diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index c2ba6781f7..5b5db30905 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -354,7 +354,7 @@ impl PySetInner { self.merge_set(any_set, vm) // check Dict } else if let Ok(dict) = iterable.to_owned().downcast_exact::(vm) { - self.merge_dict(dict, vm) + self.merge_dict(dict.into_pyref(), vm) } else { // add iterable that is not AnySet or Dict for item in iterable.try_into_value::(vm)?.iter(vm)? { @@ -799,7 +799,7 @@ impl Constructor for PyFrozenSet { let elements = if let OptionalArg::Present(iterable) = iterable { let iterable = if cls.is(vm.ctx.types.frozenset_type) { match iterable.downcast_exact::(vm) { - Ok(fs) => return Ok(fs.into()), + Ok(fs) => return Ok(fs.into_pyref().into()), Err(iterable) => iterable, } } else { diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index b5472f6998..750527384f 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -95,7 +95,7 @@ impl Constructor for PyTuple { let elements = if let OptionalArg::Present(iterable) = iterable { let iterable = if cls.is(vm.ctx.types.tuple_type) { match iterable.downcast_exact::(vm) { - Ok(tuple) => return Ok(tuple.into()), + Ok(tuple) => return Ok(tuple.into_pyref().into()), Err(iterable) => iterable, } } else { diff --git a/vm/src/bytesinner.rs b/vm/src/bytesinner.rs index 8bd4a4fe29..7e1783cba8 100644 --- a/vm/src/bytesinner.rs +++ b/vm/src/bytesinner.rs @@ -79,9 +79,7 @@ impl ByteInnerNewOptions { // construct an exact bytes from an exact bytes do not clone let obj = if cls.is(PyBytes::class(vm)) { match obj.downcast_exact::(vm) { - Ok(b) => { - return Ok(b); - } + Ok(b) => return Ok(b.into_pyref()), Err(obj) => obj, } } else { diff --git a/vm/src/object/core.rs b/vm/src/object/core.rs index 60e3731a66..c8cdd79292 100644 --- a/vm/src/object/core.rs +++ b/vm/src/object/core.rs @@ -12,7 +12,7 @@ //! but not to do to remember it is a PyRef object. use super::{ - ext::{AsObject, PyResult}, + ext::{AsObject, PyRefExact, PyResult}, payload::PyObjectPayload, PyAtomicRef, }; @@ -564,7 +564,7 @@ impl PyObjectRef { pub fn downcast_exact( self, vm: &VirtualMachine, - ) -> Result, Self> { + ) -> Result, Self> { if self.class().is(T::class(vm)) { // TODO: is this always true? assert!( @@ -572,7 +572,7 @@ impl PyObjectRef { "obj.__class__ is T::class() but payload is not T" ); // SAFETY: just asserted that payload_is::() - Ok(unsafe { PyRef::from_obj_unchecked(self) }) + Ok(unsafe { PyRefExact::new_unchecked(PyRef::from_obj_unchecked(self)) }) } else { Err(self) } diff --git a/vm/src/stdlib/builtins.rs b/vm/src/stdlib/builtins.rs index f92d54a6c9..f9832c9f71 100644 --- a/vm/src/stdlib/builtins.rs +++ b/vm/src/stdlib/builtins.rs @@ -872,7 +872,11 @@ mod builtins { // Use downcast_exact to keep ref to old object on error. let metaclass = kwargs .pop_kwarg("metaclass") - .map(|metaclass| metaclass.downcast_exact::(vm)) + .map(|metaclass| { + metaclass + .downcast_exact::(vm) + .map(|m| m.into_pyref()) + }) .unwrap_or_else(|| Ok(vm.ctx.types.type_type.to_owned())); let (metaclass, meta_name) = match metaclass { From 57fd8c97b47486655cefb8cb193c0168dd031815 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 16 Feb 2023 22:31:18 +0900 Subject: [PATCH 2/3] Add description for PyRefExact --- vm/src/object/ext.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/src/object/ext.rs b/vm/src/object/ext.rs index 5bd0779ee2..4e74a4b58b 100644 --- a/vm/src/object/ext.rs +++ b/vm/src/object/ext.rs @@ -107,6 +107,7 @@ impl std::borrow::ToOwned for PyExact { } } +/// PyRef but guaranteed not to be a subtype instance #[derive(Debug)] #[repr(transparent)] pub struct PyRefExact { From 9206ad52b83b333ec26384561a4e2591afbec60c Mon Sep 17 00:00:00 2001 From: Zhiyan Xiao Date: Fri, 17 Feb 2023 07:23:19 +0900 Subject: [PATCH 3/3] Optimize Py::to_attributes() to reuse PyRefExact --- vm/src/builtins/dict.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index 89f64df994..7fd8cac5d1 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -1,6 +1,6 @@ use super::{ set::PySetInner, IterStatus, PositionIterInternal, PyBaseExceptionRef, PyGenericAlias, - PyMappingProxy, PySet, PyStrRef, PyTupleRef, PyType, PyTypeRef, + PyMappingProxy, PySet, PyStr, PyTupleRef, PyType, PyTypeRef, }; use crate::{ atomic_func, @@ -534,9 +534,8 @@ impl Py { pub fn to_attributes(&self, vm: &VirtualMachine) -> PyAttributes { let mut attrs = PyAttributes::default(); for (key, value) in self { - // TODO: use PyRefExact for interning - let key: PyStrRef = key.downcast().expect("dict has non-string keys"); - attrs.insert(vm.ctx.intern_str(key.as_str()), value); + let key: PyRefExact = key.downcast_exact(vm).expect("dict has non-string keys"); + attrs.insert(vm.ctx.intern_str(key), value); } attrs }