Merge pull request #694 from skinny121/setattr

Various fixes for getattr/setattr/hasattr

tests/snippets/attr.py

@@ -0,0 +1,85 @@
+from testutils import assertRaises
+
+
+class A:
+    pass
+
+
+a = A()
+a.b = 10
+assert hasattr(a, 'b')
+assert a.b == 10
+
+# test override attribute
+setattr(a, 'b', 12)
+assert a.b == 12
+assert getattr(a, 'b') == 12
+
+# test non-existent attribute
+with assertRaises(AttributeError):
+    _ = a.c
+
+with assertRaises(AttributeError):
+    getattr(a, 'c')
+
+assert getattr(a, 'c', 21) == 21
+
+# test set attribute
+setattr(a, 'c', 20)
+assert hasattr(a, 'c')
+assert a.c == 20
+
+# test delete attribute
+delattr(a, 'c')
+assert not hasattr(a, 'c')
+with assertRaises(AttributeError):
+    _ = a.c
+
+
+# test setting attribute on builtin
+with assertRaises(AttributeError):
+    object().a = 1
+
+with assertRaises(AttributeError):
+    setattr(object(), 'a', 2)
+
+attrs = {}
+
+
+class CustomLookup:
+    def __getattr__(self, item):
+        return "value_{}".format(item)
+
+    def __setattr__(self, key, value):
+        attrs[key] = value
+
+
+custom = CustomLookup()
+
+assert custom.attr == "value_attr"
+
+custom.a = 2
+custom.b = 5
+assert attrs['a'] == 2
+assert attrs['b'] == 5
+
+
+class GetRaise:
+    def __init__(self, ex):
+        self.ex = ex
+
+    def __getattr__(self, item):
+        raise self.ex
+
+
+assert not hasattr(GetRaise(AttributeError()), 'a')
+with assertRaises(AttributeError):
+    getattr(GetRaise(AttributeError()), 'a')
+assert getattr(GetRaise(AttributeError()), 'a', 11) == 11
+
+with assertRaises(KeyError):
+    hasattr(GetRaise(KeyError()), 'a')
+with assertRaises(KeyError):
+    getattr(GetRaise(KeyError()), 'a')
+with assertRaises(KeyError):
+    getattr(GetRaise(KeyError()), 'a', 11)

tests/snippets/property.py

@@ -16,6 +16,8 @@ f = Fubar()
 assert f.foo == 100
 assert f.foo == 101
 
+assert type(Fubar.foo) is property
+
 
 null_property = property()
 assert type(null_property) is property
@@ -45,3 +47,5 @@ assert p1.deleter(3).fdel == 3
 assert p1.getter(None).fget == "a"
 assert p1.setter(None).fset == "b"
 assert p1.deleter(None).fdel == "c"
+
+assert p1.__get__(None, object) is p1

tests/snippets/testutils.py

@@ -29,7 +29,7 @@ class assertRaises:
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         if exc_type is None:
-            failmsg = '{!s} was not raised'.format(self.expected.__name_)
+            failmsg = '{!s} was not raised'.format(self.expected.__name__)
             assert False, failmsg            
         if not issubclass(exc_type, self.expected):
             return False

vm/src/builtins.rs

@@ -18,7 +18,7 @@ use crate::obj::objstr::{self, PyStringRef};
 use crate::obj::objtype;
 
 use crate::frame::Scope;
-use crate::function::{Args, PyFuncArgs};
+use crate::function::{Args, OptionalArg, PyFuncArgs};
 use crate::pyobject::{
     AttributeProtocol, IdProtocol, PyContext, PyObjectRef, PyResult, TypeProtocol,
 };
@@ -302,30 +302,38 @@ fn builtin_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     vm.call_method(obj, "__format__", vec![format_spec])
 }
 
-fn builtin_getattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [(obj, None), (attr, Some(vm.ctx.str_type()))]
-    );
-    vm.get_attribute(obj.clone(), attr.clone())
+fn catch_attr_exception<T>(ex: PyObjectRef, default: T, vm: &mut VirtualMachine) -> PyResult<T> {
+    if objtype::isinstance(&ex, &vm.ctx.exceptions.attribute_error) {
+        Ok(default)
+    } else {
+        Err(ex)
+    }
+}
+
+fn builtin_getattr(
+    obj: PyObjectRef,
+    attr: PyStringRef,
+    default: OptionalArg<PyObjectRef>,
+    vm: &mut VirtualMachine,
+) -> PyResult {
+    let ret = vm.get_attribute(obj.clone(), attr.into_object());
+    if let OptionalArg::Present(default) = default {
+        ret.or_else(|ex| catch_attr_exception(ex, default, vm))
+    } else {
+        ret
+    }
 }
 
 fn builtin_globals(vm: &mut VirtualMachine, _args: PyFuncArgs) -> PyResult {
     Ok(vm.current_scope().globals.clone())
 }
 
