Separate get_method (magic methods) and get_attribute (full attribute access).

Author: cthulahoops

src/main.rs

@@ -127,7 +127,8 @@ fn shell_exec(vm: &mut VirtualMachine, source: &str, scope: PyObjectRef) -> bool
         }
         Err(err) => {
             // Enum rather than special string here.
-            let msg = match vm.get_attribute(err.clone(), "msg") {
+            let name = vm.new_str("msg".to_string());
+            let msg = match vm.get_attribute(err.clone(), name) {
                 Ok(value) => objstr::get_value(&value),
                 Err(_) => panic!("Expected msg attribute on exception object!"),
             };

vm/src/builtins.rs

@@ -50,7 +50,7 @@ fn dir_object(vm: &mut VirtualMachine, obj: &PyObjectRef) -> PyObjectRef {
 
 fn builtin_abs(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(x, None)]);
-    match vm.get_attribute(x.clone(), &"__abs__") {
+    match vm.get_method(x.clone(), "__abs__") {
         Ok(attrib) => vm.invoke(attrib, PyFuncArgs::new(vec![], vec![])),
         Err(..) => Err(vm.new_type_error("bad operand for abs".to_string())),
     }
@@ -134,7 +134,7 @@ fn builtin_dir(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
 
 fn builtin_divmod(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(vm, args, required = [(x, None), (y, None)]);
-    match vm.get_attribute(x.clone(), &"__divmod__") {
+    match vm.get_method(x.clone(), "__divmod__") {
         Ok(attrib) => vm.invoke(attrib, PyFuncArgs::new(vec![y.clone()], vec![])),
         Err(..) => Err(vm.new_type_error("unsupported operand type(s) for divmod".to_string())),
     }
@@ -218,11 +218,7 @@ fn builtin_getattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         args,
         required = [(obj, None), (attr, Some(vm.ctx.str_type()))]
     );
-    if let PyObjectKind::String { ref value } = attr.borrow().kind {
-        vm.get_attribute(obj.clone(), value)
-    } else {
-        panic!("argument checking failure: attr not string")
-    }
+    vm.get_attribute(obj.clone(), attr.clone())
 }
 
 // builtin_globals
@@ -233,15 +229,11 @@ fn builtin_hasattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         args,
         required = [(obj, None), (attr, Some(vm.ctx.str_type()))]
     );
-    if let PyObjectKind::String { ref value } = attr.borrow().kind {
-        let has_attr = match vm.get_attribute(obj.clone(), value) {
-            Ok(..) => true,
-            Err(..) => false,
-        };
-        Ok(vm.context().new_bool(has_attr))
-    } else {
-        panic!("argument checking failure: attr not string")
-    }
+    let has_attr = match vm.get_attribute(obj.clone(), attr.clone()) {
+        Ok(..) => true,
+        Err(..) => false,
+    };
+    Ok(vm.context().new_bool(has_attr))
 }
 
 fn builtin_hash(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -308,7 +300,7 @@ fn builtin_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         }
         _ => {
             let len_method_name = "__len__".to_string();
-            match vm.get_attribute(obj.clone(), &len_method_name) {
+            match vm.get_method(obj.clone(), &len_method_name) {
                 Ok(value) => vm.invoke(value, PyFuncArgs::default()),
                 Err(..) => Err(vm.context().new_str(
                     format!(
@@ -444,7 +436,7 @@ fn builtin_pow(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
         optional = [(mod_value, Some(vm.ctx.int_type()))]
     );
     let pow_method_name = "__pow__".to_string();
-    let result = match vm.get_attribute(x.clone(), &pow_method_name) {
+    let result = match vm.get_method(x.clone(), &pow_method_name) {
         Ok(attrib) => vm.invoke(attrib, PyFuncArgs::new(vec![y.clone()], vec![])),
         Err(..) => Err(vm.new_type_error("unsupported operand type(s) for pow".to_string())),
     };
@@ -454,7 +446,7 @@ fn builtin_pow(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     match mod_value {
         Some(mod_value) => {
             let mod_method_name = "__mod__".to_string();
-            match vm.get_attribute(
+            match vm.get_method(
                 result.expect("result not defined").clone(),
                 &mod_method_name,
             ) {

vm/src/frame.rs

@@ -124,7 +124,7 @@ impl Frame {
                         &vm.ctx.exceptions.base_exception_type
                     ));
                     let traceback = vm
-                        .get_attribute(exception.clone(), &"__traceback__".to_string())
+                        .get_method(exception.clone(), &"__traceback__".to_string())
                         .unwrap();
                     trace!("Adding to traceback: {:?} {:?}", traceback, lineno);
                     let pos = vm.ctx.new_tuple(vec![
@@ -1007,6 +1007,7 @@ impl Frame {
 
     fn load_attr(&mut self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
         let parent = self.pop_value();
+        let attr_name = vm.new_str(attr_name.to_string());
         let obj = vm.get_attribute(parent, attr_name)?;
         self.push_value(obj);
         Ok(None)

vm/src/obj/objbool.rs

@@ -15,7 +15,7 @@ pub fn boolval(vm: &mut VirtualMachine, obj: PyObjectRef) -> Result<bool, PyObje
         PyObjectKind::String { ref value } => !value.is_empty(),
         PyObjectKind::None { .. } => false,
         _ => {
-            if let Ok(f) = vm.get_attribute(obj.clone(), &String::from("__bool__")) {
+            if let Ok(f) = vm.get_method(obj.clone(), "__bool__") {
                 let bool_res = vm.invoke(f, PyFuncArgs::default())?;
                 let v = match bool_res.borrow().kind {
                     PyObjectKind::Integer { ref value } => !value.is_zero(),

vm/src/obj/objtype.rs

@@ -300,11 +300,6 @@ fn type_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     Ok(vm.new_str(format!("<class '{}'>", type_name)))
 }
 
-pub fn call(vm: &mut VirtualMachine, typ: PyObjectRef, args: PyFuncArgs) -> PyResult {
-    let function = vm.get_attribute(typ, &String::from("__call__"))?;
-    vm.invoke(function, args)
-}
-
 #[cfg(test)]
 mod tests {
     use super::{linearise_mro, new};

vm/src/vm.rs

@@ -408,19 +408,21 @@ impl VirtualMachine {
         Ok(elements)
     }
 
-    pub fn get_attribute(&mut self, obj: PyObjectRef, attr_name: &str) -> PyResult {
+    // get_attribute should be used for full attribute access (usually from user code).
+    pub fn get_attribute(&mut self, obj: PyObjectRef, attr_name: PyObjectRef) -> PyResult {
+        self.call_method(&obj, "__getattribute__", vec![attr_name])
+    }
+
+    // get_method should be used for internal access to magic methods (by-passing
+    // the full getattribute look-up.
+    pub fn get_method(&mut self, obj: PyObjectRef, method_name: &str) -> PyResult {
         let cls = obj.typ();
-        if let Some(attr) = cls.get_attr("__getattribute__") {
-            let name = self.new_str(attr_name.to_string());
-            self.invoke(
-                attr,
-                PyFuncArgs {
-                    args: vec![obj.clone(), name],
-                    kwargs: vec![],
-                },
-            )
-        } else {
-            panic!("Everything should have a __getattribute__");
+        match cls.get_attr(method_name) {
+            Some(method) => self.call_get_descriptor(method, obj.clone()),
+            None => {
+                Err(self
+                    .new_type_error(format!("{:?} object has no method {:?}", obj, method_name)))
+            }
         }
     }