Merge pull request RustPython#3338 from AP2008/relocate-set_attrs

Relocate `vm.set_attr` to `obj.set_attr`

Author: DimitrisJim

vm/src/builtins/namespace.rs

@@ -24,7 +24,7 @@ impl Constructor for PyNamespace {
     fn py_new(cls: PyTypeRef, kwargs: Self::Args, vm: &VirtualMachine) -> PyResult {
         let zelf = PyNamespace.into_ref_with_type(vm, cls)?;
         for (name, value) in kwargs.into_iter() {
-            vm.set_attr(zelf.as_object(), name, value)?;
+            zelf.as_object().set_attr(name, value, vm)?;
         }
         Ok(zelf.into_pyobject(vm))
     }

vm/src/builtins/weakproxy.rs

@@ -66,7 +66,7 @@ impl SetAttr for PyWeakProxy {
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         match zelf.weak.upgrade() {
-            Some(obj) => vm.call_set_attr(&obj, attr_name, value),
+            Some(obj) => obj.call_set_attr(vm, attr_name, value),
             None => Err(vm.new_exception_msg(
                 vm.ctx.exceptions.reference_error.clone(),
                 "weakly-referenced object no longer exists".to_owned(),

vm/src/exceptions.rs

@@ -1079,16 +1079,16 @@ pub(super) mod types {
         args: FuncArgs,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        let exc_self = zelf.into();
-        vm.set_attr(
-            &exc_self,
+        let zelf: PyObjectRef = zelf.into();
+        zelf.set_attr(
             "name",
             vm.unwrap_or_none(args.kwargs.get("name").cloned()),
+            vm,
         )?;
-        vm.set_attr(
-            &exc_self,
+        zelf.set_attr(
             "path",
             vm.unwrap_or_none(args.kwargs.get("path").cloned()),
+            vm,
         )?;
         Ok(())
     }

vm/src/frame.rs

@@ -1651,14 +1651,14 @@ impl ExecutingFrame<'_> {
         )
         .into_object(vm);
 
-        vm.set_attr(&func_obj, "__doc__", vm.ctx.none())?;
+        func_obj.set_attr("__doc__", vm.ctx.none(), vm)?;
 
         let name = qualified_name.as_str().split('.').next_back().unwrap();
-        vm.set_attr(&func_obj, "__name__", vm.new_pyobj(name))?;
-        vm.set_attr(&func_obj, "__qualname__", qualified_name)?;
+        func_obj.set_attr("__name__", vm.new_pyobj(name), vm)?;
+        func_obj.set_attr("__qualname__", qualified_name, vm)?;
         let module = vm.unwrap_or_none(self.globals.get_item_option("__name__", vm)?);
-        vm.set_attr(&func_obj, "__module__", module)?;
-        vm.set_attr(&func_obj, "__annotations__", annotations)?;
+        func_obj.set_attr("__module__", module, vm)?;
+        func_obj.set_attr("__annotations__", annotations, vm)?;
 
         self.push_value(func_obj);
         Ok(None)
@@ -1803,7 +1803,7 @@ impl ExecutingFrame<'_> {
         let attr_name = self.code.names[attr as usize].clone();
         let parent = self.pop_value();
         let value = self.pop_value();
-        vm.set_attr(&parent, attr_name, value)?;
+        parent.set_attr(attr_name, value, vm)?;
         Ok(None)
     }
 

vm/src/import.rs

@@ -57,7 +57,7 @@ pub(crate) fn init_importlib(
                 magic = rand::thread_rng().gen::<[u8; 4]>().to_vec();
             }
             let magic: PyObjectRef = vm.ctx.new_bytes(magic).into();
-            vm.set_attr(&importlib_external, "MAGIC_NUMBER", magic)?;
+            importlib_external.set_attr("MAGIC_NUMBER", magic, vm)?;
             let zipimport_res = (|| -> PyResult<()> {
                 let zipimport = vm.import("zipimport", None, 0)?;
                 let zipimporter = zipimport.get_attr("zipimporter", vm)?;

vm/src/protocol/object.rs

@@ -35,6 +35,30 @@ impl PyObjectRef {
         getattro(self, attr_name, vm)
     }
 
+    pub fn call_set_attr(
+        &self,
+        vm: &VirtualMachine,
+        attr_name: PyStrRef,
+        attr_value: Option<PyObjectRef>,
+    ) -> PyResult<()> {
+        let setattro = {
+            let cls = self.class();
+            cls.mro_find_map(|cls| cls.slots.setattro.load())
+                .ok_or_else(|| {
+                    let assign = attr_value.is_some();
+                    let has_getattr = cls.mro_find_map(|cls| cls.slots.getattro.load()).is_some();
+                    vm.new_type_error(format!(
+                        "'{}' object has {} attributes ({} {})",
+                        cls.name(),
+                        if has_getattr { "only read-only" } else { "no" },
+                        if assign { "assign to" } else { "del" },
+                        attr_name
+                    ))
+                })?
+        };
+        setattro(self, attr_name, attr_value, vm)
+    }
+
     // PyObject *PyObject_GenericGetAttr(PyObject *o, PyObject *name)
 
     pub fn set_attr(
@@ -43,7 +67,8 @@ impl PyObjectRef {
         attr_value: impl Into<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        vm.set_attr(self, attr_name, attr_value)
+        let attr_name = attr_name.into_pystr_ref(vm);
+        self.call_set_attr(vm, attr_name, Some(attr_value.into()))
     }
 
     // int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value)

vm/src/stdlib/ast.rs

@@ -59,7 +59,7 @@ impl AstNode {
             )));
         }
         for (name, arg) in fields.iter().zip(args.args) {
-            vm.set_attr(&zelf, name.clone(), arg)?;
+            zelf.set_attr(name.clone(), arg, vm)?;
         }
         for (key, value) in args.kwargs {
             if let Some(pos) = fields.iter().position(|f| f.as_str() == key) {
@@ -71,7 +71,7 @@ impl AstNode {
                     )));
                 }
             }
-            vm.set_attr(&zelf, key, value)?;
+            zelf.set_attr(key, value, vm)?;
         }
         Ok(())
     }

vm/src/stdlib/builtins.rs

@@ -732,7 +732,7 @@ mod builtins {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        vm.set_attr(&obj, attr, value)?;
+        obj.set_attr(attr, value, vm)?;
         Ok(())
     }
 

vm/src/stdlib/errno.rs

@@ -16,7 +16,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
         errorcode
             .set_item(code.clone(), name.clone().into(), vm)
             .unwrap();
-        vm.set_attr(&module, name, code).unwrap();
+        module.set_attr(name, code, vm).unwrap();
     }
     module
 }

