Merge pull request #2518 from RustPython/get-method-opt

Add optimized method lookup to avoid creating intermediate method objects

Lib/test/test_fileio.py

@@ -427,6 +427,7 @@ class PyAutoFileTests(AutoFileTests, unittest.TestCase):
 
 class OtherFileTests:
 
+    @unittest.skip("TODO: non-deterministic failures, FileIO.seekable()?")
     def testAbles(self):
         try:
             f = self.FileIO(TESTFN, "w")
@@ -673,12 +674,6 @@ class PyOtherFileTests(OtherFileTests, unittest.TestCase):
     FileIO = _pyio.FileIO
     modulename = '_pyio'
 
-    # TODO: RUSTPYTHON
-    if sys.platform == "win32":
-        @unittest.expectedFailure
-        def testAbles(self):
-            super().testAbles()
-
     # TODO: RUSTPYTHON
     if sys.platform == "win32":
         @unittest.expectedFailure

Lib/test/test_with.py

@@ -109,8 +109,6 @@ class FailureTestCase(unittest.TestCase):
             with foo: pass
         self.assertRaises(NameError, fooNotDeclared)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def testEnterAttributeError1(self):
         class LacksEnter(object):
             def __exit__(self, type, value, traceback):

bytecode/src/lib.rs

@@ -247,9 +247,7 @@ pub enum Instruction {
     Continue {
         target: Label,
     },
-    Break {
-        target: Label,
-    },
+    Break,
     Jump {
         target: Label,
     },
@@ -281,6 +279,18 @@ pub enum Instruction {
     CallFunctionEx {
         has_kwargs: bool,
     },
+    LoadMethod {
+        idx: NameIdx,
+    },
+    CallMethodPositional {
+        nargs: u32,
+    },
+    CallMethodKeyword {
+        nargs: u32,
+    },
+    CallMethodEx {
+        has_kwargs: bool,
+    },
     ForIter {
         target: Label,
     },
@@ -288,7 +298,9 @@ pub enum Instruction {
     YieldValue,
     YieldFrom,
     SetupAnnotation,
-    SetupLoop,
+    SetupLoop {
+        break_target: Label,
+    },
 
     /// Setup a finally handler, which will be called whenever one of this events occurs:
     /// - the block is popped
@@ -824,7 +836,7 @@ impl Instruction {
             | SetupExcept { handler: l }
             | SetupWith { end: l }
             | SetupAsyncWith { end: l }
-            | Break { target: l }
+            | SetupLoop { break_target: l }
             | Continue { target: l } => Some(l),
             _ => None,
         }
@@ -844,7 +856,7 @@ impl Instruction {
             | SetupExcept { handler: l }
             | SetupWith { end: l }
             | SetupAsyncWith { end: l }
-            | Break { target: l }
+            | SetupLoop { break_target: l }
             | Continue { target: l } => Some(l),
             _ => None,
         }
@@ -853,7 +865,7 @@ impl Instruction {
     pub fn unconditional_branch(&self) -> bool {
         matches!(
             self,
-            Jump { .. } | Continue { .. } | Break { .. } | ReturnValue | Raise { .. }
+            Jump { .. } | Continue { .. } | Break | ReturnValue | Raise { .. }
         )
     }
 
@@ -880,7 +892,7 @@ impl Instruction {
             Duplicate => 1,
             GetIter => 0,
             Continue { .. } => 0,
-            Break { .. } => 0,
+            Break => 0,
             Jump { .. } => 0,
             JumpIfTrue { .. } | JumpIfFalse { .. } => -1,
             JumpIfTrueOrPop { .. } | JumpIfFalseOrPop { .. } => {
@@ -897,9 +909,13 @@ impl Instruction {
                     - flags.contains(MakeFunctionFlags::DEFAULTS) as i32
                     + 1
             }
-            CallFunctionPositional { nargs } => -1 - (*nargs as i32) + 1,
-            CallFunctionKeyword { nargs } => -2 - (*nargs as i32) + 1,
-            CallFunctionEx { has_kwargs } => -2 - *has_kwargs as i32 + 1,
+            CallFunctionPositional { nargs } => -(*nargs as i32) - 1 + 1,
+            CallMethodPositional { nargs } => -(*nargs as i32) - 3 + 1,
+            CallFunctionKeyword { nargs } => -1 - (*nargs as i32) - 1 + 1,
+            CallMethodKeyword { nargs } => -1 - (*nargs as i32) - 3 + 1,
+            CallFunctionEx { has_kwargs } => -1 - (*has_kwargs as i32) - 1 + 1,
+            CallMethodEx { has_kwargs } => -1 - (*has_kwargs as i32) - 3 + 1,
+            LoadMethod { .. } => -1 + 3,
             ForIter { .. } => {
                 if jump {
                     -1
@@ -910,7 +926,11 @@ impl Instruction {
             ReturnValue => -1,
             YieldValue => 0,
             YieldFrom => -1,
-            SetupAnnotation | SetupLoop | SetupFinally { .. } | EnterFinally | EndFinally => 0,
+            SetupAnnotation
+            | SetupLoop { .. }
+            | SetupFinally { .. }
+            | EnterFinally
+            | EndFinally => 0,
             SetupExcept { .. } => {
                 if jump {
                     1
@@ -1056,7 +1076,7 @@ impl Instruction {
             Duplicate => w!(Duplicate),
             GetIter => w!(GetIter),
             Continue { target } => w!(Continue, target),
-            Break { target } => w!(Break, target),
+            Break => w!(Break),
             Jump { target } => w!(Jump, target),
             JumpIfTrue { target } => w!(JumpIfTrue, target),
             JumpIfFalse { target } => w!(JumpIfFalse, target),
@@ -1066,12 +1086,16 @@ impl Instruction {
             CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs),
             CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs),
             CallFunctionEx { has_kwargs } => w!(CallFunctionEx, has_kwargs),
+            LoadMethod { idx } => w!(LoadMethod, name(*idx)),
+            CallMethodPositional { nargs } => w!(CallMethodPositional, nargs),
+            CallMethodKeyword { nargs } => w!(CallMethodKeyword, nargs),
+            CallMethodEx { has_kwargs } => w!(CallMethodEx, has_kwargs),
             ForIter { target } => w!(ForIter, target),
             ReturnValue => w!(ReturnValue),
             YieldValue => w!(YieldValue),
             YieldFrom => w!(YieldFrom),
             SetupAnnotation => w!(SetupAnnotation),
-            SetupLoop => w!(SetupLoop),
+            SetupLoop { break_target } => w!(SetupLoop, break_target),
             SetupExcept { handler } => w!(SetupExcept, handler),
             SetupFinally { handler } => w!(SetupFinally, handler),
             EnterFinally => w!(EnterFinally),

compiler/src/compile.rs

@@ -28,6 +28,28 @@ enum NameUsage {
     Delete,
 }
 
+enum CallType {
+    Positional { nargs: u32 },
+    Keyword { nargs: u32 },
+    Ex { has_kwargs: bool },
+}
+impl CallType {
+    fn normal_call(self) -> Instruction {
+        match self {
+            CallType::Positional { nargs } => Instruction::CallFunctionPositional { nargs },
+            CallType::Keyword { nargs } => Instruction::CallFunctionKeyword { nargs },
+            CallType::Ex { has_kwargs } => Instruction::CallFunctionEx { has_kwargs },
+        }
+    }
+    fn method_call(self) -> Instruction {
+        match self {
+            CallType::Positional { nargs } => Instruction::CallMethodPositional { nargs },
+            CallType::Keyword { nargs } => Instruction::CallMethodKeyword { nargs },
+            CallType::Ex { has_kwargs } => Instruction::CallMethodEx { has_kwargs },
+        }
+    }
+}
+
 /// Main structure holding the state of compilation.
 struct Compiler {
     code_stack: Vec<CodeInfo>,
@@ -667,14 +689,13 @@ impl Compiler {
                     self.switch_to_block(after_block);
                 }
             }
-            Break => match self.ctx.loop_data {
-                Some((_, end)) => {
-                    self.emit(Instruction::Break { target: end });
+            Break => {
+                if self.ctx.loop_data.is_some() {
+                    self.emit(Instruction::Break);
+                } else {
+                    return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location));
                 }
-                None => {
-                    return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location))
-                }
-            },
+            }
             Continue => match self.ctx.loop_data {
                 Some((start, _)) => {
                     self.emit(Instruction::Continue { target: start });
@@ -1242,7 +1263,8 @@ impl Compiler {
             value: qualified_name,
         });
 
-        self.compile_call(2, bases, keywords)?;
+        let call = self.compile_call_inner(2, bases, keywords)?;
+        self.emit(call.normal_call());
 
         self.apply_decorators(decorator_list);
 
@@ -1271,7 +1293,9 @@ impl Compiler {
         let else_block = self.new_block();
         let after_block = self.new_block();
 
-        self.emit(Instruction::SetupLoop);
+        self.emit(Instruction::SetupLoop {
+            break_target: after_block,
+        });
         self.switch_to_block(while_block);
 
         self.compile_jump_if(test, false, else_block)?;
@@ -1360,7 +1384,9 @@ impl Compiler {
         let else_block = self.new_block();
         let after_block = self.new_block();
 
-        self.emit(Instruction::SetupLoop);
+        self.emit(Instruction::SetupLoop {
+            break_target: after_block,
+        });
 
         // The thing iterated:
         self.compile_expression(iter)?;
@@ -1794,10 +1820,7 @@ impl Compiler {
                 func,
                 args,
                 keywords,
-            } => {
-                self.compile_expression(func)?;
-                self.compile_call(0, args, keywords)?;
-            }
+            } => self.compile_call(func, args, keywords)?,
             BoolOp { op, values } => self.compile_bool_op(op, values)?,
             BinOp { left, op, right } => {
                 self.compile_expression(left)?;
@@ -2103,17 +2126,41 @@ impl Compiler {
 
     fn compile_call(
         &mut self,
-        additional_positional: u32,
+        func: &ast::Expr,
         args: &[ast::Expr],
         keywords: &[ast::Keyword],
     ) -> CompileResult<()> {
+        let method = if let ast::ExprKind::Attribute { value, attr, .. } = &func.node {
+            self.compile_expression(value)?;
+            let idx = self.name(attr);
+            self.emit(Instruction::LoadMethod { idx });
+            true
+        } else {
+            self.compile_expression(func)?;
+            false
+        };
+        let call = self.compile_call_inner(0, args, keywords)?;
+        self.emit(if method {
+            call.method_call()
+        } else {
+            call.normal_call()
+        });
+        Ok(())
+    }
+
+    fn compile_call_inner(
+        &mut self,
+        additional_positional: u32,
+        args: &[ast::Expr],
+        keywords: &[ast::Keyword],
+    ) -> CompileResult<CallType> {
         let count = (args.len() + keywords.len()) as u32 + additional_positional;
 
         // Normal arguments:
         let (size, unpack) = self.gather_elements(additional_positional, args)?;
         let has_double_star = keywords.iter().any(|k| k.node.arg.is_none());
 
-        if unpack || has_double_star {
+        let call = if unpack || has_double_star {
             // Create a tuple with positional args:
             self.emit(Instruction::BuildTuple { size, unpack });
 
@@ -2122,7 +2169,7 @@ impl Compiler {
             if has_kwargs {
                 self.compile_keywords(keywords)?;
             }
-            self.emit(Instruction::CallFunctionEx { has_kwargs });
+            CallType::Ex { has_kwargs }
         } else if !keywords.is_empty() {
             let mut kwarg_names = vec![];
             for keyword in keywords {
@@ -2140,12 +2187,12 @@ impl Compiler {
             self.emit_constant(ConstantData::Tuple {
                 elements: kwarg_names,
             });
-            self.emit(Instruction::CallFunctionKeyword { nargs: count });
+            CallType::Keyword { nargs: count }
         } else {
-            self.emit(Instruction::CallFunctionPositional { nargs: count });
-        }
+            CallType::Positional { nargs: count }
+        };
 
-        Ok(())
+        Ok(call)
     }
 
     // Given a vector of expr / star expr generate code which gives either
@@ -2262,8 +2309,13 @@ impl Compiler {
                 unimplemented!("async for comprehensions");
             }
 
+            let loop_block = self.new_block();
+            let after_block = self.new_block();
+
             // Setup for loop:
-            self.emit(Instruction::SetupLoop);
+            self.emit(Instruction::SetupLoop {
+                break_target: after_block,
+            });
 
             if loop_labels.is_empty() {
                 // Load iterator onto stack (passed as first argument):
@@ -2276,8 +2328,6 @@ impl Compiler {
                 self.emit(Instruction::GetIter);
             }
 
-            let loop_block = self.new_block();
-            let after_block = self.new_block();
             loop_labels.push((loop_block, after_block));
 
             self.switch_to_block(loop_block);

compiler/src/ir.rs

@@ -167,25 +167,29 @@ impl CodeInfo {
         let mut startdepths = vec![u32::MAX; self.blocks.len()];
         startdepths[0] = 0;
         stack.push(Label(0));
+        let debug = false;
         'process_blocks: while let Some(block) = stack.pop() {
             let mut depth = startdepths[block.0 as usize];
+            if debug {
+                eprintln!("===BLOCK {}===", block.0);
+            }
             let block = &self.blocks[block.0 as usize];
             for i in &block.instructions {
                 let instr = &i.instr;
                 let effect = instr.stack_effect(false);
                 let new_depth = add_ui(depth, effect);
+                if debug {
+                    eprintln!("{:?}: {:+}, {:+} = {}", instr, effect, depth, new_depth);
+                }
                 if new_depth > maxdepth {
                     maxdepth = new_depth
                 }
-                // we don't want to worry about Continue or Break, they use unwinding to jump to
-                // their targets and as such the stack size is taken care of in frame.rs by setting
+                // we don't want to worry about Continue, it uses unwinding to jump to
+                // its targets and as such the stack size is taken care of in frame.rs by setting
                 // it back to the level it was at when SetupLoop was run
-                let jump_label = instr.label_arg().filter(|_| {
-                    !matches!(
-                        instr,
-                        Instruction::Continue { .. } | Instruction::Break { .. }
-                    )
-                });
+                let jump_label = instr
+                    .label_arg()
+                    .filter(|_| !matches!(instr, Instruction::Continue { .. }));
                 if let Some(&target_block) = jump_label {
                     let effect = instr.stack_effect(true);
                     let target_depth = add_ui(depth, effect);
@@ -215,7 +219,7 @@ fn stackdepth_push(stack: &mut Vec<Label>, startdepths: &mut [u32], target: Labe
 
 fn add_ui(a: u32, b: i32) -> u32 {
     if b < 0 {
-        a - b.abs() as u32
+        a - b.wrapping_abs() as u32
     } else {
         a + b as u32
     }

vm/src/builtins/builtinfunc.rs

@@ -137,7 +137,7 @@ impl PyValue for PyBuiltinMethod {
 
 impl fmt::Debug for PyBuiltinMethod {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "method descriptor")
+        write!(f, "method descriptor for '{}'", self.value.name)
     }
 }
 
@@ -172,7 +172,7 @@ impl Callable for PyBuiltinMethod {
     }
 }
 
-#[pyimpl(with(SlotDescriptor, Callable))]
+#[pyimpl(with(SlotDescriptor, Callable), flags(METHOD_DESCR))]
 impl PyBuiltinMethod {
     #[pyproperty(magic)]
     fn name(&self) -> PyStrRef {

vm/src/builtins/function.rs

@@ -302,7 +302,7 @@ impl PyValue for PyFunction {
     }
 }
 
-#[pyimpl(with(SlotDescriptor, Callable), flags(HAS_DICT))]
+#[pyimpl(with(SlotDescriptor, Callable), flags(HAS_DICT, METHOD_DESCR))]
 impl PyFunction {
     #[pyproperty(magic)]
     fn code(&self) -> PyCodeRef {

vm/src/builtins/getset.rs

@@ -303,29 +303,37 @@ impl PyGetSet {
 impl PyGetSet {
     // Descriptor methods
 
-    #[pymethod(magic)]
-    fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        if let Some(ref f) = self.setter {
-            f(vm, obj, value)
-        } else {
-            Err(vm.new_attribute_error(format!(
-                "attribute '{}' of '{}' objects is not writable",
-                self.name,
-                obj.class().name
-            )))
-        }
-    }
-
-    #[pymethod(magic)]
-    fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        if let Some(ref f) = self.deleter {
-            f(vm, obj)
-        } else {
-            Err(vm.new_attribute_error(format!(
-                "attribute '{}' of '{}' objects is not writable",
-                self.name,
-                obj.class().name
-            )))
+    #[pyslot]
+    fn descr_set(
+        zelf: PyObjectRef,
+        obj: PyObjectRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
+        match value {
+            Some(value) => {
+                if let Some(ref f) = zelf.setter {
+                    f(vm, obj, value)
+                } else {
+                    Err(vm.new_attribute_error(format!(
+                        "attribute '{}' of '{}' objects is not writable",
+                        zelf.name,
+                        obj.class().name
+                    )))
+                }
+            }
+            None => {
+                if let Some(ref f) = zelf.deleter {
+                    f(vm, obj)
+                } else {
+                    Err(vm.new_attribute_error(format!(
+                        "attribute '{}' of '{}' objects is not writable",
+                        zelf.name,
+                        obj.class().name
+                    )))
+                }
+            }
         }
     }
 

vm/src/builtins/iter.rs

@@ -6,7 +6,8 @@ use crossbeam_utils::atomic::AtomicCell;
 
 use super::pytype::PyTypeRef;
 use crate::pyobject::{
-    PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    ItemProtocol, PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
+    TypeProtocol,
 };
 use crate::slots::PyIter;
 use crate::vm::VirtualMachine;
@@ -61,7 +62,7 @@ impl PyIter for PySequenceIterator {
         let step: isize = if zelf.reversed { -1 } else { 1 };
         let pos = zelf.position.fetch_add(step);
         if pos >= 0 {
-            match vm.call_method(&zelf.obj, "__getitem__", (pos,)) {
+            match zelf.obj.get_item(pos, vm) {
                 Err(ref e) if e.isinstance(&vm.ctx.exceptions.index_error) => {
                     Err(vm.new_stop_iteration())
                 }

vm/src/builtins/make_module.rs

@@ -13,7 +13,7 @@ mod decl {
     use crate::builtins::code::PyCodeRef;
     use crate::builtins::dict::PyDictRef;
     use crate::builtins::function::{PyCellRef, PyFunctionRef};
-    use crate::builtins::int::{self, PyIntRef};
+    use crate::builtins::int::PyIntRef;
     use crate::builtins::iter::{PyCallableIterator, PySequenceIterator};
     use crate::builtins::list::{PyList, SortOptions};
     use crate::builtins::pybool::IntoPyBool;
@@ -37,7 +37,7 @@ mod decl {
     use crate::slots::PyComparisonOp;
     use crate::vm::VirtualMachine;
     use crate::{py_io, sysmodule};
-    use num_traits::{Signed, ToPrimitive, Zero};
+    use num_traits::{Signed, Zero};
 
     #[pyfunction]
     fn abs(x: PyObjectRef, vm: &VirtualMachine) -> PyResult {
@@ -197,7 +197,10 @@ mod decl {
     #[pyfunction]
     fn dir(obj: OptionalArg<PyObjectRef>, vm: &VirtualMachine) -> PyResult<PyList> {
         let seq = match obj {
-            OptionalArg::Present(obj) => vm.call_method(&obj, "__dir__", ())?,
+            OptionalArg::Present(obj) => vm
+                .get_special_method(obj, "__dir__")?
+                .map_err(|_obj| vm.new_type_error("object does not provide __dir__".to_owned()))?
+                .invoke((), vm)?,
             OptionalArg::Missing => vm.call_method(vm.current_locals()?.as_object(), "keys", ())?,
         };
         let sorted = sorted(seq, Default::default(), vm)?;
@@ -717,8 +720,7 @@ mod decl {
             vm.get_method_or_type_error(obj.clone(), "__getitem__", || {
                 "argument to reversed() must be a sequence".to_owned()
             })?;
-            let len = vm.call_method(&obj, "__len__", ())?;
-            let len = int::get_value(&len).to_isize().unwrap();
+            let len = vm.obj_len(&obj)? as isize;
             let obj_iterator = PySequenceIterator::new_reversed(obj, len);
             Ok(obj_iterator.into_object(vm))
         }
@@ -734,17 +736,24 @@ mod decl {
 
     #[pyfunction]
     fn round(RoundArgs { number, ndigits }: RoundArgs, vm: &VirtualMachine) -> PyResult {
-        let rounded = match ndigits.flatten() {
+        let meth = vm
+            .get_special_method(number, "__round__")?
+            .map_err(|number| {
+                vm.new_type_error(format!(
+                    "type {} doesn't define __round__",
+                    number.class().name
+                ))
+            })?;
+        match ndigits.flatten() {
             Some(obj) => {
                 let ndigits = vm.to_index(&obj)?;
-                vm.call_method(&number, "__round__", (ndigits,))?
+                meth.invoke((ndigits,), vm)
             }
             None => {
                 // without a parameter, the result type is coerced to int
-                vm.call_method(&number, "__round__", ())?
+                meth.invoke((), vm)
             }
-        };
-        Ok(rounded)
+        }
     }
 
     #[pyfunction]

vm/src/builtins/object.rs

@@ -131,39 +131,34 @@ impl PyBaseObject {
         Self::cmp(&zelf, &other, PyComparisonOp::Gt, vm)
     }
 
-    #[pymethod(magic)]
-    pub(crate) fn setattr(
+    #[pymethod]
+    fn __setattr__(
         obj: PyObjectRef,
         attr_name: PyStrRef,
         value: PyObjectRef,
         vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        setattr(&obj, attr_name, Some(value), vm)
+    }
+
+    #[pymethod]
+    fn __delattr__(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
+        setattr(&obj, attr_name, None, vm)
+    }
+
+    #[pyslot]
+    fn tp_setattro(
+        obj: &PyObjectRef,
+        attr_name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
     ) -> PyResult<()> {
         setattr(obj, attr_name, value, vm)
     }
 
     #[pymethod(magic)]
-    fn delattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) {
-            if let Some(descriptor) = attr.get_class_attr("__delete__") {
-                return vm.invoke(&descriptor, vec![attr, obj]).map(|_| ());
-            }
-        }
-
-        if let Some(dict) = obj.dict() {
-            dict.del_item(attr_name.borrow_value(), vm)?;
-            Ok(())
-        } else {
-            Err(vm.new_attribute_error(format!(
-                "'{}' object has no attribute '{}'",
-                obj.class().name,
-                attr_name.borrow_value()
-            )))
-        }
-    }
-
-    #[pymethod(magic)]
-    fn str(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        vm.call_method(&zelf, "__repr__", ())
+    fn str(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyStrRef> {
+        vm.to_repr(&zelf)
     }
 
     #[pymethod(magic)]
@@ -240,7 +235,7 @@ impl PyBaseObject {
 
     #[pymethod(name = "__getattribute__")]
     #[pyslot]
-    fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+    pub(crate) fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
         vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
         vm.generic_getattribute(obj, name)
     }
@@ -284,21 +279,26 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) -
 
 #[cfg_attr(feature = "flame-it", flame)]
 pub(crate) fn setattr(
-    obj: PyObjectRef,
+    obj: &PyObjectRef,
     attr_name: PyStrRef,
-    value: PyObjectRef,
+    value: Option<PyObjectRef>,
     vm: &VirtualMachine,
 ) -> PyResult<()> {
     vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
 
     if let Some(attr) = obj.get_class_attr(attr_name.borrow_value()) {
-        if let Some(descriptor) = attr.get_class_attr("__set__") {
-            return vm.invoke(&descriptor, vec![attr, obj, value]).map(|_| ());
+        let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
+        if let Some(descriptor) = descr_set {
+            return descriptor(attr, obj.clone(), value, vm);
         }
     }
 
     if let Some(dict) = obj.dict() {
-        dict.set_item(attr_name.borrow_value(), value, vm)?;
+        if let Some(value) = value {
+            dict.set_item(attr_name, value, vm)?;
+        } else {
+            dict.del_item(attr_name, vm)?;
+        }
         Ok(())
     } else {
         Err(vm.new_attribute_error(format!(

vm/src/builtins/property.rs

@@ -6,7 +6,7 @@ use crate::common::lock::PyRwLock;
 use super::pytype::PyTypeRef;
 use crate::function::FuncArgs;
 use crate::pyobject::{
-    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
 use crate::slots::SlotDescriptor;
 use crate::vm::VirtualMachine;
@@ -111,21 +111,29 @@ impl PyProperty {
 
     // Descriptor methods
 
-    #[pymethod(name = "__set__")]
-    fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(ref setter) = self.setter.read().as_ref() {
-            vm.invoke(setter, vec![obj, value])
-        } else {
-            Err(vm.new_attribute_error("can't set attribute".to_owned()))
-        }
-    }
-
-    #[pymethod(name = "__delete__")]
-    fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(ref deleter) = self.deleter.read().as_ref() {
-            vm.invoke(deleter, (obj,))
-        } else {
-            Err(vm.new_attribute_error("can't delete attribute".to_owned()))
+    #[pyslot]
+    fn descr_set(
+        zelf: PyObjectRef,
+        obj: PyObjectRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        let zelf = PyRef::<Self>::try_from_object(vm, zelf)?;
+        match value {
+            Some(value) => {
+                if let Some(ref setter) = zelf.setter.read().as_ref() {
+                    vm.invoke(setter, vec![obj, value]).map(drop)
+                } else {
+                    Err(vm.new_attribute_error("can't set attribute".to_owned()))
+                }
+            }
+            None => {
+                if let Some(ref deleter) = zelf.deleter.read().as_ref() {
+                    vm.invoke(deleter, (obj,)).map(drop)
+                } else {
+                    Err(vm.new_attribute_error("can't delete attribute".to_owned()))
+                }
+            }
         }
     }
 

vm/src/builtins/pytype.rs

@@ -21,7 +21,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyLease, PyObjectRef,
     PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
-use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro};
+use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro, SlotSetattro};
 use crate::vm::VirtualMachine;
 use itertools::Itertools;
 use std::ops::Deref;
@@ -145,7 +145,6 @@ impl PyType {
     }
 
     pub(crate) fn update_slot(&self, name: &str, add: bool) {
-        // self is the resolved class in get_class_magic
         macro_rules! update_slot {
             ($name:ident, $func:expr) => {{
                 self.slots.$name.store(if add { Some($func) } else { None });
@@ -153,24 +152,28 @@ impl PyType {
         }
         match name {
             "__call__" => {
-                let func: slots::GenericMethod = |zelf, args, vm| {
-                    let magic = get_class_magic(&zelf, "__call__");
-                    let magic = vm.call_if_get_descriptor(magic, zelf.clone())?;
-                    vm.invoke(&magic, args)
-                } as _;
+                let func: slots::GenericMethod =
+                    |zelf, args, vm| vm.call_special_method(zelf.clone(), "__call__", args);
                 update_slot!(call, func);
             }
             "__get__" => {
-                let func: slots::DescrGetFunc = |zelf, obj, cls, vm| {
-                    let magic = get_class_magic(&zelf, "__get__");
-                    vm.invoke(&magic, (zelf, obj, cls))
-                } as _;
+                let func: slots::DescrGetFunc =
+                    |zelf, obj, cls, vm| vm.call_special_method(zelf, "__get__", (obj, cls));
                 update_slot!(descr_get, func);
             }
+            "__set__" | "__delete__" => {
+                let func: slots::DescrSetFunc = |zelf, obj, value, vm| {
+                    match value {
+                        Some(val) => vm.call_special_method(zelf, "__set__", (obj, val)),
+                        None => vm.call_special_method(zelf, "__delete__", (obj,)),
+                    }
+                    .map(drop)
+                };
+                update_slot!(descr_set, func);
+            }
             "__hash__" => {
                 let func: slots::HashFunc = |zelf, vm| {
-                    let magic = get_class_magic(&zelf, "__hash__");
-                    let hash_obj = vm.invoke(&magic, vec![zelf.clone()])?;
+                    let hash_obj = vm.call_special_method(zelf.clone(), "__hash__", ())?;
                     match hash_obj.payload_if_subclass::<PyInt>(vm) {
                         Some(py_int) => {
                             Ok(rustpython_common::hash::hash_bigint(py_int.borrow_value()))
@@ -183,40 +186,44 @@ impl PyType {
             }
             "__del__" => {
                 let func: slots::DelFunc = |zelf, vm| {
-                    let magic = get_class_magic(&zelf, "__del__");
-                    let _ = vm.invoke(&magic, vec![zelf.clone()])?;
+                    vm.call_special_method(zelf.clone(), "__del__", ())?;
                     Ok(())
                 } as _;
                 update_slot!(del, func);
             }
             "__eq__" | "__ne__" | "__le__" | "__lt__" | "__ge__" | "__gt__" => {
                 let func: slots::CmpFunc = |zelf, other, op, vm| {
-                    // TODO: differentiate between op types; get_class_magic would panic
-                    let magic = get_class_magic(&zelf, op.method_name());
-                    vm.invoke(&magic, vec![zelf.clone(), other.clone()])
+                    vm.call_special_method(zelf.clone(), op.method_name(), (other.clone(),))
                         .map(Either::A)
                 } as _;
                 update_slot!(cmp, func);
             }
             "__getattribute__" => {
-                let func: slots::GetattroFunc = |zelf, name, vm| {
-                    let magic = get_class_magic(&zelf, "__getattribute__");
-                    vm.invoke(&magic, (zelf, name))
-                };
+                let func: slots::GetattroFunc =
+                    |zelf, name, vm| vm.call_special_method(zelf, "__getattribute__", (name,));
                 update_slot!(getattro, func);
             }
-            "__iter__" => {
-                let func: slots::IterFunc = |zelf, vm| {
-                    let magic = get_class_magic(&zelf, "__iter__");
-                    vm.invoke(&magic, (zelf,))
+            "__setattr__" => {
+                let func: slots::SetattroFunc = |zelf, name, value, vm| {
+                    match value {
+                        Some(value) => {
+                            vm.call_special_method(zelf.clone(), "__setattr__", (name, value))?;
+                        }
+                        None => {
+                            vm.call_special_method(zelf.clone(), "__delattr__", (name,))?;
+                        }
+                    };
+                    Ok(())
                 };
+                update_slot!(setattro, func);
+            }
+            "__iter__" => {
+                let func: slots::IterFunc = |zelf, vm| vm.call_special_method(zelf, "__iter__", ());
                 update_slot!(iter, func);
             }
             "__next__" => {
-                let func: slots::IterNextFunc = |zelf, vm| {
-                    let magic = get_class_magic(zelf, "__next__");
-                    vm.invoke(&magic, (zelf.clone(),))
-                };
+                let func: slots::IterNextFunc =
+                    |zelf, vm| vm.call_special_method(zelf.clone(), "__next__", ());
                 update_slot!(iternext, func);
             }
             _ => {}
@@ -238,17 +245,7 @@ impl PyTypeRef {
     }
 }
 
-#[inline]
-fn get_class_magic(zelf: &PyObjectRef, name: &str) -> PyObjectRef {
-    zelf.get_class_attr(name).unwrap()
-
-    // TODO: we already looked up the matching class but lost the information here
-    // let cls = zelf.class();
-    // let attrs = cls.attributes.read();
-    // attrs.get(name).unwrap().clone()
-}
-
-#[pyimpl(with(SlotGetattro, Callable), flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro, SlotSetattro, Callable), flags(BASETYPE))]
 impl PyType {
     #[pyproperty(name = "__mro__")]
     fn get_mro(zelf: PyRef<Self>) -> PyTuple {
@@ -339,50 +336,6 @@ impl PyType {
         vm.ctx.new_dict()
     }
 
-    #[pymethod(magic)]
-    fn setattr(
-        zelf: PyRef<Self>,
-        attr_name: PyStrRef,
-        value: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<()> {
-        if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) {
-            if let Some(ref descriptor) = attr.get_class_attr("__set__") {
-                vm.invoke(descriptor, (attr, zelf, value))?;
-                return Ok(());
-            }
-        }
-        let attr_name = attr_name.borrow_value();
-        let mut attributes = zelf.attributes.write();
-        attributes.insert(attr_name.to_owned(), value);
-        if attr_name.starts_with("__") && attr_name.ends_with("__") {
-            zelf.update_slot(attr_name, true);
-        }
-        Ok(())
-    }
-
-    #[pymethod(magic)]
-    fn delattr(zelf: PyRef<Self>, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) {
-            if let Some(ref descriptor) = attr.get_class_attr("__delete__") {
-                return vm.invoke(descriptor, (attr, zelf)).map(|_| ());
-            }
-        }
-
-        let mut attributes = zelf.attributes.write();
-        if attributes.remove(attr_name.borrow_value()).is_none() {
-            return Err(vm.new_exception(
-                vm.ctx.exceptions.attribute_error.clone(),
-                vec![attr_name.into_object()],
-            ));
-        };
-        let attr_name = attr_name.borrow_value();
-        if attr_name.starts_with("__") && attr_name.ends_with("__") {
-            zelf.update_slot(attr_name, false);
-        }
-        Ok(())
-    }
-
     #[pymethod(magic)]
     fn subclasses(&self) -> PyList {
         let mut subclasses = self.subclasses.write();
@@ -494,7 +447,7 @@ impl PyType {
         )
         .map_err(|e| vm.new_type_error(e))?;
 
-        vm.ctx.add_tp_new_wrapper(&typ);
+        vm.ctx.add_slot_wrappers(&typ);
 
         // avoid deadlock
         let attributes = typ
@@ -558,10 +511,11 @@ impl SlotGetattro for PyType {
 
         if let Some(ref attr) = mcl_attr {
             let attr_class = attr.class();
-            if attr_class.has_attr("__set__") {
-                if let Some(ref descr_get) =
-                    attr_class.mro_find_map(|cls| cls.slots.descr_get.load())
-                {
+            if attr_class
+                .mro_find_map(|cls| cls.slots.descr_set.load())
+                .is_some()
+            {
+                if let Some(descr_get) = attr_class.mro_find_map(|cls| cls.slots.descr_get.load()) {
                     let mcl = PyLease::into_pyref(mcl).into_object();
                     return descr_get(attr.clone(), Some(zelf.into_object()), Some(mcl), vm);
                 }
@@ -593,6 +547,41 @@ impl SlotGetattro for PyType {
     }
 }
 
+impl SlotSetattro for PyType {
+    fn setattro(
+        zelf: &PyRef<Self>,
+        attr_name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        if let Some(attr) = zelf.get_class_attr(attr_name.borrow_value()) {
+            let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
+            if let Some(descriptor) = descr_set {
+                return descriptor(attr, zelf.clone().into_object(), value, vm);
+            }
+        }
+        let assign = value.is_some();
+
+        let mut attributes = zelf.attributes.write();
+        if let Some(value) = value {
+            attributes.insert(attr_name.borrow_value().to_owned(), value);
+        } else {
+            let prev_value = attributes.remove(attr_name.borrow_value());
+            if prev_value.is_none() {
+                return Err(vm.new_exception(
+                    vm.ctx.exceptions.attribute_error.clone(),
+                    vec![attr_name.into_object()],
+                ));
+            }
+        }
+        let attr_name = attr_name.borrow_value();
+        if attr_name.starts_with("__") && attr_name.ends_with("__") {
+            zelf.update_slot(attr_name, assign);
+        }
+        Ok(())
+    }
+}
+
 impl Callable for PyType {
     fn call(zelf: &PyRef<Self>, args: FuncArgs, vm: &VirtualMachine) -> PyResult {
         vm_trace!("type_call: {:?}", zelf);
@@ -644,19 +633,22 @@ fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -
     let cls = obj.clone_class();
     match find_base_dict_descr(&cls, vm) {
         Some(descr) => {
-            descr
-                .get_class_attr("__set__")
-                .map(|set| vm.invoke(&set, vec![descr, obj, value]))
-                .unwrap_or_else(|| {
-                    Err(vm.new_type_error(format!(
+            let descr_set = descr
+                .class()
+                .mro_find_map(|cls| cls.slots.descr_set.load())
+                .ok_or_else(|| {
+                    vm.new_type_error(format!(
                         "this __dict__ descriptor does not support '{}' objects",
                         cls.name
-                    )))
+                    ))
                 })?;
+            descr_set(descr, obj, Some(value), vm)
+        }
+        None => {
+            object::object_set_dict(obj, PyDictRef::try_from_object(vm, value)?, vm)?;
+            Ok(())
         }
-        None => object::object_set_dict(obj, PyDictRef::try_from_object(vm, value)?, vm)?,
     }
-    Ok(())
 }
 
 /*

vm/src/builtins/weakproxy.rs

@@ -1,7 +1,9 @@
 use super::pytype::PyTypeRef;
 use super::weakref::PyWeak;
+use super::PyStrRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::slots::SlotSetattro;
 use crate::vm::VirtualMachine;
 
 #[pyclass(module = false, name = "weakproxy")]
@@ -16,7 +18,7 @@ impl PyValue for PyWeakProxy {
     }
 }
 
-#[pyimpl]
+#[pyimpl(with(SlotSetattro))]
 impl PyWeakProxy {
     // TODO: callbacks
     #[pyslot]
@@ -45,11 +47,17 @@ impl PyWeakProxy {
             )),
         }
     }
+}
 
-    #[pymethod(name = "__setattr__")]
-    fn setattr(&self, attr_name: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        match self.weak.upgrade() {
-            Some(obj) => vm.set_attr(&obj, attr_name, value),
+impl SlotSetattro for PyWeakProxy {
+    fn setattro(
+        zelf: &PyRef<Self>,
+        attr_name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        match zelf.weak.upgrade() {
+            Some(obj) => vm.call_set_attr(&obj, 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/cformat.rs

@@ -380,13 +380,16 @@ impl CFormatSpec {
                         };
                         Ok(self.format_bytes(bytes))
                     } else {
-                        let bytes = vm.call_method(&obj, "__bytes__", ())
-                            .map_err(|_| {
+                        let bytes = vm
+                            .get_special_method(obj, "__bytes__")?
+                            .map_err(|obj| {
                                 vm.new_type_error(format!(
-                                    "%b requires a bytes-like object, or an object that implements __bytes__, not '{}'",
+                                    "%b requires a bytes-like object, or an object that \
+                                     implements __bytes__, not '{}'",
                                     obj.class().name
                                 ))
-                            })?;
+                            })?
+                            .invoke((), vm)?;
                         let bytes = PyBytes::try_from_object(vm, bytes)?;
                         Ok(self.format_bytes(bytes.borrow_value()))
                     }

vm/src/format.rs

@@ -1,4 +1,4 @@
-use crate::builtins::pystr;
+use crate::builtins::{self, pystr};
 use crate::common::float_ops;
 use crate::exceptions::{IntoPyException, PyBaseExceptionRef};
 use crate::function::FuncArgs;
@@ -891,13 +891,13 @@ fn call_object_format(
     format_spec: &str,
 ) -> PyResult {
     let argument = match preconversion_spec.and_then(FormatPreconversor::from_char) {
-        Some(FormatPreconversor::Str) => vm.call_method(&argument, "__str__", ())?,
-        Some(FormatPreconversor::Repr) => vm.call_method(&argument, "__repr__", ())?,
-        Some(FormatPreconversor::Ascii) => vm.call_method(&argument, "__repr__", ())?,
+        Some(FormatPreconversor::Str) => vm.to_str(&argument)?.into_object(),
+        Some(FormatPreconversor::Repr) => vm.to_repr(&argument)?.into_object(),
+        Some(FormatPreconversor::Ascii) => vm.ctx.new_str(builtins::ascii(argument, vm)?),
         Some(FormatPreconversor::Bytes) => vm.call_method(&argument, "decode", ())?,
         None => argument,
     };
-    let result = vm.call_method(&argument, "__format__", (format_spec,))?;
+    let result = vm.call_special_method(argument, "__format__", (format_spec,))?;
     if !result.isinstance(&vm.ctx.types.str_type) {
         return Err(vm.new_type_error(format!(
             "__format__ must return a str, not {}",

vm/src/frame.rs

@@ -26,8 +26,8 @@ use crate::exceptions::{self, ExceptionCtor, PyBaseExceptionRef};
 use crate::function::FuncArgs;
 use crate::iterator;
 use crate::pyobject::{
-    BorrowValue, IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
-    TypeProtocol,
+    BorrowValue, IdProtocol, ItemProtocol, PyMethod, PyObjectRef, PyRef, PyResult, PyValue,
+    TryFromObject, TypeProtocol,
 };
 use crate::scope::Scope;
 use crate::slots::PyComparisonOp;
@@ -43,7 +43,9 @@ struct Block {
 
 #[derive(Clone, Debug)]
 enum BlockType {
-    Loop,
+    Loop {
+        break_target: bytecode::Label,
+    },
     TryExcept {
         handler: bytecode::Label,
     },
@@ -77,7 +79,7 @@ enum UnwindReason {
 
     // NoWorries,
     /// We are unwinding blocks, since we hit break
-    Break { target: bytecode::Label },
+    Break,
 
     /// We are unwinding blocks since we hit a continue statements.
     Continue { target: bytecode::Label },
@@ -750,8 +752,10 @@ impl ExecutingFrame<'_> {
                 }
                 Ok(None)
             }
-            bytecode::Instruction::SetupLoop => {
-                self.push_block(BlockType::Loop);
+            bytecode::Instruction::SetupLoop { break_target } => {
+                self.push_block(BlockType::Loop {
+                    break_target: *break_target,
+                });
                 Ok(None)
             }
             bytecode::Instruction::SetupExcept { handler } => {
@@ -792,7 +796,7 @@ impl ExecutingFrame<'_> {
                 let exit = vm.get_attribute(context_manager.clone(), "__exit__")?;
                 self.push_value(exit);
                 // Call enter:
-                let enter_res = vm.call_method(&context_manager, "__enter__", ())?;
+                let enter_res = vm.call_special_method(context_manager, "__enter__", ())?;
                 self.push_block(BlockType::Finally { handler: *end });
                 self.push_value(enter_res);
                 Ok(None)
@@ -801,7 +805,7 @@ impl ExecutingFrame<'_> {
                 let mgr = self.pop_value();
                 let aexit = vm.get_attribute(mgr.clone(), "__aexit__")?;
                 self.push_value(aexit);
-                let aenter_res = vm.call_method(&mgr, "__aenter__", ())?;
+                let aenter_res = vm.call_special_method(mgr, "__aenter__", ())?;
                 self.push_value(aenter_res);
 
                 Ok(None)
@@ -883,17 +887,17 @@ impl ExecutingFrame<'_> {
             }
             bytecode::Instruction::GetAIter => {
                 let aiterable = self.pop_value();
-                let aiter = vm.call_method(&aiterable, "__aiter__", ())?;
+                let aiter = vm.call_special_method(aiterable, "__aiter__", ())?;
                 self.push_value(aiter);
                 Ok(None)
             }
             bytecode::Instruction::GetANext => {
                 let aiter = self.last_value();
-                let awaitable = vm.call_method(&aiter, "__anext__", ())?;
+                let awaitable = vm.call_special_method(aiter, "__anext__", ())?;
                 let awaitable = if awaitable.payload_is::<PyCoroutine>() {
                     awaitable
                 } else {
-                    vm.call_method(&awaitable, "__await__", ())?
+                    vm.call_special_method(awaitable, "__await__", ())?
                 };
                 self.push_value(awaitable);
                 Ok(None)
@@ -910,13 +914,43 @@ impl ExecutingFrame<'_> {
             bytecode::Instruction::ForIter { target } => self.execute_for_iter(vm, *target),
             bytecode::Instruction::MakeFunction(flags) => self.execute_make_function(vm, *flags),
             bytecode::Instruction::CallFunctionPositional { nargs } => {
-                self.execute_call_function_positional(vm, *nargs)
+                let args = self.collect_positional_args(*nargs);
+                self.execute_call(args, vm)
             }
             bytecode::Instruction::CallFunctionKeyword { nargs } => {
-                self.execute_call_function_keyword(vm, *nargs)
+                let args = self.collect_keyword_args(*nargs);
+                self.execute_call(args, vm)
             }
             bytecode::Instruction::CallFunctionEx { has_kwargs } => {
-                self.execute_call_function_ex(vm, *has_kwargs)
+                let args = self.collect_ex_args(vm, *has_kwargs)?;
+                self.execute_call(args, vm)
+            }
+            bytecode::Instruction::LoadMethod { idx } => {
+                let obj = self.pop_value();
+                let method_name = self.code.names[*idx as usize].clone();
+                let method = PyMethod::get(obj, method_name, vm)?;
+                let (target, is_method, func) = match method {
+                    PyMethod::Function { target, func } => (target, true, func),
+                    PyMethod::Attribute(val) => (vm.ctx.none(), false, val),
+                };
+                // TODO: figure out a better way to communicate PyMethod::Attribute - CPython uses
+                // target==NULL, maybe we could use a sentinel value or something?
+                self.push_value(target);
+                self.push_value(vm.ctx.new_bool(is_method));
+                self.push_value(func);
+                Ok(None)
+            }
+            bytecode::Instruction::CallMethodPositional { nargs } => {
+                let args = self.collect_positional_args(*nargs);
+                self.execute_method_call(args, vm)
+            }
+            bytecode::Instruction::CallMethodKeyword { nargs } => {
+                let args = self.collect_keyword_args(*nargs);
+                self.execute_method_call(args, vm)
+            }
+            bytecode::Instruction::CallMethodEx { has_kwargs } => {
+                let args = self.collect_ex_args(vm, *has_kwargs)?;
+                self.execute_method_call(args, vm)
             }
             bytecode::Instruction::Jump { target } => {
                 self.jump(*target);
@@ -964,9 +998,7 @@ impl ExecutingFrame<'_> {
 
             bytecode::Instruction::Raise { kind } => self.execute_raise(vm, *kind),
 
-            bytecode::Instruction::Break { target } => {
-                self.unwind_blocks(vm, UnwindReason::Break { target: *target })
-            }
+            bytecode::Instruction::Break => self.unwind_blocks(vm, UnwindReason::Break),
             bytecode::Instruction::Continue { target } => {
                 self.unwind_blocks(vm, UnwindReason::Continue { target: *target })
             }
@@ -1030,7 +1062,7 @@ impl ExecutingFrame<'_> {
                 };
 
                 let spec = self.pop_value();
-                let formatted = vm.call_method(&value, "__format__", (spec,))?;
+                let formatted = vm.call_special_method(value, "__format__", (spec,))?;
                 self.push_value(formatted);
                 Ok(None)
             }
@@ -1144,10 +1176,10 @@ impl ExecutingFrame<'_> {
         // First unwind all existing blocks on the block stack:
         while let Some(block) = self.current_block() {
             match block.typ {
-                BlockType::Loop => match reason {
-                    UnwindReason::Break { target } => {
+                BlockType::Loop { break_target } => match reason {
+                    UnwindReason::Break => {
                         self.pop_block();
-                        self.jump(target);
+                        self.jump(break_target);
                         return Ok(None);
                     }
                     UnwindReason::Continue { target } => {
@@ -1303,19 +1335,14 @@ impl ExecutingFrame<'_> {
         Ok(None)
     }
 
-    fn execute_call_function_positional(&mut self, vm: &VirtualMachine, nargs: u32) -> FrameResult {
-        let args = FuncArgs {
+    fn collect_positional_args(&mut self, nargs: u32) -> FuncArgs {
+        FuncArgs {
             args: self.pop_multiple(nargs as usize).collect(),
             kwargs: IndexMap::new(),
-        };
-
-        let func_ref = self.pop_value();
-        let value = vm.invoke(&func_ref, args)?;
-        self.push_value(value);
-        Ok(None)
+        }
     }
 
-    fn execute_call_function_keyword(&mut self, vm: &VirtualMachine, nargs: u32) -> FrameResult {
+    fn collect_keyword_args(&mut self, nargs: u32) -> FuncArgs {
         let kwarg_names = self
             .pop_value()
             .downcast::<PyTuple>()
@@ -1326,15 +1353,10 @@ impl ExecutingFrame<'_> {
             .borrow_value()
             .iter()
             .map(|pyobj| pystr::clone_value(pyobj));
-        let args = FuncArgs::with_kwargs_names(args, kwarg_names);
-
-        let func_ref = self.pop_value();
-        let value = vm.invoke(&func_ref, args)?;
-        self.push_value(value);
-        Ok(None)
+        FuncArgs::with_kwargs_names(args, kwarg_names)
     }
 
-    fn execute_call_function_ex(&mut self, vm: &VirtualMachine, has_kwargs: bool) -> FrameResult {
+    fn collect_ex_args(&mut self, vm: &VirtualMachine, has_kwargs: bool) -> PyResult<FuncArgs> {
         let kwargs = if has_kwargs {
             let kw_dict: PyDictRef = self.pop_value().downcast().map_err(|_| {
                 // TODO: check collections.abc.Mapping
@@ -1353,14 +1375,33 @@ impl ExecutingFrame<'_> {
         };
         let args = self.pop_value();
         let args = vm.extract_elements(&args)?;
-        let args = FuncArgs { args, kwargs };
+        Ok(FuncArgs { args, kwargs })
+    }
 
+    #[inline]
+    fn execute_call(&mut self, args: FuncArgs, vm: &VirtualMachine) -> FrameResult {
         let func_ref = self.pop_value();
         let value = vm.invoke(&func_ref, args)?;
         self.push_value(value);
         Ok(None)
     }
 
+    #[inline]
+    fn execute_method_call(&mut self, args: FuncArgs, vm: &VirtualMachine) -> FrameResult {
+        let func = self.pop_value();
+        let is_method = self.pop_value().is(&vm.ctx.true_value);
+        let target = self.pop_value();
+        let method = if is_method {
+            PyMethod::Function { target, func }
+        } else {
+            drop(target); // should be None
+            PyMethod::Attribute(func)
+        };
+        let value = method.invoke(args, vm)?;
+        self.push_value(value);
+        Ok(None)
+    }
+
     fn execute_raise(&mut self, vm: &VirtualMachine, kind: bytecode::RaiseKind) -> FrameResult {
         let cause = match kind {
             bytecode::RaiseKind::RaiseCause => {
@@ -1630,9 +1671,33 @@ impl ExecutingFrame<'_> {
     fn execute_unop(&mut self, vm: &VirtualMachine, op: &bytecode::UnaryOperator) -> FrameResult {
         let a = self.pop_value();
         let value = match *op {
-            bytecode::UnaryOperator::Minus => vm.call_method(&a, "__neg__", ())?,
-            bytecode::UnaryOperator::Plus => vm.call_method(&a, "__pos__", ())?,
-            bytecode::UnaryOperator::Invert => vm.call_method(&a, "__invert__", ())?,
+            bytecode::UnaryOperator::Minus => vm
+                .get_special_method(a, "__neg__")?
+                .map_err(|a| {
+                    vm.new_type_error(format!(
+                        "bad operand type for unary -: '{}'",
+                        a.class().name
+                    ))
+                })?
+                .invoke((), vm)?,
+            bytecode::UnaryOperator::Plus => vm
+                .get_special_method(a, "__pos__")?
+                .map_err(|a| {
+                    vm.new_type_error(format!(
+                        "bad operand type for unary +: '{}'",
+                        a.class().name
+                    ))
+                })?
+                .invoke((), vm)?,
+            bytecode::UnaryOperator::Invert => vm
+                .get_special_method(a, "__invert__")?
+                .map_err(|a| {
+                    vm.new_type_error(format!(
+                        "bad operand type for unary ~: '{}'",
+                        a.class().name
+                    ))
+                })?
+                .invoke((), vm)?,
             bytecode::UnaryOperator::Not => {
                 let value = pybool::boolval(vm, a)?;
                 vm.ctx.new_bool(!value)
@@ -1747,6 +1812,7 @@ impl ExecutingFrame<'_> {
         match self.state.stack.try_push(obj) {
             Ok(()) => {}
             Err(_e) => {
+                dbg!(self);
                 panic!("tried to push value onto stack but overflowed max_stacksize")
             }
         }

vm/src/function.rs

@@ -13,8 +13,13 @@ use result_like::impl_option_like;
 use std::marker::PhantomData;
 use std::ops::RangeInclusive;
 
-pub trait IntoFuncArgs {
+pub trait IntoFuncArgs: Sized {
     fn into_args(self, vm: &VirtualMachine) -> FuncArgs;
+    fn into_method_args(self, obj: PyObjectRef, vm: &VirtualMachine) -> FuncArgs {
+        let mut args = self.into_args(vm);
+        args.prepend_arg(obj);
+        args
+    }
 }
 
 impl<T> IntoFuncArgs for T
@@ -26,11 +31,6 @@ where
     }
 }
 
-macro_rules! count {
-    () => (0usize);
-    ( $x:tt $($xs:tt)* ) => (1usize + count!($($xs)*));
-}
-
 // A tuple of values that each implement `IntoPyObject` represents a sequence of
 // arguments that can be bound and passed to a built-in function.
 macro_rules! into_func_args_from_tuple {
@@ -42,9 +42,13 @@ macro_rules! into_func_args_from_tuple {
             #[inline]
             fn into_args(self, vm: &VirtualMachine) -> FuncArgs {
                 let ($($n,)*) = self;
-                let mut result = Vec::with_capacity(count!($($n,)*) + 1);
-                $(result.push($n.into_pyobject(vm), );)*
-                result.into()
+                vec![$($n.into_pyobject(vm),)*].into()
+            }
+
+            #[inline]
+            fn into_method_args(self, obj: PyObjectRef, vm: &VirtualMachine) -> FuncArgs {
+                let ($($n,)*) = self;
+                vec![obj, $($n.into_pyobject(vm),)*].into()
             }
         }
     };

vm/src/macros.rs

@@ -30,7 +30,7 @@ macro_rules! py_class {
             $($crate::py_class!(@extract_slots($ctx, &mut slots, $name, $value));)*
             let py_class = $ctx.new_class($class_name, $class_base, slots);
             $($crate::py_class!(@extract_attrs($ctx, &py_class, $name, $value));)*
-            $ctx.add_tp_new_wrapper(&py_class);
+            $ctx.add_slot_wrappers(&py_class);
             py_class
         }
     };
@@ -55,7 +55,7 @@ macro_rules! extend_class {
         $(
             $class.set_str_attr($name, $value);
         )*
-        $ctx.add_tp_new_wrapper(&$class);
+        $ctx.add_slot_wrappers(&$class);
     };
 }
 

vm/src/pyobject.rs

@@ -359,7 +359,7 @@ impl PyContext {
         PyObject::new(object::PyBaseObject, class, dict)
     }
 
-    pub fn add_tp_new_wrapper(&self, ty: &PyTypeRef) {
+    pub fn add_slot_wrappers(&self, ty: &PyTypeRef) {
         if !ty.attributes.read().contains_key("__new__") {
             let new_wrapper =
                 self.new_bound_method(self.tp_new_wrapper.clone(), ty.clone().into_object());
@@ -643,15 +643,38 @@ where
     T: IntoPyObject,
 {
     fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult {
-        vm.call_method(self, "__getitem__", (key,))
+        vm.get_special_method(self.clone(), "__getitem__")?
+            .map_err(|obj| {
+                vm.new_type_error(format!(
+                    "'{}' object is not subscriptable",
+                    obj.class().name
+                ))
+            })?
+            .invoke((key,), vm)
     }
 
     fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        vm.call_method(self, "__setitem__", (key, value)).map(drop)
+        vm.get_special_method(self.clone(), "__setitem__")?
+            .map_err(|obj| {
+                vm.new_type_error(format!(
+                    "'{}' does not support item assignment",
+                    obj.class().name
+                ))
+            })?
+            .invoke((key, value), vm)?;
+        Ok(())
     }
 
     fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()> {
-        vm.call_method(self, "__delitem__", (key,)).map(drop)
+        vm.get_special_method(self.clone(), "__delitem__")?
+            .map_err(|obj| {
+                vm.new_type_error(format!(
+                    "'{}' does not support item deletion",
+                    obj.class().name
+                ))
+            })?
+            .invoke((key,), vm)?;
+        Ok(())
     }
 }
 
@@ -1075,7 +1098,7 @@ pub trait PyClassImpl: PyClassDef {
             );
         }
         Self::impl_extend_class(ctx, class);
-        ctx.add_tp_new_wrapper(&class);
+        ctx.add_slot_wrappers(&class);
         if let Some(doc) = Self::DOC {
             class.set_str_attr("__doc__", ctx.new_str(doc));
         }
@@ -1251,3 +1274,129 @@ pub fn hash_iter_unordered<'a, I: IntoIterator<Item = &'a PyObjectRef>>(
 ) -> PyResult<rustpython_common::hash::PyHash> {
     rustpython_common::hash::hash_iter_unordered(iter, |obj| vm._hash(obj))
 }
+
+#[derive(Debug)]
+pub enum PyMethod {
+    Function {
+        target: PyObjectRef,
+        func: PyObjectRef,
+    },
+    Attribute(PyObjectRef),
+}
+
+impl PyMethod {
+    pub fn get(obj: PyObjectRef, name: pystr::PyStrRef, vm: &VirtualMachine) -> PyResult<Self> {
+        let cls = obj.class();
+        let getattro = cls.mro_find_map(|cls| cls.slots.getattro.load()).unwrap();
+        if getattro as usize != object::PyBaseObject::getattro as usize {
+            drop(cls);
+            return getattro(obj, name, vm).map(Self::Attribute);
+        }
+
+        let mut is_method = false;
+
+        let cls_attr = match cls.get_attr(name.borrow_value()) {
+            Some(descr) => {
+                let descr_cls = descr.class();
+                let descr_get = if descr_cls.slots.flags.has_feature(PyTpFlags::METHOD_DESCR) {
+                    is_method = true;
+                    None
+                } else {
+                    let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
+                    if let Some(descr_get) = descr_get {
+                        if descr_cls
+                            .mro_find_map(|cls| cls.slots.descr_set.load())
+                            .is_some()
+                        {
+                            drop(descr_cls);
+                            let cls = PyLease::into_pyref(cls).into_object();
+                            return descr_get(descr, Some(obj), Some(cls), vm).map(Self::Attribute);
+                        }
+                    }
+                    descr_get
+                };
+                drop(descr_cls);
+                Some((descr, descr_get))
+            }
+            None => None,
+        };
+
+        if let Some(dict) = obj.dict() {
+            if let Some(attr) = dict.get_item_option(name.clone(), vm)? {
+                return Ok(Self::Attribute(attr));
+            }
+        }
+
+        if let Some((attr, descr_get)) = cls_attr {
+            match descr_get {
+                None if is_method => {
+                    drop(cls);
+                    Ok(Self::Function {
+                        target: obj,
+                        func: attr,
+                    })
+                }
+                Some(descr_get) => {
+                    let cls = PyLease::into_pyref(cls).into_object();
+                    descr_get(attr, Some(obj), Some(cls), vm).map(Self::Attribute)
+                }
+                None => Ok(Self::Attribute(attr)),
+            }
+        } else if let Some(getter) = cls.get_attr("__getattr__") {
+            drop(cls);
+            vm.invoke(&getter, (obj, name)).map(Self::Attribute)
+        } else {
+            Err(vm
+                .new_attribute_error(format!("'{}' object has no attribute '{}'", cls.name, name)))
+        }
+    }
+
+    pub(crate) fn get_special(
+        obj: PyObjectRef,
+        name: &str,
+        vm: &VirtualMachine,
+    ) -> PyResult<Result<Self, PyObjectRef>> {
+        let obj_cls = obj.class();
+        let func = match obj_cls.get_attr(name) {
+            Some(f) => f,
+            None => {
+                drop(obj_cls);
+                return Ok(Err(obj));
+            }
+        };
+        let meth = if func
+            .class()
+            .slots
+            .flags
+            .has_feature(PyTpFlags::METHOD_DESCR)
+        {
+            drop(obj_cls);
+            Self::Function { target: obj, func }
+        } else {
+            let obj_cls = PyLease::into_pyref(obj_cls).into_object();
+            let attr = vm
+                .call_get_descriptor_specific(func, Some(obj), Some(obj_cls))
+                .unwrap_or_else(Ok)?;
+            Self::Attribute(attr)
+        };
+        Ok(Ok(meth))
+    }
+
+    pub fn invoke(self, args: impl IntoFuncArgs, vm: &VirtualMachine) -> PyResult {
+        let (func, args) = match self {
+            PyMethod::Function { target, func } => (func, args.into_method_args(target, vm)),
+            PyMethod::Attribute(func) => (func, args.into_args(vm)),
+        };
+        vm.invoke(&func, args)
+    }
+
+    pub fn invoke_ref(&self, args: impl IntoFuncArgs, vm: &VirtualMachine) -> PyResult {
+        let (func, args) = match self {
+            PyMethod::Function { target, func } => {
+                (func, args.into_method_args(target.clone(), vm))
+            }
+            PyMethod::Attribute(func) => (func, args.into_args(vm)),
+        };
+        vm.invoke(func, args)
+    }
+}

vm/src/pyobjectrc.rs

@@ -360,7 +360,6 @@ impl fmt::Debug for PyObjectWeak {
 /// A `PyRef<T>` can be directly returned from a built-in function to handle
 /// situations (such as when implementing in-place methods such as `__iadd__`)
 /// where a reference to the same object must be returned.
-#[derive(Debug)]
 #[repr(transparent)]
 pub struct PyRef<T: PyObjectPayload> {
     // invariant: this obj must always have payload of type T
@@ -368,6 +367,12 @@ pub struct PyRef<T: PyObjectPayload> {
     _payload: PhantomData<PyRc<T>>,
 }
 
+impl<T: PyObjectPayload> fmt::Debug for PyRef<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.obj.fmt(f)
+    }
+}
+
 impl<T: PyObjectPayload> Clone for PyRef<T> {
     fn clone(&self) -> Self {
         Self {

vm/src/slots.rs

@@ -15,6 +15,7 @@ bitflags! {
     pub struct PyTpFlags: u64 {
         const HEAPTYPE = 1 << 9;
         const BASETYPE = 1 << 10;
+        const METHOD_DESCR = 1 << 17;
         const HAS_DICT = 1 << 40;
 
         #[cfg(debug_assertions)]
@@ -46,6 +47,8 @@ pub(crate) type GenericMethod = fn(&PyObjectRef, FuncArgs, &VirtualMachine) -> P
 pub(crate) type DelFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<()>;
 pub(crate) type DescrGetFunc =
     fn(PyObjectRef, Option<PyObjectRef>, Option<PyObjectRef>, &VirtualMachine) -> PyResult;
+pub(crate) type DescrSetFunc =
+    fn(PyObjectRef, PyObjectRef, Option<PyObjectRef>, &VirtualMachine) -> PyResult<()>;
 pub(crate) type HashFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<PyHash>;
 pub(crate) type CmpFunc = fn(
     &PyObjectRef,
@@ -54,6 +57,8 @@ pub(crate) type CmpFunc = fn(
     &VirtualMachine,
 ) -> PyResult<Either<PyObjectRef, PyComparisonValue>>;
 pub(crate) type GetattroFunc = fn(PyObjectRef, PyStrRef, &VirtualMachine) -> PyResult;
+pub(crate) type SetattroFunc =
+    fn(&PyObjectRef, PyStrRef, Option<PyObjectRef>, &VirtualMachine) -> PyResult<()>;
 pub(crate) type BufferFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult<Box<dyn Buffer>>;
 pub(crate) type IterFunc = fn(PyObjectRef, &VirtualMachine) -> PyResult;
 pub(crate) type IterNextFunc = fn(&PyObjectRef, &VirtualMachine) -> PyResult;
@@ -66,9 +71,11 @@ pub struct PyTypeSlots {
     pub del: AtomicCell<Option<DelFunc>>,
     pub call: AtomicCell<Option<GenericMethod>>,
     pub descr_get: AtomicCell<Option<DescrGetFunc>>,
+    pub descr_set: AtomicCell<Option<DescrSetFunc>>,
     pub hash: AtomicCell<Option<HashFunc>>,
     pub cmp: AtomicCell<Option<CmpFunc>>,
     pub getattro: AtomicCell<Option<GetattroFunc>>,
+    pub setattro: AtomicCell<Option<SetattroFunc>>,
     pub buffer: Option<BufferFunc>,
     pub iter: AtomicCell<Option<IterFunc>>,
     pub iternext: AtomicCell<Option<IterNextFunc>>,
@@ -409,6 +416,46 @@ pub trait SlotGetattro: PyValue {
         Self::getattro(zelf, name, vm)
     }
 }
+
+#[pyimpl]
+pub trait SlotSetattro: PyValue {
+    #[pyslot]
+    fn tp_setattro(
+        obj: &PyObjectRef,
+        name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        if let Some(zelf) = obj.downcast_ref::<Self>() {
+            Self::setattro(zelf, name, value, vm)
+        } else {
+            Err(vm.new_type_error("unexpected payload for __setattr__".to_owned()))
+        }
+    }
+
+    fn setattro(
+        zelf: &PyRef<Self>,
+        name: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()>;
+
+    #[pymethod]
+    fn __setattr__(
+        zelf: PyRef<Self>,
+        name: PyStrRef,
+        value: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        Self::setattro(&zelf, name, Some(value), vm)
+    }
+
+    #[pymethod]
+    fn __delattr__(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
+        Self::setattro(&zelf, name, None, vm)
+    }
+}
+
 #[pyimpl]
 pub trait BufferProtocol: PyValue {
     #[pyslot]

vm/src/stdlib/io.rs

@@ -259,7 +259,7 @@ mod _io {
     }
 
     fn check_readable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        if pybool::boolval(vm, call_method(vm, file, "readable", ())?)? {
+        if pybool::boolval(vm, vm.call_method(file, "readable", ())?)? {
             Ok(())
         } else {
             Err(new_unsupported_operation(
@@ -270,7 +270,7 @@ mod _io {
     }
 
     fn check_writable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        if pybool::boolval(vm, call_method(vm, file, "writable", ())?)? {
+        if pybool::boolval(vm, vm.call_method(file, "writable", ())?)? {
             Ok(())
         } else {
             Err(new_unsupported_operation(
@@ -281,7 +281,7 @@ mod _io {
     }
 
     fn check_seekable(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        if pybool::boolval(vm, call_method(vm, file, "seekable", ())?)? {
+        if pybool::boolval(vm, vm.call_method(file, "seekable", ())?)? {
             Ok(())
         } else {
             Err(new_unsupported_operation(
@@ -308,7 +308,7 @@ mod _io {
         }
         #[pymethod]
         fn tell(zelf: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-            call_method(vm, &zelf, "seek", (0, 1))
+            vm.call_method(&zelf, "seek", (0, 1))
         }
         #[pymethod]
         fn truncate(zelf: PyObjectRef, _pos: OptionalArg, vm: &VirtualMachine) -> PyResult {
@@ -328,7 +328,7 @@ mod _io {
 
         #[pyslot]
         fn tp_del(instance: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-            let _ = call_method(vm, instance, "close", ());
+            let _ = vm.call_method(instance, "close", ());
             Ok(())
         }
 
@@ -339,7 +339,7 @@ mod _io {
 
         #[pymethod(magic)]
         fn exit(instance: PyObjectRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> {
-            call_method(vm, &instance, "close", ())?;
+            vm.call_method(&instance, "close", ())?;
             Ok(())
         }
 
@@ -428,7 +428,7 @@ mod _io {
         ) -> PyResult<()> {
             check_closed(&instance, vm)?;
             for line in lines.iter(vm)? {
-                call_method(vm, &instance, "write", (line?,))?;
+                vm.call_method(&instance, "write", (line?,))?;
             }
             Ok(())
         }
@@ -461,7 +461,7 @@ mod _io {
         }
         #[pyslot]
         fn tp_iternext(instance: &PyObjectRef, vm: &VirtualMachine) -> PyResult {
-            let line = call_method(vm, &instance, "readline", ())?;
+            let line = vm.call_method(&instance, "readline", ())?;
             if !pybool::boolval(vm, line.clone())? {
                 Err(vm.new_stop_iteration())
             } else {
@@ -476,7 +476,7 @@ mod _io {
 
     pub(super) fn iobase_close(file: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         if !file_closed(file, vm)? {
-            let res = call_method(vm, file, "flush", ());
+            let res = vm.call_method(file, "flush", ());
             vm.set_attr(file, "__closed", vm.ctx.new_bool(true))?;
             res?;
         }
@@ -496,7 +496,7 @@ mod _io {
                 let b = PyByteArray::from(vec![0; size]).into_ref(vm);
                 let n = <Option<usize>>::try_from_object(
                     vm,
-                    call_method(vm, &instance, "readinto", (b.clone(),))?,
+                    vm.call_method(&instance, "readinto", (b.clone(),))?,
                 )?;
                 Ok(n.map(|n| {
                     let bytes = &mut b.borrow_value_mut().elements;
@@ -505,7 +505,7 @@ mod _io {
                 })
                 .into_pyobject(vm))
             } else {
-                call_method(vm, &instance, "readall", ())
+                vm.call_method(&instance, "readall", ())
             }
         }
 
@@ -514,7 +514,7 @@ mod _io {
             let mut chunks = Vec::new();
             let mut total_len = 0;
             loop {
-                let data = call_method(vm, &instance, "read", (DEFAULT_BUFFER_SIZE,))?;
+                let data = vm.call_method(&instance, "read", (DEFAULT_BUFFER_SIZE,))?;
                 let data = <Option<PyBytesRef>>::try_from_object(vm, data)?;
                 match data {
                     None => {
@@ -562,7 +562,7 @@ mod _io {
         ) -> PyResult<usize> {
             let b = PyRwBytesLike::new(vm, &bufobj)?;
             let l = b.len();
-            let data = call_method(vm, &zelf, method, (l,))?;
+            let data = vm.call_method(&zelf, method, (l,))?;
             if data.is(&bufobj) {
                 return Ok(l);
             }
@@ -732,7 +732,7 @@ mod _io {
         }
 
         fn raw_seek(&mut self, pos: Offset, whence: i32, vm: &VirtualMachine) -> PyResult<Offset> {
-            let ret = call_method(vm, self.check_init(vm)?, "seek", (pos, whence))?;
+            let ret = vm.call_method(self.check_init(vm)?, "seek", (pos, whence))?;
             let offset = get_offset(ret, vm)?;
             if offset < 0 {
                 return Err(
@@ -777,7 +777,7 @@ mod _io {
         }
 
         fn raw_tell(&mut self, vm: &VirtualMachine) -> PyResult<Offset> {
-            let ret = call_method(vm, self.check_init(vm)?, "tell", ())?;
+            let ret = vm.call_method(self.check_init(vm)?, "tell", ())?;
             let offset = get_offset(ret, vm)?;
             if offset < 0 {
                 return Err(
@@ -809,7 +809,7 @@ mod _io {
                     .into_pyobject(vm);
 
                 // TODO: loop if write() raises an interrupt
-                call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj,))?
+                vm.call_method(self.raw.as_ref().unwrap(), "write", (memobj,))?
             } else {
                 let options = BufferOptions {
                     len,
@@ -827,7 +827,7 @@ mod _io {
                         .into_ref(vm);
 
                 // TODO: loop if write() raises an interrupt
-                let res = call_method(vm, self.raw.as_ref().unwrap(), "write", (memobj.clone(),));
+                let res = vm.call_method(self.raw.as_ref().unwrap(), "write", (memobj.clone(),));
 
                 memobj.released.store(true);
                 self.buffer = std::mem::take(&mut writebuf.data.lock());
@@ -1064,12 +1064,8 @@ mod _io {
                     .into_ref(vm);
 
                     // TODO: loop if readinto() raises an interrupt
-                    let res = call_method(
-                        vm,
-                        self.raw.as_ref().unwrap(),
-                        "readinto",
-                        (memobj.clone(),),
-                    );
+                    let res =
+                        vm.call_method(self.raw.as_ref().unwrap(), "readinto", (memobj.clone(),));
 
                     memobj.released.store(true);
                     std::mem::swap(v, &mut readbuf.data.lock());
@@ -1080,7 +1076,7 @@ mod _io {
                     let memobj =
                         PyMemoryView::from_buffer_range(vm.ctx.none(), buf, buf_range, vm)?;
                     // TODO: loop if readinto() raises an interrupt
-                    call_method(vm, self.raw.as_ref().unwrap(), "readinto", (memobj,))?
+                    vm.call_method(self.raw.as_ref().unwrap(), "readinto", (memobj,))?
                 }
             };
 
@@ -1135,7 +1131,7 @@ mod _io {
 
             let mut read_size = 0;
             loop {
-                let read_data = call_method(vm, self.raw.as_ref().unwrap(), "read", ())?;
+                let read_data = vm.call_method(self.raw.as_ref().unwrap(), "read", ())?;
                 let read_data = <Option<PyBytesRef>>::try_from_object(vm, read_data)?;
 
                 match read_data {
@@ -1388,13 +1384,13 @@ mod _io {
             if data.writable() {
                 data.flush_rewind(vm)?;
             }
-            let res = call_method(vm, data.raw.as_ref().unwrap(), "truncate", (pos,))?;
+            let res = vm.call_method(data.raw.as_ref().unwrap(), "truncate", (pos,))?;
             let _ = data.raw_tell(vm);
             Ok(res)
         }
         #[pymethod]
         fn detach(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
-            call_method(vm, zelf.as_object(), "flush", ())?;
+            vm.call_method(zelf.as_object(), "flush", ())?;
             let mut data = zelf.lock(vm)?;
             data.flags.insert(BufferedFlags::DETACHED);
             data.raw
@@ -1403,7 +1399,7 @@ mod _io {
         }
         #[pymethod]
         fn seekable(&self, vm: &VirtualMachine) -> PyResult {
-            call_method(vm, self.lock(vm)?.check_init(vm)?, "seekable", ())
+            vm.call_method(self.lock(vm)?.check_init(vm)?, "seekable", ())
         }
         #[pyproperty]
         fn raw(&self, vm: &VirtualMachine) -> PyResult<Option<PyObjectRef>> {
@@ -1423,11 +1419,11 @@ mod _io {
         }
         #[pymethod]
         fn fileno(&self, vm: &VirtualMachine) -> PyResult {
-            call_method(vm, self.lock(vm)?.check_init(vm)?, "fileno", ())
+            vm.call_method(self.lock(vm)?.check_init(vm)?, "fileno", ())
         }
         #[pymethod]
         fn isatty(&self, vm: &VirtualMachine) -> PyResult {
-            call_method(vm, self.lock(vm)?.check_init(vm)?, "isatty", ())
+            vm.call_method(self.lock(vm)?.check_init(vm)?, "isatty", ())
         }
 
         #[pymethod(magic)]
@@ -1464,7 +1460,7 @@ mod _io {
                 return Ok(vm.ctx.none());
             }
             let flush_res = data.flush(vm);
-            let close_res = call_method(vm, data.raw.as_ref().unwrap(), "close", ());
+            let close_res = vm.call_method(data.raw.as_ref().unwrap(), "close", ());
             exceptions::chain(flush_res, close_res)
         }
 
@@ -1477,10 +1473,10 @@ mod _io {
                     return Ok(vm.ctx.none());
                 }
             }
-            let flush_res = call_method(vm, zelf.as_object(), "flush", ()).map(drop);
+            let flush_res = vm.call_method(zelf.as_object(), "flush", ()).map(drop);
             let data = zelf.lock(vm)?;
             let raw = data.raw.as_ref().unwrap();
-            let close_res = call_method(vm, raw, "close", ());
+            let close_res = vm.call_method(raw, "close", ());
             exceptions::chain(flush_res, close_res)
         }
 
@@ -1500,19 +1496,6 @@ mod _io {
         }
     }
 
-    // vm.call_method() only calls class attributes
-    // TODO: have this be the implementation of vm.call_method() once the current implementation isn't needed
-    // anymore because of slots
-    pub fn call_method(
-        vm: &VirtualMachine,
-        obj: &PyObjectRef,
-        name: impl crate::pyobject::TryIntoRef<PyStr>,
-        args: impl crate::function::IntoFuncArgs,
-    ) -> PyResult {
-        let meth = vm.get_attribute(obj.clone(), name)?;
-        vm.invoke(&meth, args)
-    }
-
     #[pyimpl]
     trait BufferedReadable: PyValue {
         type Reader: BufferedMixin;
@@ -1921,13 +1904,13 @@ mod _io {
                     "can't do nonzero end-relative seeks".to_owned(),
                 ));
             }
-            call_method(vm, &buffer, "seek", (offset, how))
+            vm.call_method(&buffer, "seek", (offset, how))
         }
 
         #[pymethod]
         fn tell(&self, vm: &VirtualMachine) -> PyResult {
             let buffer = self.lock(vm)?.buffer.clone();
-            call_method(vm, &buffer, "tell", ())
+            vm.call_method(&buffer, "tell", ())
         }
 
         #[pyproperty]
@@ -1947,7 +1930,7 @@ mod _io {
         #[pymethod]
         fn fileno(&self, vm: &VirtualMachine) -> PyResult {
             let buffer = self.lock(vm)?.buffer.clone();
-            call_method(vm, &buffer, "fileno", ())
+            vm.call_method(&buffer, "fileno", ())
         }
 
         #[pymethod]
@@ -1955,7 +1938,7 @@ mod _io {
             let buffer = self.lock(vm)?.buffer.clone();
             check_readable(&buffer, vm)?;
 
-            let bytes = call_method(vm, &buffer, "read", (size.flatten(),))?;
+            let bytes = vm.call_method(&buffer, "read", (size.flatten(),))?;
             let bytes = PyBytesLike::try_from_object(vm, bytes)?;
             //format bytes into string
             let rust_string = String::from_utf8(bytes.to_cow().into_owned()).map_err(|e| {
@@ -1976,9 +1959,9 @@ mod _io {
 
             let bytes = obj.borrow_value().as_bytes();
 
-            let len = call_method(vm, &buffer, "write", (bytes.to_owned(),));
+            let len = vm.call_method(&buffer, "write", (bytes.to_owned(),));
             if obj.borrow_value().contains('\n') {
-                let _ = call_method(vm, &buffer, "flush", ());
+                let _ = vm.call_method(&buffer, "flush", ());
             }
             let len = usize::try_from_object(vm, len?)?;
 
@@ -1994,14 +1977,14 @@ mod _io {
         fn flush(&self, vm: &VirtualMachine) -> PyResult {
             let buffer = self.lock(vm)?.buffer.clone();
             check_closed(&buffer, vm)?;
-            call_method(vm, &buffer, "flush", ())
+            vm.call_method(&buffer, "flush", ())
         }
 
         #[pymethod]
         fn isatty(&self, vm: &VirtualMachine) -> PyResult {
             let buffer = self.lock(vm)?.buffer.clone();
             check_closed(&buffer, vm)?;
-            call_method(vm, &buffer, "isatty", ())
+            vm.call_method(&buffer, "isatty", ())
         }
 
         #[pymethod]
@@ -2013,7 +1996,7 @@ mod _io {
             let buffer = self.lock(vm)?.buffer.clone();
             check_readable(&buffer, vm)?;
 
-            let bytes = call_method(vm, &buffer, "readline", (size.flatten(),))?;
+            let bytes = vm.call_method(&buffer, "readline", (size.flatten(),))?;
             let bytes = PyBytesLike::try_from_object(vm, bytes)?;
             //format bytes into string
             let rust_string = String::from_utf8(bytes.borrow_value().to_vec()).map_err(|e| {
@@ -2028,7 +2011,7 @@ mod _io {
         #[pymethod]
         fn close(&self, vm: &VirtualMachine) -> PyResult {
             let buffer = self.lock(vm)?.buffer.clone();
-            call_method(vm, &buffer, "close", ())
+            vm.call_method(&buffer, "close", ())
         }
         #[pyproperty]
         fn closed(&self, vm: &VirtualMachine) -> PyResult {

vm/src/stdlib/thread.rs

@@ -9,7 +9,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, ItemProtocol, PyCallable, PyClassImpl, PyObjectRef, PyRef,
     PyResult, PyValue, StaticType, TypeProtocol,
 };
-use crate::slots::SlotGetattro;
+use crate::slots::{SlotGetattro, SlotSetattro};
 use crate::vm::VirtualMachine;
 
 use parking_lot::{
@@ -304,7 +304,7 @@ impl PyValue for PyLocal {
     }
 }
 
-#[pyimpl(with(SlotGetattro), flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro, SlotSetattro), flags(BASETYPE))]
 impl PyLocal {
     fn ldict(&self, vm: &VirtualMachine) -> PyDictRef {
         self.data.get_or(|| vm.ctx.new_dict()).clone()
@@ -317,37 +317,6 @@ impl PyLocal {
         }
         .into_ref_with_type(vm, cls)
     }
-
-    #[pymethod(magic)]
-    fn setattr(
-        zelf: PyRef<Self>,
-        attr: PyStrRef,
-        value: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<()> {
-        if attr.borrow_value() == "__dict__" {
-            Err(vm.new_attribute_error(format!(
-                "{} attribute '__dict__' is read-only",
-                zelf.as_object()
-            )))
-        } else {
-            zelf.ldict(vm).set_item(attr.into_object(), value, vm)?;
-            Ok(())
-        }
-    }
-
-    #[pymethod(magic)]
-    fn delattr(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        if attr.borrow_value() == "__dict__" {
-            Err(vm.new_attribute_error(format!(
-                "{} attribute '__dict__' is read-only",
-                zelf.as_object()
-            )))
-        } else {
-            zelf.ldict(vm).del_item(attr.into_object(), vm)?;
-            Ok(())
-        }
-    }
 }
 
 impl SlotGetattro for PyLocal {
@@ -365,6 +334,30 @@ impl SlotGetattro for PyLocal {
     }
 }
 
+impl SlotSetattro for PyLocal {
+    fn setattro(
+        zelf: &PyRef<Self>,
+        attr: PyStrRef,
+        value: Option<PyObjectRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
+        if attr.borrow_value() == "__dict__" {
+            Err(vm.new_attribute_error(format!(
+                "{} attribute '__dict__' is read-only",
+                zelf.as_object()
+            )))
+        } else {
+            let dict = zelf.ldict(vm);
+            if let Some(value) = value {
+                dict.set_item(attr, value, vm)?;
+            } else {
+                dict.del_item(attr, vm)?;
+            }
+            Ok(())
+        }
+    }
+}
+
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
     let ctx = &vm.ctx;
 

vm/src/vm.rs

@@ -29,8 +29,8 @@ use crate::frame::{ExecutionResult, Frame, FrameRef};
 use crate::function::{FuncArgs, IntoFuncArgs};
 use crate::pyobject::{
     BorrowValue, Either, IdProtocol, IntoPyObject, ItemProtocol, PyArithmaticValue, PyContext,
-    PyObject, PyObjectRef, PyRef, PyRefExact, PyResult, PyValue, TryFromObject, TryIntoRef,
-    TypeProtocol,
+    PyLease, PyMethod, PyObject, PyObjectRef, PyRef, PyRefExact, PyResult, PyValue, TryFromObject,
+    TryIntoRef, TypeProtocol,
 };
 use crate::scope::Scope;
 use crate::slots::PyComparisonOp;
@@ -815,7 +815,7 @@ impl VirtualMachine {
         if obj.class().is(&self.ctx.types.str_type) {
             Ok(obj.clone().downcast().unwrap())
         } else {
-            let s = self.call_method(&obj, "__str__", ())?;
+            let s = self.call_special_method(obj.clone(), "__str__", ())?;
             PyStrRef::try_from_object(self, s)
         }
     }
@@ -826,8 +826,8 @@ impl VirtualMachine {
     }
 
     pub fn to_repr(&self, obj: &PyObjectRef) -> PyResult<PyStrRef> {
-        let repr = self.call_method(obj, "__repr__", ())?;
-        TryFromObject::try_from_object(self, repr)
+        let repr = self.call_special_method(obj.clone(), "__repr__", ())?;
+        PyStrRef::try_from_object(self, repr)
     }
 
     pub fn to_index_opt(&self, obj: PyObjectRef) -> Option<PyResult<PyIntRef>> {
@@ -925,7 +925,11 @@ impl VirtualMachine {
         if obj.class().is(cls) {
             Ok(true)
         } else {
-            let ret = self.call_method(cls.as_object(), "__instancecheck__", (obj.clone(),))?;
+            let ret = self.call_special_method(
+                cls.as_object().clone(),
+                "__instancecheck__",
+                (obj.clone(),),
+            )?;
             pybool::boolval(self, ret)
         }
     }
@@ -933,7 +937,11 @@ impl VirtualMachine {
     /// Determines if `subclass` is a subclass of `cls`, either directly, indirectly or virtually
     /// via the __subclasscheck__ magic method.
     pub fn issubclass(&self, subclass: &PyTypeRef, cls: &PyTypeRef) -> PyResult<bool> {
-        let ret = self.call_method(cls.as_object(), "__subclasscheck__", (subclass.clone(),))?;
+        let ret = self.call_special_method(
+            cls.as_object().clone(),
+            "__subclasscheck__",
+            (subclass.clone(),),
+        )?;
         pybool::boolval(self, ret)
     }
 
@@ -963,21 +971,35 @@ impl VirtualMachine {
         self.call_get_descriptor(attr, obj).unwrap_or_else(Ok)
     }
 
+    #[inline]
     pub fn call_method<T>(&self, obj: &PyObjectRef, method_name: &str, args: T) -> PyResult
     where
         T: IntoFuncArgs,
     {
         flame_guard!(format!("call_method({:?})", method_name));
 
-        // This is only used in the vm for magic methods, which use a greatly simplified attribute lookup.
-        match obj.get_class_attr(method_name) {
-            Some(func) => {
-                vm_trace!("vm.call_method {:?} {:?} -> {:?}", obj, method_name, func);
-                let wrapped = self.call_if_get_descriptor(func, obj.clone())?;
-                self.invoke(&wrapped, args)
-            }
-            None => Err(self.new_type_error(format!("Unsupported method: {}", method_name))),
-        }
+        PyMethod::get(obj.clone(), PyStr::from(method_name).into_ref(self), self)?
+            .invoke(args, self)
+    }
+
+    #[inline]
+    pub(crate) fn get_special_method(
+        &self,
+        obj: PyObjectRef,
+        method: &str,
+    ) -> PyResult<Result<PyMethod, PyObjectRef>> {
+        PyMethod::get_special(obj, method, self)
+    }
+
+    pub(crate) fn call_special_method(
+        &self,
+        obj: PyObjectRef,
+        method: &str,
+        args: impl IntoFuncArgs,
+    ) -> PyResult {
+        self.get_special_method(obj, method)?
+            .map_err(|_obj| self.new_attribute_error(method.to_owned()))?
+            .invoke(args, self)
     }
 
     fn _invoke(&self, callable: &PyObjectRef, args: FuncArgs) -> PyResult {
@@ -1149,18 +1171,42 @@ impl VirtualMachine {
         }
     }
 
-    pub fn set_attr<K, V>(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult
+    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: TryIntoRef<PyStr>,
         V: Into<PyObjectRef>,
     {
         let attr_name = attr_name.try_into_ref(self)?;
-        self.call_method(obj, "__setattr__", (attr_name, attr_value.into()))
+        self.call_set_attr(obj, attr_name, Some(attr_value.into()))
     }
 
-    pub fn del_attr(&self, obj: &PyObjectRef, attr_name: PyObjectRef) -> PyResult<()> {
-        self.call_method(&obj, "__delattr__", (attr_name,))?;
-        Ok(())
+    pub fn del_attr(&self, obj: &PyObjectRef, attr_name: impl TryIntoRef<PyStr>) -> PyResult<()> {
+        let attr_name = attr_name.try_into_ref(self)?;
+        self.call_set_attr(obj, attr_name, None)
     }
 
     // get_method should be used for internal access to magic methods (by-passing
@@ -1180,8 +1226,8 @@ impl VirtualMachine {
         }
     }
 
-    /// May return exception, if `__get__` descriptor raises one
-    pub fn get_method(&self, obj: PyObjectRef, method_name: &str) -> Option<PyResult> {
+    // TODO: remove + transfer over to get_special_method
+    pub(crate) fn get_method(&self, obj: PyObjectRef, method_name: &str) -> Option<PyResult> {
         let method = obj.get_class_attr(method_name)?;
         Some(self.call_if_get_descriptor(method, obj))
     }
@@ -1257,15 +1303,26 @@ impl VirtualMachine {
         dict: Option<PyDictRef>,
     ) -> PyResult<Option<PyObjectRef>> {
         let name = name_str.borrow_value();
-        let cls_attr = obj.class().get_attr(name);
-
-        if let Some(ref attr) = cls_attr {
-            if attr.class().has_attr("__set__") {
-                if let Ok(r) = self.call_get_descriptor(attr.clone(), obj.clone()) {
-                    return r.map(Some);
+        let obj_cls = obj.class();
+        let cls_attr = match obj_cls.get_attr(name) {
+            Some(descr) => {
+                let descr_cls = descr.class();
+                let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
+                if let Some(descr_get) = descr_get {
+                    if descr_cls
+                        .mro_find_map(|cls| cls.slots.descr_set.load())
+                        .is_some()
+                    {
+                        drop(descr_cls);
+                        let cls = PyLease::into_pyref(obj_cls).into_object();
+                        return descr_get(descr, Some(obj), Some(cls), self).map(Some);
+                    }
                 }
+                drop(descr_cls);
+                Some((descr, descr_get))
             }
-        }
+            None => None,
+        };
 
         let dict = dict.or_else(|| obj.dict());
 
@@ -1277,9 +1334,16 @@ impl VirtualMachine {
 
         if let Some(obj_attr) = attr {
             Ok(Some(obj_attr))
-        } else if let Some(attr) = cls_attr {
-            self.call_if_get_descriptor(attr, obj).map(Some)
-        } else if let Some(getter) = obj.clone_class().get_attr("__getattr__") {
+        } else if let Some((attr, descr_get)) = cls_attr {
+            match descr_get {
+                Some(descr_get) => {
+                    let cls = PyLease::into_pyref(obj_cls).into_object();
+                    descr_get(attr, Some(obj), Some(cls), self).map(Some)
+                }
+                None => Ok(Some(attr)),
+            }
+        } else if let Some(getter) = obj_cls.get_attr("__getattr__") {
+            drop(obj_cls);
             self.invoke(&getter, (obj, name_str)).map(Some)
         } else {
             Ok(None)
@@ -1632,25 +1696,30 @@ impl VirtualMachine {
     }
 
     pub fn obj_len_opt(&self, obj: &PyObjectRef) -> Option<PyResult<usize>> {
-        self.get_method(obj.clone(), "__len__").map(|len| {
-            let len = self.invoke(&len?, ())?;
-            let len = len
-                .payload_if_subclass::<PyInt>(self)
-                .ok_or_else(|| {
-                    self.new_type_error(format!(
-                        "'{}' object cannot be interpreted as an integer",
-                        len.class().name
-                    ))
-                })?
-                .borrow_value();
-            if len.is_negative() {
-                return Err(self.new_value_error("__len__() should return >= 0".to_owned()));
-            }
-            let len = len.to_isize().ok_or_else(|| {
-                self.new_overflow_error("cannot fit 'int' into an index-sized integer".to_owned())
-            })?;
-            Ok(len as usize)
-        })
+        self.get_special_method(obj.clone(), "__len__")
+            .map(Result::ok)
+            .transpose()
+            .map(|meth| {
+                let len = meth?.invoke((), self)?;
+                let len = len
+                    .payload_if_subclass::<PyInt>(self)
+                    .ok_or_else(|| {
+                        self.new_type_error(format!(
+                            "'{}' object cannot be interpreted as an integer",
+                            len.class().name
+                        ))
+                    })?
+                    .borrow_value();
+                if len.is_negative() {
+                    return Err(self.new_value_error("__len__() should return >= 0".to_owned()));
+                }
+                let len = len.to_isize().ok_or_else(|| {
+                    self.new_overflow_error(
+                        "cannot fit 'int' into an index-sized integer".to_owned(),
+                    )
+                })?;
+                Ok(len as usize)
+            })
     }
 
     pub fn obj_len(&self, obj: &PyObjectRef) -> PyResult<usize> {
@@ -1679,11 +1748,9 @@ impl VirtualMachine {
     }
 
     pub fn _membership(&self, haystack: PyObjectRef, needle: PyObjectRef) -> PyResult {
-        if let Some(method_or_err) = self.get_method(haystack.clone(), "__contains__") {
-            let method = method_or_err?;
-            self.invoke(&method, vec![needle])
-        } else {
-            self._membership_iter_search(haystack, needle)
+        match PyMethod::get_special(haystack, "__contains__", self)? {
+            Ok(method) => method.invoke((needle,), self),
+            Err(haystack) => self._membership_iter_search(haystack, needle),
         }
     }
 
@@ -1798,7 +1865,7 @@ impl VirtualMachine {
         attr_value: impl Into<PyObjectRef>,
     ) -> PyResult<()> {
         let val = attr_value.into();
-        object::setattr(module.clone(), attr_name.try_into_ref(self)?, val, self)
+        object::setattr(module, attr_name.try_into_ref(self)?, Some(val), self)
     }
 }