-fn builtin_hasattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [(obj, None), (attr, Some(vm.ctx.str_type()))]
-    );
-    let has_attr = match vm.get_attribute(obj.clone(), attr.clone()) {
-        Ok(..) => true,
-        Err(..) => false,
-    };
-    Ok(vm.context().new_bool(has_attr))
+fn builtin_hasattr(obj: PyObjectRef, attr: PyStringRef, vm: &mut VirtualMachine) -> PyResult<bool> {
+    if let Err(ex) = vm.get_attribute(obj.clone(), attr.into_object()) {
+        catch_attr_exception(ex, false, vm)
+    } else {
+        Ok(true)
+    }
 }
 
 fn builtin_hash(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -658,15 +666,14 @@ fn builtin_round(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     }
 }
 
-fn builtin_setattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [(obj, None), (attr, Some(vm.ctx.str_type())), (value, None)]
-    );
-    let name = objstr::get_value(attr);
-    vm.ctx.set_attr(obj, &name, value.clone());
-    Ok(vm.get_none())
+fn builtin_setattr(
+    obj: PyObjectRef,
+    attr: PyStringRef,
+    value: PyObjectRef,
+    vm: &mut VirtualMachine,
+) -> PyResult<()> {
+    vm.set_attr(&obj, attr.into_object(), value)?;
+    Ok(())
 }
 
 // builtin_slice

vm/src/frame.rs

@@ -1146,7 +1146,7 @@ impl Frame {
     fn store_attr(&self, vm: &mut VirtualMachine, attr_name: &str) -> FrameResult {
         let parent = self.pop_value();
         let value = self.pop_value();
-        vm.ctx.set_attr(&parent, attr_name, value);
+        vm.set_attr(&parent, vm.new_str(attr_name.to_string()), value)?;
         Ok(None)
     }
 

vm/src/obj/objmodule.rs

@@ -1,3 +1,4 @@
+use crate::obj::objstr::PyStringRef;
 use crate::pyobject::{DictProtocol, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
 use crate::vm::VirtualMachine;
 
@@ -24,10 +25,15 @@ impl PyModuleRef {
             .collect();
         Ok(vm.ctx.new_list(keys))
     }
+
+    fn set_attr(self, attr: PyStringRef, value: PyObjectRef, vm: &mut VirtualMachine) {
+        self.dict.set_item(&vm.ctx, &attr.value, value)
+    }
 }
 
 pub fn init(context: &PyContext) {
     extend_class!(&context, &context.module_type, {
-        "__dir__" => context.new_rustfunc(PyModuleRef::dir)
+        "__dir__" => context.new_rustfunc(PyModuleRef::dir),
+        "__setattr__" => context.new_rustfunc(PyModuleRef::set_attr)
     });
 }

vm/src/obj/objnone.rs

@@ -1,6 +1,9 @@
 use crate::function::PyFuncArgs;
+use crate::obj::objproperty::PyPropertyRef;
+use crate::obj::objstr::PyStringRef;
 use crate::pyobject::{
-    IntoPyObject, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    AttributeProtocol, IntoPyObject, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
+    TryFromObject, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -39,6 +42,57 @@ impl PyNoneRef {
     fn bool(self, _vm: &mut VirtualMachine) -> PyResult<bool> {
         Ok(false)
     }
+
+    fn get_attribute(self, name: PyStringRef, vm: &mut VirtualMachine) -> PyResult {
+        trace!("None.__getattribute__({:?}, {:?})", self, name);
+        let cls = self.typ().into_object();
+
+        // Properties use a comparision with None to determine if they are either invoked by am
+        // instance binding or a class binding. But if the object itself is None then this detection
+        // won't work. Instead we call a special function on property that bypasses this check, as
+        // we are invoking it as a instance binding.
+        //
+        // In CPython they instead call the slot tp_descr_get with NULL to indicates it's an
+        // instance binding.
+        // https://github.com/python/cpython/blob/master/Objects/typeobject.c#L3281
+        fn call_descriptor(
+            descriptor: PyObjectRef,
+            get_func: PyObjectRef,
+            obj: PyObjectRef,
+            cls: PyObjectRef,
+            vm: &mut VirtualMachine,
+        ) -> PyResult {
+            if let Ok(property) = PyPropertyRef::try_from_object(vm, descriptor.clone()) {
+                property.instance_binding_get(obj, vm)
+            } else {
+                vm.invoke(get_func, vec![descriptor, obj, cls])
+            }
+        }
+
+        if let Some(attr) = cls.get_attr(&name.value) {
+            let attr_class = attr.typ();
+            if attr_class.has_attr("__set__") {
+                if let Some(get_func) = attr_class.get_attr("__get__") {
+                    return call_descriptor(attr, get_func, self.into_object(), cls.clone(), vm);
+                }
+            }
+        }
+
+        if let Some(obj_attr) = self.as_object().get_attr(&name.value) {
+            Ok(obj_attr)
+        } else if let Some(attr) = cls.get_attr(&name.value) {
+            let attr_class = attr.typ();
+            if let Some(get_func) = attr_class.get_attr("__get__") {
+                call_descriptor(attr, get_func, self.into_object(), cls.clone(), vm)
+            } else {
+                Ok(attr)
+            }
+        } else if let Some(getter) = cls.get_attr("__getattr__") {
+            vm.invoke(getter, vec![self.into_object(), name.into_object()])
+        } else {
+            Err(vm.new_attribute_error(format!("{} has no attribute '{}'", self.as_object(), name)))
+        }
+    }
 }
 
 fn none_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -55,5 +109,6 @@ pub fn init(context: &PyContext) {
         "__new__" => context.new_rustfunc(none_new),
         "__repr__" => context.new_rustfunc(PyNoneRef::repr),
         "__bool__" => context.new_rustfunc(PyNoneRef::bool),
+        "__getattribute__" => context.new_rustfunc(PyNoneRef::get_attribute)
     });
 }