vm/src/stdlib/io.rs

@@ -542,7 +542,7 @@ mod _io {
     pub(super) fn iobase_close(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         if !file_closed(file, vm)? {
             let res = vm.call_method(file, "flush", ());
-            vm.set_attr(file, "__closed", vm.new_pyobj(true))?;
+            file.set_attr("__closed", vm.new_pyobj(true), vm)?;
             res?;
         }
         Ok(())
@@ -3622,7 +3622,7 @@ mod _io {
                         line_buffering,
                     ),
                 )?;
-                vm.set_attr(&wrapper, "mode", vm.new_pyobj(mode_string))?;
+                wrapper.set_attr("mode", vm.new_pyobj(mode_string), vm)?;
                 Ok(wrapper)
             }
             EncodeMode::Bytes => Ok(buffered),
@@ -3876,7 +3876,7 @@ mod fileio {
             zelf.closefd.store(args.closefd);
             #[cfg(windows)]
             crate::stdlib::msvcrt::setmode_binary(fd);
-            vm.set_attr(zelf.as_object(), "name", name)?;
+            zelf.as_object().set_attr("name", name, vm)?;
             Ok(())
         }
 

vm/src/stdlib/os.rs

@@ -296,11 +296,13 @@ impl IntoPyException for IOErrorBuilder {
         let excp = self.error.into_pyexception(vm);
 
         if let Some(filename) = self.filename {
-            vm.set_attr(excp.as_object(), "filename", filename.filename(vm))
+            excp.as_object()
+                .set_attr("filename", filename.filename(vm), vm)
                 .unwrap();
         }
         if let Some(filename2) = self.filename2 {
-            vm.set_attr(excp.as_object(), "filename2", filename2.filename(vm))
+            excp.as_object()
+                .set_attr("filename2", filename2.filename(vm), vm)
                 .unwrap();
         }
         excp

vm/src/stdlib/sys.rs

@@ -289,11 +289,11 @@ mod sys {
             return Ok(());
         }
         // set to none to avoid recursion while printing
-        vm.set_attr(&vm.builtins, "_", vm.ctx.none())?;
+        vm.builtins.set_attr("_", vm.ctx.none(), vm)?;
         // TODO: catch encoding errors
         let repr = vm.to_repr(&obj)?.into();
         builtins::print(PosArgs::new(vec![repr]), Default::default(), vm)?;
-        vm.set_attr(&vm.builtins, "_", obj)?;
+        vm.builtins.set_attr("_", obj, vm)?;
         Ok(())
     }
 

vm/src/vm.rs

