Skip to content

Commit a41a153

Browse files
lostmsulostmsu
committed
base accessors were not exposed to Python because .NET PropertyInfo GetMethod would not return base non-overriden accessor for a partially overriden property
because of that when constructing PropertyObject we scan base classes to find base accessor (if any) this might have performance implications due to replacement of PropertyInfo.GetValue with getter.Invoke (not tested) fixes #1455
1 parent 8d61215 commit a41a153

File tree

3 files changed

+100
-9
lines changed

3 files changed

+100
-9
lines changed

‎src/embed_tests/Inheritance.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,15 @@ public void BaseClearIsCalled()
111111
scope.Set("exn", null);
112112
Assert.AreEqual(1, msg.Refcount);
113113
}
114+
115+
// https://github.com/pythonnet/pythonnet/issues/1455
116+
[Test]
117+
public void PropertyAccessorOverridden()
118+
{
119+
using var derived = new PropertyAccessorDerived().ToPython();
120+
derived.SetAttr(nameof(PropertyAccessorDerived.VirtualProp), "hi".ToPython());
121+
Assert.AreEqual("HI", derived.GetAttr(nameof(PropertyAccessorDerived.VirtualProp)).As<string>());
122+
}
114123
}
115124

116125
class ExtraBaseTypeProvider : IPythonBaseTypeProvider
@@ -183,4 +192,16 @@ public int XProp
183192
set => this.extras[nameof(this.XProp)] = value;
184193
}
185194
}
195+
196+
public class PropertyAccessorBase
197+
{
198+
public virtual string VirtualProp { get; set; }
199+
}
200+
201+
public class PropertyAccessorIntermediate: PropertyAccessorBase { }
202+
203+
public class PropertyAccessorDerived: PropertyAccessorIntermediate
204+
{
205+
public override string VirtualProp { set => base.VirtualProp = value.ToUpperInvariant(); }
206+
}
186207
}

‎src/runtime/ReflectionUtil.cs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
namespace Python.Runtime;
2+
3+
using System;
4+
using System.Reflection;
5+
6+
static class ReflectionUtil
7+
{
8+
public static MethodInfo? GetBaseGetMethod(this PropertyInfo property, bool nonPublic)
9+
{
10+
if (property is null) throw new ArgumentNullException(nameof(property));
11+
12+
Type baseType = property.DeclaringType.BaseType;
13+
BindingFlags bindingFlags = property.GetBindingFlags();
14+
15+
while (baseType is not null)
16+
{
17+
var baseProperty = baseType.GetProperty(property.Name, bindingFlags | BindingFlags.DeclaredOnly);
18+
var accessor = baseProperty?.GetGetMethod(nonPublic);
19+
if (accessor is not null)
20+
return accessor;
21+
22+
baseType = baseType.BaseType;
23+
}
24+
25+
return null;
26+
}
27+
28+
public static MethodInfo? GetBaseSetMethod(this PropertyInfo property, bool nonPublic)
29+
{
30+
if (property is null) throw new ArgumentNullException(nameof(property));
31+
32+
Type baseType = property.DeclaringType.BaseType;
33+
BindingFlags bindingFlags = property.GetBindingFlags();
34+
35+
while (baseType is not null)
36+
{
37+
var baseProperty = baseType.GetProperty(property.Name, bindingFlags | BindingFlags.DeclaredOnly);
38+
var accessor = baseProperty?.GetSetMethod(nonPublic);
39+
if (accessor is not null)
40+
return accessor;
41+
42+
baseType = baseType.BaseType;
43+
}
44+
45+
return null;
46+
}
47+
48+
public static BindingFlags GetBindingFlags(this PropertyInfo property)
49+
{
50+
var accessor = property.GetMethod ?? property.SetMethod;
51+
BindingFlags flags = default;
52+
flags |= accessor.IsStatic ? BindingFlags.Static : BindingFlags.Instance;
53+
flags |= accessor.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic;
54+
return flags;
55+
}
56+
}

‎src/runtime/propertyobject.cs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Reflection;
3+
using System.Runtime.Serialization;
34

45
namespace Python.Runtime
56
{
@@ -8,17 +9,25 @@ namespace Python.Runtime
89
/// Implements a Python descriptor type that manages CLR properties.
910
/// </summary>
1011
[Serializable]
11-
internal class PropertyObject : ExtensionType
12+
internal class PropertyObject : ExtensionType, IDeserializationCallback
1213
{
1314
internal MaybeMemberInfo<PropertyInfo> info;
14-
private MaybeMethodInfo getter;
15-
private MaybeMethodInfo setter;
15+
[NonSerialized]
16+
private MethodInfo? getter;
17+
[NonSerialized]
18+
private MethodInfo? setter;
1619

1720
public PropertyObject(PropertyInfo md)
1821
{
19-
getter = md.GetGetMethod(true);
20-
setter = md.GetSetMethod(true);
2122
info = new MaybeMemberInfo<PropertyInfo>(md);
23+
CacheAccessors();
24+
}
25+
26+
void CacheAccessors()
27+
{
28+
PropertyInfo md = info.Value;
29+
getter = md.GetGetMethod(true) ?? md.GetBaseGetMethod(true);
30+
setter = md.GetSetMethod(true) ?? md.GetBaseSetMethod(true);
2231
}
2332

2433

@@ -35,7 +44,7 @@ public static NewReference tp_descr_get(BorrowedReference ds, BorrowedReference
3544
return Exceptions.RaiseTypeError(self.info.DeletedMessage);
3645
}
3746
var info = self.info.Value;
38-
MethodInfo getter = self.getter.UnsafeValue;
47+
MethodInfo? getter = self.getter;
3948
object result;
4049

4150

@@ -70,7 +79,7 @@ public static NewReference tp_descr_get(BorrowedReference ds, BorrowedReference
7079

7180
try
7281
{
73-
result = info.GetValue(co.inst, null);
82+
result = getter.Invoke(co.inst, Array.Empty<object>());
7483
return Converter.ToPython(result, info.PropertyType);
7584
}
7685
catch (Exception e)
@@ -100,7 +109,7 @@ public static int tp_descr_set(BorrowedReference ds, BorrowedReference ob, Borro
100109
}
101110
var info = self.info.Value;
102111

103-
MethodInfo setter = self.setter.UnsafeValue;
112+
MethodInfo? setter = self.setter;
104113

105114
if (val == null)
106115
{
@@ -141,7 +150,7 @@ public static int tp_descr_set(BorrowedReference ds, BorrowedReference ob, Borro
141150
Exceptions.RaiseTypeError("invalid target");
142151
return -1;
143152
}
144-
info.SetValue(co.inst, newval, null);
153+
setter.Invoke(co.inst, new object?[] { newval });
145154
}
146155
else
147156
{
@@ -169,5 +178,10 @@ public static NewReference tp_repr(BorrowedReference ob)
169178
var self = (PropertyObject)GetManagedObject(ob)!;
170179
return Runtime.PyString_FromString($"<property '{self.info}'>");
171180
}
181+
182+
void IDeserializationCallback.OnDeserialization(object sender)
183+
{
184+
CacheAccessors();
185+
}
172186
}
173187
}

0 commit comments

Comments
 (0)