getattr to take &Py<PyStr>

Author: youknowone

vm/src/builtins/genericalias.rs

@@ -393,7 +393,7 @@ impl GetAttr for PyGenericAlias {
     fn getattro(zelf: &Py<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
         for exc in ATTR_EXCEPTIONS.iter() {
             if *(*exc) == attr.to_string() {
-                return zelf.as_object().generic_getattr(attr, vm);
+                return zelf.as_object().generic_getattr(&attr, vm);
             }
         }
         zelf.origin().get_attr(attr, vm)

vm/src/builtins/module.rs

@@ -26,7 +26,6 @@ pub struct ModuleInitArgs {
     doc: Option<PyStrRef>,
 }
 
-#[pyclass(with(GetAttr, Initializer, Representable), flags(BASETYPE, HAS_DICT))]
 impl PyModule {
     // pub(crate) fn new(d: PyDictRef) -> Self {
     //     PyModule { dict: d.into() }
@@ -37,49 +36,31 @@ impl PyModule {
     //     self.dict.get()
     // }
 
-    #[pyslot]
-    fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
-        PyModule {}.into_ref_with_type(vm, cls).map(Into::into)
-    }
-
     fn getattr_inner(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(attr) = zelf
-            .as_object()
-            .generic_getattr_opt(name.clone(), None, vm)?
-        {
+        if let Some(attr) = zelf.as_object().generic_getattr_opt(&name, None, vm)? {
             return Ok(attr);
         }
         if let Ok(getattr) = zelf.dict().get_item(identifier!(vm, __getattr__), vm) {
             return getattr.call((name,), vm);
         }
-        let module_name = if let Some(name) = Self::name(zelf.to_owned(), vm) {
+        let module_name = if let Some(name) = zelf.name(vm) {
             format!(" '{name}'")
         } else {
             "".to_owned()
         };
         Err(vm.new_attribute_error(format!("module{module_name} has no attribute '{name}'")))
     }
+}
 
-    fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<PyStrRef> {
-        let name = zelf
+impl Py<PyModule> {
+    fn name(&self, vm: &VirtualMachine) -> Option<PyStrRef> {
+        let name = self
             .as_object()
-            .generic_getattr_opt(identifier!(vm, __name__).to_owned(), None, vm)
+            .generic_getattr_opt(identifier!(vm, __name__), None, vm)
             .unwrap_or_default()?;
         name.downcast::<PyStr>().ok()
     }
 
-    #[pymethod(magic)]
-    fn dir(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
-        let dict = zelf
-            .as_object()
-            .dict()
-            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
-        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
-        Ok(attrs)
-    }
-}
-
-impl Py<PyModule> {
     // TODO: to be replaced by the commented-out dict method above once dictoffsets land
     pub fn dict(&self) -> PyDictRef {
         self.as_object().dict().unwrap()
@@ -117,6 +98,24 @@ impl Py<PyModule> {
     }
 }
 
+#[pyclass(with(GetAttr, Initializer, Representable), flags(BASETYPE, HAS_DICT))]
+impl PyModule {
+    #[pyslot]
+    fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
+        PyModule {}.into_ref_with_type(vm, cls).map(Into::into)
+    }
+
+    #[pymethod(magic)]
+    fn dir(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
+        let dict = zelf
+            .as_object()
+            .dict()
+            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
+        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
+        Ok(attrs)
+    }
+}
+
 impl Initializer for PyModule {
     type Args = ModuleInitArgs;
 

vm/src/builtins/object.rs

@@ -298,7 +298,7 @@ impl PyBaseObject {
     #[pyslot]
     pub(crate) fn getattro(obj: &PyObject, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
         vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
-        obj.as_object().generic_getattr(name, vm)
+        obj.as_object().generic_getattr(&name, vm)
     }
 
     #[pymethod(magic)]

vm/src/builtins/super.rs

@@ -130,13 +130,13 @@ impl GetAttr for PySuper {
         let skip = |zelf: &Py<Self>, name| zelf.as_object().generic_getattr(name, vm);
         let (obj, start_type): (PyObjectRef, PyTypeRef) = match zelf.obj.clone() {
             Some(o) => o,
-            None => return skip(zelf, name),
+            None => return skip(zelf, &name),
         };
         // We want __class__ to return the class of the super object
         // (i.e. super, or a subclass), not the class of su->obj.
 
         if name.as_str() == "__class__" {
-            return skip(zelf, name);
+            return skip(zelf, &name);
         }
 
         if let Some(name) = vm.ctx.interned_str(&*name) {
@@ -159,7 +159,7 @@ impl GetAttr for PySuper {
                 }
             }
         }
-        skip(zelf, name)
+        skip(zelf, &name)
     }
 }
 

vm/src/builtins/union.rs

@@ -304,7 +304,7 @@ impl GetAttr for PyUnion {
     fn getattro(zelf: &Py<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
         for &exc in CLS_ATTRS {
             if *exc == attr.to_string() {
-                return zelf.as_object().generic_getattr(attr, vm);
+                return zelf.as_object().generic_getattr(&attr, vm);
             }
         }
         zelf.as_object().to_pyobject(vm).get_attr(attr, vm)

vm/src/intern.rs

@@ -164,6 +164,13 @@ impl<T: PyPayload> std::hash::Hash for PyInterned<T> {
     }
 }
 
+impl<T: PyPayload> AsRef<Py<T>> for PyInterned<T> {
+    #[inline(always)]
+    fn as_ref(&self) -> &Py<T> {
+        &self.inner
+    }
+}
+
 impl<T: PyPayload> Deref for PyInterned<T> {
     type Target = Py<T>;
     #[inline(always)]

vm/src/protocol/object.rs

@@ -13,7 +13,7 @@ use crate::{
     function::{Either, OptionalArg, PyArithmeticValue, PySetterValue},
     protocol::{PyIter, PyMapping, PySequence},
     types::{Constructor, PyComparisonOp},
-    AsObject, PyObject, PyObjectRef, PyResult, TryFromObject, VirtualMachine,
+    AsObject, Py, PyObject, PyObjectRef, PyResult, TryFromObject, VirtualMachine,
 };
 
 // RustPython doesn't need these items
@@ -182,27 +182,26 @@ impl PyObject {
         }
     }
 
-    pub fn generic_getattr(&self, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        self.generic_getattr_opt(name.clone(), None, vm)?
-            .ok_or_else(|| {
-                vm.new_attribute_error(format!(
-                    "'{}' object has no attribute '{}'",
-                    self.class().name(),
-                    name
-                ))
-            })
+    pub fn generic_getattr(&self, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
+        self.generic_getattr_opt(name, None, vm)?.ok_or_else(|| {
+            vm.new_attribute_error(format!(
+                "'{}' object has no attribute '{}'",
+                self.class().name(),
+                name
+            ))
+        })
     }
 
     /// CPython _PyObject_GenericGetAttrWithDict
     pub fn generic_getattr_opt(
         &self,
-        name_str: PyStrRef,
+        name_str: &Py<PyStr>,
         dict: Option<PyDictRef>,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
         let name = name_str.as_str();
         let obj_cls = self.class();
-        let cls_attr_name = vm.ctx.interned_str(&*name_str);
+        let cls_attr_name = vm.ctx.interned_str(name_str);
         let cls_attr = match cls_attr_name.and_then(|name| obj_cls.get_attr(name)) {
             Some(descr) => {
                 let descr_cls = descr.class();

vm/src/stdlib/thread.rs

@@ -366,7 +366,7 @@ pub(crate) mod _thread {
                 Ok(ldict.into())
             } else {
                 zelf.as_object()
-                    .generic_getattr_opt(attr.clone(), Some(ldict), vm)?
+                    .generic_getattr_opt(&attr, Some(ldict), vm)?
                     .ok_or_else(|| {
                         vm.new_attribute_error(format!(
                             "{} has no attribute '{}'",