Simplify the setattr and getattr functions a bit

filmor · filmor · commit ea2469fd0abb · 2026-05-05T21:56:54.000+02:00
diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs
@@ -5,6 +5,7 @@
 using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Diagnostics;
+
 using Python.Runtime.Native;
 using Python.Runtime.StateSerialization;
 
@@ -76,74 +77,48 @@ static bool TryGetDynamicInstance(BorrowedReference ob, out object instance, out
             return false;
         }
 
-        static bool TryGetManagedDynamicInstance(BorrowedReference ob, out object instance, out IDynamicMetaObjectProvider dynamicObject)
+        public static NewReference tp_getattro_dlr_proxy(BorrowedReference ob, BorrowedReference key)
         {
-            if (ManagedType.GetManagedObject(ob) is CLRObject co && co.inst is IDynamicMetaObjectProvider coDynamic)
+            var isDynamic = TryGetDynamicInstance(ob, out object instance, out IDynamicMetaObjectProvider dynamicObject);
+
+            // The whole DLR machinery only makes sense with string keys and dynamic objects
+            if (!isDynamic || !Runtime.PyString_Check(key))
             {
-                instance = co.inst;
-                dynamicObject = coDynamic;
-                return true;
+                return Runtime.PyObject_GenericGetAttr(ob, key);
             }
 
-            instance = null!;
-            dynamicObject = null!;
-            return false;
-        }
-
-        public static NewReference tp_getattro_dlr_proxy(BorrowedReference ob, BorrowedReference key)
-        {
-            bool hasStringKey = Runtime.PyString_Check(key);
-            string? memberName = hasStringKey ? Runtime.GetManagedString(key) : null;
+            string memberName = Runtime.GetManagedString(key)!;
 
-            if (memberName is not null && TryGetManagedDynamicInstance(ob, out object preInstance, out var preDynamicObject))
+            // Forward requests to GetDynamicMemberNames to the mixin implementation
+            if (memberName == nameof(DynamicObjectMemberAccessor.GetDynamicMemberNames)
+                && !HasClrMember(instance, memberName))
             {
-                if (memberName == nameof(DynamicObjectMemberAccessor.GetDynamicMemberNames)
-                    && !HasClrMember(preInstance, memberName))
-                {
-                    using var pyMemberNames = new Func<IReadOnlyCollection<string>>(
-                        () => dynamicMemberAccessor.GetDynamicMemberNames(preDynamicObject)).ToPython();
-                    return pyMemberNames.NewReferenceOrNull();
-                }
+                using var pyMemberNames = new Func<IReadOnlyCollection<string>>(
+                    () => dynamicMemberAccessor.GetDynamicMemberNames(dynamicObject)
+                ).ToPython();
+                return pyMemberNames.NewReferenceOrNull();
             }
 
+            // Now, first try to access the Python attribute
             var attr = Runtime.PyObject_GenericGetAttr(ob, key);
             if (!attr.IsNull())
-            {
                 return attr;
-            }
-
-            if (memberName == null)
-            {
-                return default;
-            }
 
+            // attr is null, so an exception must be set. If that exception is not an AttributeError,
+            // we return from this function immediately without clearing. All later returns until the
+            // very end will lead to the AttributeError getting raised.
             if (Runtime.PyErr_ExceptionMatches(Exceptions.AttributeError) == 0)
             {
                 return default;
             }
 
-            if (!TryGetManagedDynamicInstance(ob, out object instance, out var dynamicObject))
+            if (HasClrMember(instance, memberName) || IsPythonSpecialAttributeName(memberName))
             {
                 return default;
             }
 
-            if (memberName is null)
-            {
-                return default;
-            }
-
-            if (HasClrMember(instance, memberName))
-            {
-                return default;
-            }
-
-            if (IsPythonSpecialAttributeName(memberName))
-            {
-                return default;
-            }
-
-            bool resolved;
-            object? value;
+            bool resolved = false;
+            object? value = null;
             try
             {
                 resolved = dynamicMemberAccessor.TryGetMember(dynamicObject, memberName, out value);
@@ -166,64 +141,55 @@ public static NewReference tp_getattro_dlr_proxy(BorrowedReference ob, BorrowedR
 
         public static int tp_setattro_dlr_proxy(BorrowedReference ob, BorrowedReference key, BorrowedReference val)
         {
-            string? memberName = Runtime.PyString_Check(key) ? Runtime.GetManagedString(key) : null;
-            bool hasDynamicInstance = TryGetDynamicInstance(ob, out object instance, out var dynamicObject);
-            bool canUseDynamic = hasDynamicInstance
-                && memberName is not null
-                && !HasClrMember(instance, memberName);
+            var isDynamic = TryGetDynamicInstance(ob, out object instance, out IDynamicMetaObjectProvider dynamicObject);
+
+            // The whole DLR machinery only makes sense with string keys and dynamic objects
+            if (!isDynamic || !Runtime.PyString_Check(key))
+            {
+                return Runtime.PyObject_GenericSetAttr(ob, key, val);
+            }
+
+            string memberName = Runtime.GetManagedString(key)!;
 
             // For Python-derived types (IPythonDerivedType), the Python descriptor protocol
-            // (e.g. @property setters) takes priority over DLR member storage. Try
-            // PyObject_GenericSetAttr first; only fall back to DLR on AttributeError.
-            if (canUseDynamic && instance is IPythonDerivedType)
+            // (e.g. @property setters) takes priority over DLR member storage.
+            if (instance is IPythonDerivedType)
             {
                 int pyResult = Runtime.PyObject_GenericSetAttr(ob, key, val);
-                if (pyResult == 0) return 0;
-                if (Runtime.PyErr_ExceptionMatches(Exceptions.AttributeError) == 0) return pyResult;
+                if (pyResult == 0)
+                    return 0;
+
+                if (Runtime.PyErr_ExceptionMatches(Exceptions.AttributeError) == 0)
+                    return pyResult;
+
                 Runtime.PyErr_Clear();
-                // fall through to DLR path below
+                // Fall through to DLR fallback below
             }
 
-            if (canUseDynamic)
+            if (!HasClrMember(instance, memberName) && !IsPythonSpecialAttributeName(memberName))
             {
+                // Try DLR member storage first
+                bool handled = false;
+
                 if (val == null)
                 {
-                    if (dynamicMemberAccessor.TryDeleteMember(dynamicObject, memberName!))
-                    {
-                        Runtime.PyErr_Clear();
-                        return 0;
-                    }
+                    handled = dynamicMemberAccessor.TryDeleteMember(dynamicObject, memberName);
                 }
                 else
                 {
                     object? managedValue = null;
                     if (val != Runtime.PyNone && !Converter.ToManaged(val, typeof(object), out managedValue, true))
-                    {
                         return -1;
-                    }
 
-                    if (dynamicMemberAccessor.TrySetMember(dynamicObject, memberName!, managedValue))
-                    {
-                        Runtime.PyErr_Clear();
-                        return 0;
-                    }
+                    handled = dynamicMemberAccessor.TrySetMember(dynamicObject, memberName, managedValue);
                 }
-            }
 
-            int result = Runtime.PyObject_GenericSetAttr(ob, key, val);
-            if (result == 0 && canUseDynamic)
-            {
-                object? managedValue = null;
-                if (val != null && val != Runtime.PyNone && !Converter.ToManaged(val, typeof(object), out managedValue, true))
-                {
+                if (handled)
                     return 0;
-                }
-
-                dynamicMemberAccessor.TrySetMember(dynamicObject, memberName!, managedValue);
-                Runtime.PyErr_Clear();
             }
 
-            return result;
+            // Fall back to Python attribute setting
+            return Runtime.PyObject_GenericSetAttr(ob, key, val);
         }
 
         internal static void Initialize()
diff --git a/src/runtime/Types/ClassDerived.cs b/src/runtime/Types/ClassDerived.cs
@@ -41,7 +41,7 @@ static ClassDerivedObject()
             moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();
         }
 
-        public static new void Reset()
+        public static void Reset()
         {
             assemblyBuilders = new Dictionary<string, AssemblyBuilder>();
             moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ static ClassDerivedObject()`
`41`	`41`	`moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();`
`42`	`42`	`}`
`43`	`43`
`44`		`- public static new void Reset()`
	`44`	`+ public static void Reset()`
`45`	`45`	`{`
`46`	`46`	`assemblyBuilders = new Dictionary<string, AssemblyBuilder>();`
`47`	`47`	`moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();`