@@ -360,20 +360,20 @@ impl VirtualMachine {
                         Default::default(),
                         self,
                     )?;
-                    self.set_attr(
-                        &self.sys_module,
+                    self.sys_module.set_attr(
                         format!("__{}__", name), // e.g. __stdin__
                         stdio.clone(),
+                        self,
                     )?;
-                    self.set_attr(&self.sys_module, name, stdio)?;
+                    self.sys_module.set_attr(name, stdio, self)?;
                     Ok(())
                 };
                 set_stdio("stdin", 0, "r")?;
                 set_stdio("stdout", 1, "w")?;
                 set_stdio("stderr", 2, "w")?;
 
                 let io_open = io.get_attr("open", self)?;
-                self.set_attr(&self.builtins, "open", io_open)?;
+                self.builtins.set_attr("open", io_open, self)?;
             }
 
             Ok(())
@@ -778,29 +778,34 @@ impl VirtualMachine {
         let syntax_error = self.new_exception_msg(syntax_error_type, error.to_string());
         let lineno = self.ctx.new_int(error.location.row());
         let offset = self.ctx.new_int(error.location.column());
-        self.set_attr(syntax_error.as_object(), "lineno", lineno)
+        syntax_error
+            .as_object()
+            .set_attr("lineno", lineno, self)
             .unwrap();
-        self.set_attr(syntax_error.as_object(), "offset", offset)
+        syntax_error
+            .as_object()
+            .set_attr("offset", offset, self)
+            .unwrap();
+        syntax_error
+            .as_object()
+            .set_attr("text", error.statement.clone().into_pyobject(self), self)
+            .unwrap();
+        syntax_error
+            .as_object()
+            .set_attr(
+                "filename",
+                self.ctx.new_str(error.source_path.clone()),
+                self,
+            )
             .unwrap();
-        self.set_attr(
-            syntax_error.as_object(),
-            "text",
-            error.statement.clone().into_pyobject(self),
-        )
-        .unwrap();
-        self.set_attr(
-            syntax_error.as_object(),
-            "filename",
-            self.ctx.new_str(error.source_path.clone()),
-        )
-        .unwrap();
         syntax_error
     }
 
     pub fn new_import_error(&self, msg: String, name: impl IntoPyStrRef) -> PyBaseExceptionRef {
         let import_error = self.ctx.exceptions.import_error.clone();
         let exc = self.new_exception_msg(import_error, msg);
-        self.set_attr(exc.as_object(), "name", name.into_pystr_ref(self))
+        exc.as_object()
+            .set_attr("name", name.into_pystr_ref(self), self)
             .unwrap();
         exc
     }
@@ -1408,42 +1413,9 @@ impl VirtualMachine {
         }
     }
 
-    pub fn call_set_attr(
-        &self,
-        obj: &PyObjectRef,
-        attr_name: PyStrRef,
-        attr_value: Option<PyObjectRef>,
-    ) -> PyResult<()> {
-        let setattro = {
-            let cls = obj.class();
-            cls.mro_find_map(|cls| cls.slots.setattro.load())
-                .ok_or_else(|| {
-                    let assign = attr_value.is_some();
-                    let has_getattr = cls.mro_find_map(|cls| cls.slots.getattro.load()).is_some();
-                    self.new_type_error(format!(
-                        "'{}' object has {} attributes ({} {})",
-                        cls.name(),
-                        if has_getattr { "only read-only" } else { "no" },
-                        if assign { "assign to" } else { "del" },
-                        attr_name
-                    ))
-                })?
-        };
-        setattro(obj, attr_name, attr_value, self)
-    }
-
-    pub fn set_attr<K, V>(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult<()>
-    where
-        K: IntoPyStrRef,
-        V: Into<PyObjectRef>,
-    {
-        let attr_name = attr_name.into_pystr_ref(self);
-        self.call_set_attr(obj, attr_name, Some(attr_value.into()))
-    }
-
     pub fn del_attr(&self, obj: &PyObjectRef, attr_name: impl IntoPyStrRef) -> PyResult<()> {
         let attr_name = attr_name.into_pystr_ref(self);
-        self.call_set_attr(obj, attr_name, None)
+        obj.call_set_attr(self, attr_name, None)
     }
 
     // get_method should be used for internal access to magic methods (by-passing

wasm/lib/src/vm_class.rs

@@ -246,7 +246,7 @@ impl WASMVirtualMachine {
             } else {
                 return Err(error());
             };
-            vm.set_attr(&vm.sys_module, "stdout", stdout).unwrap();
+            vm.sys_module.set_attr("stdout", stdout, vm).unwrap();
             Ok(())
         })?
     }