Treewalk: Implement descriptor protocol for non-data descriptors

docs/SUPPORTED.md

@@ -222,7 +222,7 @@ Attribute Access
 \_\_setattr__(self, name, value)
 \_\_delattr__(self, name)
 Descriptors
-\_\_get__(self, instance, owner)
+\_\_get__(self, instance, owner)|✅
 \_\_set__(self, instance, value)
 \_\_delete__(self, instance)
 Callable Objects

examples/descriptor_protocol.py

@@ -0,0 +1,28 @@
+class MyNonDataDescriptor:
+    def __get__(self, instance, _):
+        return 4 * instance.val
+
+    # TODO test a data descriptor here
+    #def __set__(self, instance, value):
+    #    ...
+
+    #def __delete__(self, instance):
+    #    ...
+
+class MyClass:
+    non_data = MyNonDataDescriptor()
+
+    def __init__(self):
+        self.val = 11
+
+a = MyClass()
+print(a.non_data)
+
+# Because this is a non-data descriptor, this will remove the descriptor entirely.
+a.non_data = 33
+print(a.non_data)
+#
+# del a.attribute
+# print(a.attribute)
+# del a.attribute
+# print(a.attribute)

src/treewalk/interpreter.rs

@@ -8523,4 +8523,34 @@ d = c(g)
             }
         }
     }
+
+    #[test]
+    fn descriptor_protocol() {
+        let input = r#"
+class MyDescriptor:
+    def __get__(self, instance, owner):
+        return 4 * instance.val
+
+class MyClass:
+    attribute = MyDescriptor()
+
+    def __init__(self):
+        self.val = 11
+
+obj = MyClass()
+a = obj.attribute
+"#;
+
+        let (mut parser, mut interpreter) = init(input);
+
+        match interpreter.run(&mut parser) {
+            Err(e) => panic!("Interpreter error: {:?}", e),
+            Ok(_) => {
+                assert_eq!(
+                    interpreter.state.read("a"),
+                    Some(ExprResult::Integer(44.store()))
+                );
+            }
+        }
+    }
 }

src/treewalk/types/descriptor.rs

@@ -2,30 +2,36 @@ use std::fmt::{Display, Error, Formatter};
 
 use crate::{core::Container, treewalk::Interpreter, types::errors::InterpreterError};
 
-use super::{traits::Callable, utils::ResolvedArguments, Class, ExprResult};
+use super::{
+    traits::{Callable, NonDataDescriptorTrait},
+    utils::ResolvedArguments,
+    Class, ExprResult,
+};
 
 /// This is sometimes known as a non-data descriptor because it only has a getter. Those with a
 /// setter or deleter are known as data scriptors.
 #[derive(Clone, Debug, PartialEq)]
