From 7a83cb6afaf48acee17d98db1bb5a398020e736a Mon Sep 17 00:00:00 2001
From: konstin <konstin@mailbox.org>
Date: Wed, 5 Jun 2019 11:32:46 +0200
Subject: [PATCH 1/2] Use existing fields and methods before calling custom
 __getattr__

Previously, defining `__getattr__` would override all existing fields and methods. This changes it to behave like a `__getattr__` method defined in python, i.e. the custom method is only called if there isn't a field or method of that name
---
 CHANGELOG.md       |  1 +
 src/class/basic.rs | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 45f08d7c..ab58727a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
  * `module` argument to `pyclass` macro. [#499](https://github.com/PyO3/pyo3/pull/499)
  * `py_run!` macro [#512](https://github.com/PyO3/pyo3/pull/512)
+ * Use existing fields and methods before calling custom __getattr__. [#505](https://github.com/PyO3/pyo3/pull/512)
 
 ### Fixed
 
diff --git a/src/class/basic.rs b/src/class/basic.rs
index ed71e2d9..a23c2718 100644
--- a/src/class/basic.rs
+++ b/src/class/basic.rs
@@ -207,12 +207,37 @@ where
     T: for<'p> PyObjectGetAttrProtocol<'p>,
 {
     fn tp_getattro() -> Option<ffi::binaryfunc> {
-        py_binary_func!(
-            PyObjectGetAttrProtocol,
-            T::__getattr__,
-            T::Success,
-            PyObjectCallbackConverter
-        )
+        #[allow(unused_mut)]
+        unsafe extern "C" fn wrap<T>(
+            slf: *mut ffi::PyObject,
+            arg: *mut ffi::PyObject,
+        ) -> *mut ffi::PyObject
+        where
+            T: for<'p> PyObjectGetAttrProtocol<'p>,
+        {
+            let _pool = crate::GILPool::new();
+            let py = Python::assume_gil_acquired();
+
+            // Behave like python's __getattr__ (as opposed to __getattribute__) and check
+            // for existing fields and methods first
+            let existing = ffi::PyObject_GenericGetAttr(slf, arg);
+            if existing == std::ptr::null_mut() {
+                // PyObject_HasAttr also tries to get an object and clears the error if it fails
+                ffi::PyErr_Clear();
+            } else {
+                return existing;
+            }
+
+            let slf = py.mut_from_borrowed_ptr::<T>(slf);
+            let arg = py.from_borrowed_ptr::<crate::types::PyAny>(arg);
+
+            let result = match arg.extract() {
+                Ok(arg) => slf.__getattr__(arg).into(),
+                Err(e) => Err(e.into()),
+            };
+            crate::callback::cb_convert(PyObjectCallbackConverter, py, result)
+        }
+        Some(wrap::<T>)
     }
 }
 

From 60cbe2f47d6a953e8799a253fa8a0a85d01687ad Mon Sep 17 00:00:00 2001
From: kngwyu <yuji.kngw.80s.revive@gmail.com>
Date: Sat, 13 Jul 2019 23:46:25 +0900
Subject: [PATCH 2/2] Add a test that confirms __getattr__ doesn't override

---
 tests/test_dunder.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 mode change 100644 => 100755 tests/test_dunder.rs

diff --git a/tests/test_dunder.rs b/tests/test_dunder.rs
old mode 100644
new mode 100755
index 290819ab..bd1e89f9
--- a/tests/test_dunder.rs
+++ b/tests/test_dunder.rs
@@ -462,3 +462,25 @@ fn weakref_dunder_dict_support() {
         "import weakref; assert weakref.ref(inst)() is inst; inst.a = 1; assert inst.a == 1"
     );
 }
+
+#[pyclass]
+struct ClassWithGetAttr {
+    #[pyo3(get, set)]
+    data: u32,
+}
+
+#[pyproto]
+impl PyObjectProtocol for ClassWithGetAttr {
+    fn __getattr__(&self, _name: &str) -> PyResult<u32> {
+        Ok(self.data * 2)
+    }
+}
+
+#[test]
+fn getattr_doesnt_override_member() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    let inst = PyRef::new(py, ClassWithGetAttr { data: 4 }).unwrap();
+    py_assert!(py, inst, "inst.data == 4");
+    py_assert!(py, inst, "inst.a == 8");
+}