Merge pull request RustPython#2336 from RustPython/coolreader18/pyobjectref-custom-vtable

Manually implement a vtable for PyObjectRef

Author: coolreader18

.github/workflows/ci.yaml

@@ -161,6 +161,21 @@ jobs:
         run: python -m pip install flake8
       - name: run lint
         run: flake8 . --count --exclude=./.*,./Lib,./vm/Lib  --select=E9,F63,F7,F82 --show-source --statistics
+  miri:
+    name: Run tests under miri
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@master
+      - uses: actions-rs/toolchain@v1
+        with:
+            profile: minimal
+            toolchain: nightly
+            components: miri
+            override: true
+      - name: Run tests under miri
+        # miri-ignore-leaks because the type-object circular reference means that there will always be
+        # a memory leak, at least until we have proper cyclic gc
+        run: MIRIFLAGS='-Zmiri-ignore-leaks' cargo +nightly miri test -p rustpython-vm -- miri_test
 
   wasm:
     name: Check the WASM package and demo

Cargo.lock

@@ -79,6 +79,7 @@ cfg-if = "0.1.10"
 timsort = "0.1"
 thiserror = "1.0"
 atty = "0.2"
+static_assertions = "1.1"
 
 ## unicode stuff
 unicode_names2 = "0.4"

vm/Cargo.toml