-pub struct MemberDescriptor {
+pub struct NonDataDescriptor {
     class: Container<Class>,
-    get: Container<Box<dyn Callable>>,
+    get_func: Container<Box<dyn Callable>>,
 }
 
-impl MemberDescriptor {
-    pub fn new(class: Container<Class>, get: Container<Box<dyn Callable>>) -> Self {
-        Self { class, get }
+impl NonDataDescriptor {
+    pub fn new(class: Container<Class>, get_func: Container<Box<dyn Callable>>) -> Self {
+        Self { class, get_func }
     }
+}
 
-    pub fn get(
+impl NonDataDescriptorTrait for NonDataDescriptor {
+    fn get(
         &self,
         interpreter: &Interpreter,
         instance: ExprResult,
     ) -> Result<ExprResult, InterpreterError> {
         // It's a bit weird to have to convert to an `ExprResult` just to call `bind_if_needed`.
         // Maybe we can clean that up sometime, but for now I think its better to ensure we are
         // using the same binding mechanism everywhere.
-        let callable = ExprResult::BuiltinFunction(self.get.clone())
+        let callable = ExprResult::BuiltinFunction(self.get_func.clone())
             .bind_if_needed(interpreter, instance)
             .as_callable()
             .ok_or(InterpreterError::ExpectedFunction(
@@ -36,12 +42,12 @@ impl MemberDescriptor {
     }
 }
 
-impl Display for MemberDescriptor {
+impl Display for NonDataDescriptor {
     fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
         write!(
             f,
             "<member {} of {} objects>",
-            self.get.borrow().name(),
+            self.get_func.borrow().name(),
             self.class.borrow().name
         )
     }

src/treewalk/types/function.rs

@@ -15,7 +15,7 @@ use super::{
     builtins::NoopCallable,
     traits::{AttributeResolver, Callable, MemberAccessor},
     utils::{Dunder, EnvironmentFrame, ResolvedArguments},
-    Cell, Class, ExprResult, MemberDescriptor, Module, Tuple,
+    Cell, Class, ExprResult, Module, NonDataDescriptor, Tuple,
 };
 
 /// How we evaluate a [`Function`] depends on whether it is async or a generator or a
@@ -248,7 +248,7 @@ struct CodeAttribute;
 
 impl AttributeResolver for CodeAttribute {
     fn resolve(&self, _class: Container<Class>) -> ExprResult {
-        ExprResult::GetSetDescriptor
+        ExprResult::DataDescriptor
     }
 
     fn name(&self) -> &'static str {
@@ -260,8 +260,8 @@ struct GlobalsAttribute;
 
 impl AttributeResolver for GlobalsAttribute {
     fn resolve(&self, class: Container<Class>) -> ExprResult {
-        let descriptor = MemberDescriptor::new(class, Container::new(Box::new(NoopCallable)));
-        ExprResult::MemberDescriptor(descriptor)
+        let descriptor = NonDataDescriptor::new(class, Container::new(Box::new(NoopCallable)));
+        ExprResult::NonDataDescriptor(descriptor)
     }
 
     fn name(&self) -> &'static str {

src/treewalk/types/mod.rs

@@ -47,7 +47,7 @@ pub use cell::Cell;
 pub use class::Class;
 pub use classmethod::Classmethod;
 pub use coroutine::Coroutine;
-pub use descriptor::MemberDescriptor;
+pub use descriptor::NonDataDescriptor;
 pub use dict::Dict;
 pub use dict_items::DictItems;
 pub use dict_keys::DictKeys;

src/treewalk/types/object.rs

@@ -10,7 +10,7 @@ use super::{
     builtins::utils,
     class::InstantiationType,
     function::BindingType,
-    traits::{Callable, MemberAccessor},
+    traits::{Callable, MemberAccessor, NonDataDescriptorTrait},
     utils::{Dunder, ResolvedArguments},
     Class, ExprResult,
 };
@@ -117,18 +117,53 @@ impl MemberAccessor for Container<Object> {
             log(LogLevel::Debug, || {
                 format!("Found: {}::{}", self.borrow().class, name)
             });
-            return match attr {
-                ExprResult::MemberDescriptor(descriptor) => descriptor
-                    .get(interpreter, ExprResult::Object(self.clone()))
-                    .ok(),
-                _ => Some(attr),
+            return match attr.as_member_descriptor(interpreter) {
+                Some(descriptor) => {
+                    let result = descriptor.get(interpreter, ExprResult::Object(self.clone()));
+                    if let Err(err) = result {
+                        // TODO this whole method should probably return a Result so we don't have
+                        // to do this here.
+                        dbg!(&err);
+                        panic!();
+                    }
+                    result.ok()
+                }
+                None => Some(attr),
             };
         }
 
         None
     }
 }
 
+impl NonDataDescriptorTrait for Container<Object> {
+    fn get(
+        &self,
+        interpreter: &Interpreter,
+        instance: ExprResult,
+    ) -> Result<ExprResult, InterpreterError> {
+        let get_func = MemberAccessor::get(self, interpreter, Dunder::Get.value()).ok_or(
+            InterpreterError::FunctionNotFound(Dunder::Get.into(), interpreter.state.call_stack()),
+        )?;
+
+        let get_method = get_func
+            .bind_if_needed(interpreter, ExprResult::Object(self.clone()))
+            .as_callable()
+            .ok_or(InterpreterError::ExpectedFunction(
+                interpreter.state.call_stack(),
+            ))?;
+
+        let owner = ExprResult::Class(instance.get_class(interpreter));
+
+        interpreter.call(
+            get_method,
+            &ResolvedArguments::default()
+                .add_arg(instance)
+                .add_arg(owner),
+        )
+    }
+}
+
 impl Display for Container<Object> {
     fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
         write!(

src/treewalk/types/property.rs

@@ -7,7 +7,7 @@ use super::{
     function::BindingType,
     traits::Callable,
     utils::{Dunder, ResolvedArguments},
-    ExprResult, MemberDescriptor,
+    ExprResult, NonDataDescriptor,
 };
 
 pub struct Property;
@@ -43,9 +43,9 @@ impl Callable for NewBuiltin {
                 interpreter.state.call_stack(),
             ))?;
 
-        let descriptor = MemberDescriptor::new(class, function);
+        let descriptor = NonDataDescriptor::new(class, function);
 
-        Ok(ExprResult::MemberDescriptor(descriptor))
+        Ok(ExprResult::NonDataDescriptor(descriptor))
     }
 
     fn name(&self) -> String {

src/treewalk/types/result.rs

@@ -15,11 +15,13 @@ use super::{
         DictItemsIterator, DictKeysIterator, DictValuesIterator, GeneratorIterator, ListIterator,
         RangeIterator, ReversedIterator, StringIterator, ZipIterator,
     },
-    traits::{Callable, IndexRead, IndexWrite, MemberAccessor, ModuleInterface},
+    traits::{
+        Callable, IndexRead, IndexWrite, MemberAccessor, ModuleInterface, NonDataDescriptorTrait,
+    },
     types::TypeExpr,
     utils::{Dunder, ResolvedArguments},
     ByteArray, Bytes, Cell, Class, Code, Coroutine, Dict, DictItems, DictKeys, DictValues,
-    FrozenSet, Function, List, MappingProxy, MemberDescriptor, Method, Module, Object, Range, Set,
+    FrozenSet, Function, List, MappingProxy, Method, Module, NonDataDescriptor, Object, Range, Set,
     Slice, Str, Super, Traceback, Tuple, Type,
 };
 
@@ -39,9 +41,9 @@ pub enum ExprResult {
     Module(Container<Module>),
     Super(Container<Super>),
     /// TODO implement the descriptor protocol for `Dunder::Code` and others.
-    GetSetDescriptor,
+    DataDescriptor,
     /// TODO implement the descriptor protocol for `Dunder::Globals` and others.
-    MemberDescriptor(MemberDescriptor),
+    NonDataDescriptor(NonDataDescriptor),
     Function(Container<Function>),
     Method(Container<Method>),
     BuiltinFunction(Container<Box<dyn Callable>>),
@@ -194,8 +196,8 @@ impl ExprResult {
             ExprResult::Ellipsis => write!(f, "Ellipsis"),
             ExprResult::NotImplemented => write!(f, "NotImplemented"),
             ExprResult::Super(_) => write!(f, "<super>"),
-            ExprResult::GetSetDescriptor => write!(f, "<attribute todo of todo objects>"),
-            ExprResult::MemberDescriptor(m) => write!(f, "{}", m),
+            ExprResult::DataDescriptor => write!(f, "<attribute todo of todo objects>"),
+            ExprResult::NonDataDescriptor(m) => write!(f, "{}", m),
             ExprResult::Class(c) => write!(f, "{}", c),
             ExprResult::Object(o) => write!(f, "{}", o),
             ExprResult::Method(m) => write!(f, "{}", m),
@@ -302,8 +304,8 @@ impl ExprResult {
             ExprResult::Class(_) => Type::Type,
             ExprResult::Object(_) => Type::Object,
             ExprResult::Super(_) => Type::Super,
-            ExprResult::GetSetDescriptor => Type::GetSetDescriptor,
-            ExprResult::MemberDescriptor(_) => Type::MemberDescriptor,
+            ExprResult::DataDescriptor => Type::GetSetDescriptor,
+            ExprResult::NonDataDescriptor(_) => Type::MemberDescriptor,
             ExprResult::Method(_) => Type::Method,
             ExprResult::Function(_) => Type::Function,
             ExprResult::BuiltinFunction(_) => Type::BuiltinFunction,
@@ -500,6 +502,22 @@ impl ExprResult {
         }
     }
 
+    pub fn as_member_descriptor(
+        &self,
+        interpreter: &Interpreter,
+    ) -> Option<Box<dyn NonDataDescriptorTrait>> {
+        match self {
+            ExprResult::NonDataDescriptor(i) => Some(Box::new(i.clone())),
+            ExprResult::Object(i) => {
+                match MemberAccessor::get(i, interpreter, Dunder::Get.value()).is_some() {
+                    true => Some(Box::new(i.clone())),
+                    false => None,
+                }
+            }
+            _ => None,
+        }
+    }
+
     pub fn as_callable(&self) -> Option<Container<Box<dyn Callable>>> {
         match self {
             ExprResult::Function(i) => {

src/treewalk/types/traits.rs

@@ -51,13 +51,21 @@ impl PartialEq for dyn Callable {
 }
 
 pub trait MemberAccessor {
-    /// A pointer to the `Interpreter` is sometimes not needed, but is required to evalute method
+    /// A pointer to the [`Interpreter`] is sometimes not needed, but is required to evalute method
     /// calls for descriptors.
     fn get(&self, interpreter: &Interpreter, name: &str) -> Option<ExprResult>;
     fn insert(&mut self, name: &str, value: ExprResult);
     fn delete(&mut self, name: &str) -> Option<ExprResult>;
 }
 
+pub trait NonDataDescriptorTrait {
+    fn get(
+        &self,
+        interpreter: &Interpreter,
+        instance: ExprResult,
+    ) -> Result<ExprResult, InterpreterError>;
+}
+
 pub trait IndexRead {
     fn get(&self, index: &ExprResult) -> Option<ExprResult>;
 }

src/treewalk/types/utils/dunder.rs

@@ -8,6 +8,7 @@ pub enum Dunder {
     Ne,
     Enter,
     Exit,
+    Get,
     // Attributes
     Code,
     Globals,
@@ -28,6 +29,7 @@ static DUNDER_MAPPINGS: &[(Dunder, &str)] = &[
     (Dunder::Ne, "__ne__"),
     (Dunder::Enter, "__enter__"),
     (Dunder::Exit, "__exit__"),
+    (Dunder::Get, "__get__"),
     (Dunder::Code, "__code__"),
     (Dunder::Globals, "__globals__"),
     (Dunder::Closure, "__closure__"),