vm/src/obj/objobject.rs

@@ -4,8 +4,8 @@ use super::objtype;
 use crate::function::PyFuncArgs;
 use crate::obj::objproperty::PropertyBuilder;
 use crate::pyobject::{
-    AttributeProtocol, DictProtocol, IdProtocol, PyAttributes, PyContext, PyObjectRef, PyRef,
-    PyResult, PyValue, TypeProtocol,
+    AttributeProtocol, DictProtocol, IdProtocol, PyAttributes, PyContext, PyObject, PyObjectRef,
+    PyRef, PyResult, PyValue, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -22,9 +22,12 @@ pub type PyInstanceRef = PyRef<PyInstance>;
 
 pub fn new_instance(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> PyResult {
     // more or less __new__ operator
-    let type_ref = args.shift();
-    let obj = vm.ctx.new_instance(type_ref.clone(), None);
-    Ok(obj)
+    let cls = args.shift();
+    Ok(if cls.is(&vm.ctx.object) {
+        PyObject::new_without_dict(PyInstance, cls)
+    } else {
+        PyObject::new(PyInstance, cls)
+    })
 }
 
 fn object_eq(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
@@ -93,7 +96,25 @@ fn object_hash(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     Err(vm.new_type_error("unhashable type".to_string()))
 }
 
-// TODO: is object the right place for delattr?
+fn object_setattr(
+    obj: PyInstanceRef,
+    attr_name: PyStringRef,
+    value: PyObjectRef,
+    vm: &mut VirtualMachine,
+) -> PyResult<()> {
+    trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);
+    if let Some(ref dict) = obj.as_object().dict {
+        dict.borrow_mut().insert(attr_name.value.clone(), value);
+        Ok(())
+    } else {
+        let type_name = objtype::get_type_name(&obj.as_object().typ());
+        Err(vm.new_attribute_error(format!(
+            "'{}' object has no attribute '{}'",
+            type_name, &attr_name.value
+        )))
+    }
+}
+
 fn object_delattr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     arg_check!(
         vm,
@@ -167,6 +188,7 @@ pub fn init(context: &PyContext) {
     context.set_attr(&object, "__le__", context.new_rustfunc(object_le));
     context.set_attr(&object, "__gt__", context.new_rustfunc(object_gt));
     context.set_attr(&object, "__ge__", context.new_rustfunc(object_ge));
+    context.set_attr(&object, "__setattr__", context.new_rustfunc(object_setattr));
     context.set_attr(&object, "__delattr__", context.new_rustfunc(object_delattr));
     context.set_attr(&object, "__dict__", context.new_property(object_dict));
     context.set_attr(&object, "__dir__", context.new_rustfunc(object_dir));
@@ -238,7 +260,7 @@ fn object_getattribute(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
     } else if let Some(attr) = cls.get_attr(&name) {
         vm.call_get_descriptor(attr, obj.clone())
     } else if let Some(getter) = cls.get_attr("__getattr__") {
-        vm.invoke(getter, vec![cls, name_str.clone()])
+        vm.invoke(getter, vec![obj.clone(), name_str.clone()])
     } else {
         Err(vm.new_attribute_error(format!("{} has no attribute '{}'", obj, name)))
     }

vm/src/obj/objproperty.rs

@@ -6,7 +6,7 @@ use crate::function::IntoPyNativeFunc;
 use crate::function::OptionalArg;
 use crate::obj::objstr::PyStringRef;
 use crate::obj::objtype::PyClassRef;
-use crate::pyobject::{PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{IdProtocol, PyContext, PyObject, PyObjectRef, PyRef, PyResult, PyValue};
 use crate::vm::VirtualMachine;
 
 /// Read-only property, doesn't have __set__ or __delete__
@@ -25,7 +25,11 @@ pub type PyReadOnlyPropertyRef = PyRef<PyReadOnlyProperty>;
 
 impl PyReadOnlyPropertyRef {
     fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult {
-        vm.invoke(self.getter.clone(), obj)
+        if obj.is(&vm.ctx.none) {
+            Ok(self.into_object())
+        } else {
+            vm.invoke(self.getter.clone(), obj)
+        }
     }
 }
 
@@ -64,7 +68,12 @@ impl PyPropertyRef {
 
     // Descriptor methods
 
-    fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult {
+    // specialised version that doesn't check for None
+    pub(crate) fn instance_binding_get(
+        self,
+        obj: PyObjectRef,
+        vm: &mut VirtualMachine,
+    ) -> PyResult {
         if let Some(getter) = self.getter.as_ref() {
             vm.invoke(getter.clone(), obj)
         } else {
@@ -72,6 +81,18 @@ impl PyPropertyRef {
         }
     }
 
+    fn get(self, obj: PyObjectRef, _owner: PyClassRef, vm: &mut VirtualMachine) -> PyResult {
+        if let Some(getter) = self.getter.as_ref() {
+            if obj.is(&vm.ctx.none) {
+                Ok(self.into_object())
+            } else {
+                vm.invoke(getter.clone(), obj)
+            }
+        } else {
+            Err(vm.new_attribute_error("unreadable attribute".to_string()))
+        }
+    }
+
     fn set(self, obj: PyObjectRef, value: PyObjectRef, vm: &mut VirtualMachine) -> PyResult {
         if let Some(setter) = self.setter.as_ref() {
             vm.invoke(setter.clone(), vec![obj, value])

vm/src/pyobject.rs

@@ -1112,6 +1112,15 @@ impl PyObject {
         .into_ref()
     }
 
+    pub fn new_without_dict<T: PyObjectPayload>(payload: T, typ: PyObjectRef) -> PyObjectRef {
+        PyObject {
+            typ,
+            dict: None,
+            payload: Box::new(payload),
+        }
+        .into_ref()
+    }
+
     // Move this object into a reference object, transferring ownership.
     pub fn into_ref(self) -> PyObjectRef {
         Rc::new(self)

vm/src/vm.rs

@@ -547,6 +547,15 @@ impl VirtualMachine {
         self.call_method(&obj, "__getattribute__", vec![attr_name])
     }
 
+    pub fn set_attr(
+        &mut self,
+        obj: &PyObjectRef,
+        attr_name: PyObjectRef,
+        attr_value: PyObjectRef,
+    ) -> PyResult {
+        self.call_method(&obj, "__setattr__", vec![attr_name, attr_value])
+    }
+
     pub fn del_attr(&mut self, obj: &PyObjectRef, attr_name: PyObjectRef) -> PyResult {
         self.call_method(&obj, "__delattr__", vec![attr_name])
     }