@@ -198,7 +198,7 @@ impl PyCode {}
 #[pyimpl]
 impl PyCodeRef {
     #[pyslot]
-    fn tp_new(_cls: PyTypeRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
+    fn tp_new(_cls: PyTypeRef, vm: &VirtualMachine) -> PyResult<Self> {
         Err(vm.new_type_error("Cannot directly create code object".to_owned()))
     }
 

vm/src/builtins/code.rs

@@ -597,7 +597,7 @@ macro_rules! dict_iterator {
                 let s = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {
                     let mut str_parts = vec![];
                     for (key, value) in zelf.dict.clone() {
-                        let s = vm.to_repr(&$result_fn(vm, key, value))?;
+                        let s = vm.to_repr(&($result_fn)(vm, key, value))?;
                         str_parts.push(s.borrow_value().to_owned());
                     }
                     format!("{}([{}])", $class_name, str_parts.join(", "))
@@ -694,7 +694,7 @@ macro_rules! dict_iterator {
                 match zelf.dict.entries.next_entry(&mut position) {
                     Some((key, value)) => {
                         zelf.position.store(position);
-                        Ok($result_fn(vm, key, value))
+                        Ok(($result_fn)(vm, key, value))
                     }
                     None => Err(vm.new_stop_iteration()),
                 }
@@ -743,7 +743,7 @@ macro_rules! dict_iterator {
                 match zelf.dict.len().checked_sub(count) {
                     Some(mut pos) => {
                         let (key, value) = zelf.dict.entries.next_entry(&mut pos).unwrap();
-                        Ok($result_fn(vm, key, value))
+                        Ok(($result_fn)(vm, key, value))
                     }
                     None => {
                         zelf.position.store(std::isize::MAX as usize);

vm/src/builtins/dict.rs

@@ -217,10 +217,10 @@ impl PyBaseObject {
     #[pyproperty(name = "__class__", setter)]
     fn set_class(instance: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         if instance.payload_is::<PyBaseObject>() {
-            match value.downcast_generic::<PyType>() {
+            match value.downcast::<PyType>() {
                 Ok(cls) => {
                     // FIXME(#1979) cls instances might have a payload
-                    *instance.typ.write() = cls;
+                    *instance.class_lock().write() = cls;
                     Ok(())
                 }
                 Err(value) => {

vm/src/builtins/object.rs

@@ -240,7 +240,10 @@ impl PyStr {
         if other.isinstance(&vm.ctx.types.str_type) {
             Ok(self.value.py_add(borrow_value(&other)))
         } else {
-            Err(vm.new_type_error(format!("Cannot add {} and {}", self, other)))
+            Err(vm.new_type_error(format!(
+                "can only concatenate str (not \"{}\") to str",
+                other.class().name
+            )))
         }
     }
 

vm/src/builtins/pystr.rs

@@ -727,24 +727,22 @@ pub fn tp_new_wrapper(
     call_tp_new(zelf, cls, args, vm)
 }
 
-fn take_next_base(mut bases: Vec<Vec<PyTypeRef>>) -> (Option<PyTypeRef>, Vec<Vec<PyTypeRef>>) {
-    bases = bases.into_iter().filter(|x| !x.is_empty()).collect();
-
-    for base in &bases {
+fn take_next_base(bases: &mut Vec<Vec<PyTypeRef>>) -> Option<PyTypeRef> {
+    for base in bases.iter() {
         let head = base[0].clone();
-        if !(&bases).iter().any(|x| x[1..].iter().any(|x| x.is(&head))) {
+        if !bases.iter().any(|x| x[1..].iter().any(|x| x.is(&head))) {
             // Remove from other heads.
-            for item in &mut bases {
+            for item in bases.iter_mut() {
                 if item[0].is(&head) {
                     item.remove(0);
                 }
             }
 
-            return (Some(head), bases);
+            return Some(head);
         }
     }
 
-    (None, bases)
+    None
 }
 
 fn linearise_mro(mut bases: Vec<Vec<PyTypeRef>>) -> Result<Vec<PyTypeRef>, String> {
@@ -768,22 +766,19 @@ fn linearise_mro(mut bases: Vec<Vec<PyTypeRef>>) -> Result<Vec<PyTypeRef>, Strin
     }
 
     let mut result = vec![];
-    loop {
-        if (&bases).iter().all(Vec::is_empty) {
-            break;
-        }
-        let (head, new_bases) = take_next_base(bases);
-        let head = head.ok_or_else(|| {
+    while !bases.is_empty() {
+        let head = take_next_base(&mut bases).ok_or_else(|| {
             // Take the head class of each class here. Now that we have reached the problematic bases.
             // Because this failed, we assume the lists cannot be empty.
             format!(
                 "Cannot create a consistent method resolution order (MRO) for bases {}",
-                new_bases.iter().map(|x| x.first().unwrap()).join(", ")
+                bases.iter().map(|x| x.first().unwrap()).format(", ")
             )
-        });
+        })?;
+
+        result.push(head);
 
-        result.push(head.unwrap());
-        bases = new_bases;
+        bases.retain(|x| !x.is_empty());
     }
     Ok(result)
 }

vm/src/builtins/pytype.rs

@@ -2,9 +2,9 @@ use super::pytype::PyTypeRef;
 use crate::common::hash::PyHash;
 use crate::function::{FuncArgs, OptionalArg};
 use crate::pyobject::{
-    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyObjectWeak, PyRef, PyResult, PyValue,
+    TypeProtocol,
 };
-use crate::pyobjectrc::{PyObjectRc, PyObjectWeak};
 use crate::slots::{Callable, Comparable, Hashable, PyComparisonOp};
 use crate::vm::VirtualMachine;
 
@@ -20,7 +20,7 @@ pub struct PyWeak {
 impl PyWeak {
     pub fn downgrade(obj: &PyObjectRef) -> PyWeak {
         PyWeak {
-            referent: PyObjectRc::downgrade(obj),
+            referent: PyObjectRef::downgrade(obj),
             hash: AtomicCell::new(None),
         }
     }

vm/src/builtins/weakref.rs

@@ -14,8 +14,8 @@ use crate::byteslike::try_bytes_like;
 use crate::cformat::CFormatBytes;
 use crate::function::{OptionalArg, OptionalOption};
 use crate::pyobject::{
-    BorrowValue, Either, IdProtocol, PyComparisonValue, PyIterable, PyObjectRef, PyRef, PyResult,
-    PyValue, TryFromObject, TypeProtocol,
+    BorrowValue, Either, IdProtocol, PyComparisonValue, PyIterable, PyObjectRef, PyResult, PyValue,
+    TryFromObject, TypeProtocol,
 };
 use crate::sliceable::PySliceableSequence;
 use crate::slots::PyComparisonOp;
@@ -91,10 +91,10 @@ impl ByteInnerNewOptions {
 
     pub fn get_bytes(mut self, cls: PyTypeRef, vm: &VirtualMachine) -> PyResult<PyBytesRef> {
         let inner = if let OptionalArg::Present(source) = self.source.take() {
-            if source.class().is(&PyBytes::class(vm)) && cls.is(&PyBytes::class(vm)) {
-                return self
-                    .check_args(vm)
-                    .map(|_| unsafe { PyRef::from_obj_unchecked(source) });
+            if cls.is(&PyBytes::class(vm)) {
+                if let Ok(source) = source.clone().downcast_exact(vm) {
+                    return self.check_args(vm).map(|()| source);
+                }
             }
 
             match_class!(match source {