diff --git a/CHANGELOG.md b/CHANGELOG.md index 86c2a808a..16489a9c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ details about the cause of the failure - floating point values passed from Python are no longer silently truncated when .NET expects an integer [#1342][i1342] - More specific error messages for method argument mismatch +- BREAKING: when inheriting from .NET types in Python if you override `__init__` you +must explicitly call base constructor using `super().__init__(.....)`. Not doing so will lead +to undefined behavior. - BREAKING: most `PyScope` methods will never return `null`. Instead, `PyObject` `None` will be returned. - BREAKING: `PyScope` was renamed to `PyModule` - BREAKING: Methods with `ref` or `out` parameters and void return type return a tuple of only the `ref` and `out` parameters. @@ -85,6 +88,7 @@ Instead, `PyIterable` does that. ### Fixed - Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash +- Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238]) - Fix incorrect dereference in params array handling - Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097]) - Fix `object[]` parameters taking precedence when should not in overload resolution @@ -874,3 +878,4 @@ This version improves performance on benchmarks significantly compared to 2.3. [p534]: https://github.com/pythonnet/pythonnet/pull/534 [i449]: https://github.com/pythonnet/pythonnet/issues/449 [i1342]: https://github.com/pythonnet/pythonnet/issues/1342 +[i238]: https://github.com/pythonnet/pythonnet/issues/238 diff --git a/src/embed_tests/StateSerialization/MethodSerialization.cs b/src/embed_tests/StateSerialization/MethodSerialization.cs index 0e584fc37..80b7a08ee 100644 --- a/src/embed_tests/StateSerialization/MethodSerialization.cs +++ b/src/embed_tests/StateSerialization/MethodSerialization.cs @@ -19,6 +19,16 @@ public void GenericRoundtrip() Assert.AreEqual(method, restored.Value); } + [Test] + public void ConstrctorRoundtrip() + { + var ctor = typeof(MethodTestHost).GetConstructor(new[] { typeof(int) }); + var maybeConstructor = new MaybeMethodBase(ctor); + var restored = SerializationRoundtrip(maybeConstructor); + Assert.IsTrue(restored.Valid); + Assert.AreEqual(ctor, restored.Value); + } + static T SerializationRoundtrip(T item) { using var buf = new MemoryStream(); @@ -31,5 +41,6 @@ static T SerializationRoundtrip(T item) public class MethodTestHost { + public MethodTestHost(int _) { } public void Generic(T item, T[] array, ref T @ref) { } } diff --git a/src/runtime/NewReference.cs b/src/runtime/NewReference.cs index bbd021ad3..91ebbdb01 100644 --- a/src/runtime/NewReference.cs +++ b/src/runtime/NewReference.cs @@ -41,6 +41,17 @@ public PyObject MoveToPyObject() return new PyObject(this.StealNullable()); } + /// + /// Creates new instance of which now owns the pointer. + /// Sets the original reference to null, as it no longer owns the pointer. + /// + public NewReference Move() + { + var result = new NewReference(this); + this.pointer = default; + return result; + } + /// Moves ownership of this instance to unmanged pointer public IntPtr DangerousMoveToPointer() { diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 349d20b6b..028788742 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -556,6 +556,17 @@ public virtual void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsH } } + public virtual bool HasCustomNew() => this.GetType().GetMethod("tp_new") is not null; + + public override bool Init(BorrowedReference obj, BorrowedReference args, BorrowedReference kw) + { + if (this.HasCustomNew()) + // initialization must be done in tp_new + return true; + + return base.Init(obj, args, kw); + } + protected virtual void OnDeserialization(object sender) { this.dotNetMembers = new List(); diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index 5b9e630ca..47c9b4e0e 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -50,23 +50,26 @@ internal ClassDerivedObject(Type tp) : base(tp) { } - /// - /// Implements __new__ for derived classes of reflected classes. - /// - public new static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw) + protected override NewReference NewObjectToPython(object obj, BorrowedReference tp) { - var cls = (ClassDerivedObject)GetManagedObject(tp)!; + var self = base.NewObjectToPython(obj, tp); - // call the managed constructor - object? obj = cls.binder.InvokeRaw(null, args, kw); - if (obj == null) + SetPyObj((IPythonDerivedType)obj, self.Borrow()); + + // Decrement the python object's reference count. + // This doesn't actually destroy the object, it just sets the reference to this object + // to be a weak reference and it will be destroyed when the C# object is destroyed. + if (!self.IsNull()) { - return default; + Runtime.XDecref(self.Steal()); } - // return the pointer to the python object - // (this indirectly calls ClassDerivedObject.ToPython) - return Converter.ToPython(obj, cls.GetType()); + return Converter.ToPython(obj, type.Value); + } + + protected override void SetTypeNewSlot(BorrowedReference pyType, SlotsHolder slotsHolder) + { + // Python derived types rely on base tp_new and overridden __init__ } public new static void tp_dealloc(NewReference ob) @@ -824,37 +827,24 @@ public static void InvokeSetProperty(IPythonDerivedType obj, string propertyN public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args) { + var selfRef = GetPyObj(obj); + if (selfRef.Ref == null) + { + // this might happen when the object is created from .NET + using var _ = Py.GIL(); + // In the end we decrement the python object's reference count. + // This doesn't actually destroy the object, it just sets the reference to this object + // to be a weak reference and it will be destroyed when the C# object is destroyed. + using var self = CLRObject.GetReference(obj, obj.GetType()); + SetPyObj(obj, self.Borrow()); + } + // call the base constructor obj.GetType().InvokeMember(origCtorName, BindingFlags.InvokeMethod, null, obj, args); - - NewReference self = default; - PyGILState gs = Runtime.PyGILState_Ensure(); - try - { - // create the python object - var type = ClassManager.GetClass(obj.GetType()); - self = CLRObject.GetReference(obj, type); - - // set __pyobj__ to self and deref the python object which will allow this - // object to be collected. - SetPyObj(obj, self.Borrow()); - } - finally - { - // Decrement the python object's reference count. - // This doesn't actually destroy the object, it just sets the reference to this object - // to be a weak reference and it will be destroyed when the C# object is destroyed. - if (!self.IsNull()) - { - Runtime.XDecref(self.Steal()); - } - - Runtime.PyGILState_Release(gs); - } } public static void PyFinalize(IPythonDerivedType obj) @@ -890,7 +880,7 @@ internal static UnsafeReferenceWithRun GetPyObj(IPythonDerivedType obj) return (UnsafeReferenceWithRun)fi.GetValue(obj); } - static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj) + internal static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj) { FieldInfo fi = GetPyObjField(obj.GetType())!; fi.SetValue(obj, new UnsafeReferenceWithRun(pyObj)); diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index bfc07874f..f8e108f41 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -210,7 +210,7 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p // information, including generating the member descriptors // that we'll be putting in the Python class __dict__. - ClassInfo info = GetClassInfo(type); + ClassInfo info = GetClassInfo(type, impl); impl.indexer = info.indexer; impl.richcompare.Clear(); @@ -252,16 +252,17 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p // required that the ClassObject.ctors be changed to internal if (co != null) { - if (co.NumCtors > 0) + if (co.NumCtors > 0 && !co.HasCustomNew()) { // Implement Overloads on the class object if (!CLRModule._SuppressOverloads) { - using var ctors = new ConstructorBinding(type, pyType, co.binder).Alloc(); - // ExtensionType types are untracked, so don't Incref() them. + // HACK: __init__ points to instance constructors. + // When unbound they fully instantiate object, so we get overloads for free from MethodBinding. + var init = info.members["__init__"]; // TODO: deprecate __overloads__ soon... - Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, ctors.Borrow()); - Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, ctors.Borrow()); + Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, init); + Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, init); } // don't generate the docstring if one was already set from a DocStringAttribute. @@ -320,10 +321,10 @@ internal static bool ShouldBindEvent(EventInfo ei) return ShouldBindMethod(ei.GetAddMethod(true)); } - private static ClassInfo GetClassInfo(Type type) + private static ClassInfo GetClassInfo(Type type, ClassBase impl) { var ci = new ClassInfo(); - var methods = new Dictionary>(); + var methods = new Dictionary>(); MethodInfo meth; ExtensionType ob; string name; @@ -420,13 +421,33 @@ private static ClassInfo GetClassInfo(Type type) continue; } name = meth.Name; + + //TODO mangle? + if (name == "__init__" && !impl.HasCustomNew()) + continue; + if (!methods.TryGetValue(name, out var methodList)) { - methodList = methods[name] = new List(); + methodList = methods[name] = new List(); } methodList.Add(meth); continue; + case MemberTypes.Constructor when !impl.HasCustomNew(): + var ctor = (ConstructorInfo)mi; + if (ctor.IsStatic) + { + continue; + } + + name = "__init__"; + if (!methods.TryGetValue(name, out methodList)) + { + methodList = methods[name] = new List(); + } + methodList.Add(ctor); + continue; + case MemberTypes.Property: var pi = (PropertyInfo)mi; diff --git a/src/runtime/classobject.cs b/src/runtime/classobject.cs index 6a5c17236..5ba83c25e 100644 --- a/src/runtime/classobject.cs +++ b/src/runtime/classobject.cs @@ -1,6 +1,8 @@ using System; +using System.Diagnostics; using System.Linq; using System.Reflection; +using System.Runtime.Serialization; namespace Python.Runtime { @@ -13,18 +15,12 @@ namespace Python.Runtime [Serializable] internal class ClassObject : ClassBase { - internal ConstructorBinder binder; - internal int NumCtors = 0; + internal readonly int NumCtors = 0; internal ClassObject(Type tp) : base(tp) { var _ctors = type.Value.GetConstructors(); NumCtors = _ctors.Length; - binder = new ConstructorBinder(type.Value); - foreach (ConstructorInfo t in _ctors) - { - binder.AddMethod(t); - } } @@ -33,7 +29,12 @@ internal ClassObject(Type tp) : base(tp) /// internal NewReference GetDocString() { - MethodBase[] methods = binder.GetMethods(); + if (!type.Valid) + { + return Exceptions.RaiseTypeError(type.DeletedMessage); + } + + MethodBase[] methods = type.Value.GetConstructors(); var str = ""; foreach (MethodBase t in methods) { @@ -50,7 +51,7 @@ internal NewReference GetDocString() /// /// Implements __new__ for reflected classes and value types. /// - public static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw) + static NewReference tp_new_impl(BorrowedReference tp, BorrowedReference args, BorrowedReference kw) { var self = GetManagedObject(tp) as ClassObject; @@ -100,15 +101,49 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args, return NewEnum(type, args, tp); } - object? obj = self.binder.InvokeRaw(null, args, kw); - if (obj == null) + if (IsGenericNullable(type)) { - return default; + // Nullable has special handling in .NET runtime. + // Invoking its constructor via reflection on an uninitialized instance + // does not actually set the object fields. + return NewNullable(type, args, kw, tp); } - return CLRObject.GetReference(obj, tp); + object obj = FormatterServices.GetUninitializedObject(type); + + return self.NewObjectToPython(obj, tp); + } + + protected virtual void SetTypeNewSlot(BorrowedReference pyType, SlotsHolder slotsHolder) + { + TypeManager.InitializeSlotIfEmpty(pyType, TypeOffset.tp_new, new Interop.BBB_N(tp_new_impl), slotsHolder); + } + + public override bool HasCustomNew() + { + if (base.HasCustomNew()) return true; + + Type clrType = type.Value; + return clrType.IsPrimitive + || clrType.IsEnum + || clrType == typeof(string) + || IsGenericNullable(clrType); } + static bool IsGenericNullable(Type type) + => type.IsValueType && type.IsGenericType + && type.GetGenericTypeDefinition() == typeof(Nullable<>); + + public override void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsHolder) + { + base.InitializeSlots(pyType, slotsHolder); + + this.SetTypeNewSlot(pyType, slotsHolder); + } + + protected virtual NewReference NewObjectToPython(object obj, BorrowedReference tp) + => CLRObject.GetReference(obj, tp); + private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedReference tp) { nint argCount = Runtime.PyTuple_Size(args); @@ -146,6 +181,28 @@ private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedR return CLRObject.GetReference(enumValue, tp); } + private static NewReference NewNullable(Type type, BorrowedReference args, BorrowedReference kw, BorrowedReference tp) + { + Debug.Assert(IsGenericNullable(type)); + + if (kw != null) + { + return Exceptions.RaiseTypeError("System.Nullable constructor does not support keyword arguments"); + } + + nint argsCount = Runtime.PyTuple_Size(args); + if (argsCount != 1) + { + return Exceptions.RaiseTypeError("System.Nullable constructor expects 1 argument, got " + (int)argsCount); + } + + var value = Runtime.PyTuple_GetItem(args, 0); + var elementType = type.GetGenericArguments()[0]; + return Converter.ToManaged(value, elementType, out var result, setError: true) + ? CLRObject.GetReference(result!, tp) + : default; + } + /// /// Implementation of [] semantics for reflected types. This exists diff --git a/src/runtime/constructorbinder.cs b/src/runtime/constructorbinder.cs deleted file mode 100644 index 4868c5f1a..000000000 --- a/src/runtime/constructorbinder.cs +++ /dev/null @@ -1,138 +0,0 @@ -using System; -using System.Reflection; -using System.Text; - -namespace Python.Runtime -{ - /// - /// A ConstructorBinder encapsulates information about one or more managed - /// constructors, and is responsible for selecting the right constructor - /// given a set of Python arguments. This is slightly different than the - /// standard MethodBinder because of a difference in invoking constructors - /// using reflection (which is seems to be a CLR bug). - /// - [Serializable] - internal class ConstructorBinder : MethodBinder - { - private MaybeType _containingType; - - internal ConstructorBinder(Type containingType) - { - _containingType = containingType; - } - - /// - /// Constructors get invoked when an instance of a wrapped managed - /// class or a subclass of a managed class is created. This differs - /// from the MethodBinder implementation in that we return the raw - /// result of the constructor rather than wrapping it as a Python - /// object - the reason is that only the caller knows the correct - /// Python type to use when wrapping the result (may be a subclass). - /// - internal object? InvokeRaw(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) - { - return InvokeRaw(inst, args, kw, null); - } - - /// - /// Allows ctor selection to be limited to a single attempt at a - /// match by providing the MethodBase to use instead of searching - /// the entire MethodBinder.list (generic ArrayList) - /// - /// (possibly null) instance - /// PyObject* to the arg tuple - /// PyObject* to the keyword args dict - /// The sole ContructorInfo to use or null - /// The result of the constructor call with converted params - /// - /// 2010-07-24 BC: I added the info parameter to the call to Bind() - /// Binding binding = this.Bind(inst, args, kw, info); - /// to take advantage of Bind()'s ability to use a single MethodBase (CI or MI). - /// - internal object? InvokeRaw(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info) - { - if (!_containingType.Valid) - { - Exceptions.RaiseTypeError(_containingType.DeletedMessage); - return null; - } - object result; - Type tp = _containingType.Value; - - if (tp.IsValueType && !tp.IsPrimitive && - !tp.IsEnum && tp != typeof(decimal) && - Runtime.PyTuple_Size(args) == 0) - { - // If you are trying to construct an instance of a struct by - // calling its default constructor, that ConstructorInfo - // instance will not appear in reflection and the object must - // instead be constructed via a call to - // Activator.CreateInstance(). - try - { - result = Activator.CreateInstance(tp); - } - catch (Exception e) - { - if (e.InnerException != null) - { - e = e.InnerException; - } - Exceptions.SetError(e); - return null; - } - return result; - } - - Binding? binding = Bind(inst, args, kw, info); - - if (binding == null) - { - // It is possible for __new__ to be invoked on construction - // of a Python subclass of a managed class, so args may - // reflect more args than are required to instantiate the - // class. So if we cant find a ctor that matches, we'll see - // if there is a default constructor and, if so, assume that - // any extra args are intended for the subclass' __init__. - - using var eargs = Runtime.PyTuple_New(0); - binding = Bind(inst, eargs.BorrowOrThrow(), kw: null); - - if (binding == null) - { - var errorMessage = new StringBuilder("No constructor matches given arguments"); - if (info != null && info.IsConstructor && info.DeclaringType != null) - { - errorMessage.Append(" for ").Append(info.DeclaringType.Name); - } - - errorMessage.Append(": "); - Runtime.PyErr_Fetch(out var errType, out var errVal, out var errTrace); - AppendArgumentTypes(to: errorMessage, args); - Runtime.PyErr_Restore(errType.StealNullable(), errVal.StealNullable(), errTrace.StealNullable()); - Exceptions.RaiseTypeError(errorMessage.ToString()); - return null; - } - } - - // Fire the selected ctor and catch errors... - var ci = (ConstructorInfo)binding.info; - // Object construction is presumed to be non-blocking and fast - // enough that we shouldn't really need to release the GIL. - try - { - result = ci.Invoke(binding.args); - } - catch (Exception e) - { - if (e.InnerException != null) - { - e = e.InnerException; - } - Exceptions.SetError(e); - return null; - } - return result; - } - } -} diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs deleted file mode 100644 index 4f82c7728..000000000 --- a/src/runtime/constructorbinding.cs +++ /dev/null @@ -1,222 +0,0 @@ -using System; -using System.Diagnostics; -using System.Reflection; - -namespace Python.Runtime -{ - /// - /// Implements a Python type that wraps a CLR ctor call. Constructor objects - /// support a .Overloads[] syntax to allow explicit ctor overload selection. - /// - /// - /// ClassManager stores a ConstructorBinding instance in the class's __dict__['Overloads'] - /// SomeType.Overloads[Type, ...] works like this: - /// 1) Python retrieves the Overloads attribute from this ClassObject's dictionary normally - /// and finds a non-null tp_descr_get slot which is called by the interpreter - /// and returns an IncRef()ed pyHandle to itself. - /// 2) The ConstructorBinding object handles the [] syntax in its mp_subscript by matching - /// the Type object parameters to a constructor overload using Type.GetConstructor() - /// [NOTE: I don't know why method overloads are not searched the same way.] - /// and creating the BoundContructor object which contains ContructorInfo object. - /// 3) In tp_call, if ctorInfo is not null, ctorBinder.InvokeRaw() is called. - /// - [Serializable] - internal class ConstructorBinding : ExtensionType - { - private MaybeType type; // The managed Type being wrapped in a ClassObject - private ConstructorBinder ctorBinder; - - [NonSerialized] - private PyObject? repr; - - public ConstructorBinding(Type type, ReflectedClrType typeToCreate, ConstructorBinder ctorBinder) - { - this.type = type; - Debug.Assert(typeToCreate == ReflectedClrType.GetOrCreate(type)); - this.ctorBinder = ctorBinder; - } - - /// - /// Descriptor __get__ implementation. - /// Implements a Python type that wraps a CLR ctor call that requires the use - /// of a .Overloads[pyTypeOrType...] syntax to allow explicit ctor overload - /// selection. - /// - /// PyObject* to a Constructors wrapper - /// - /// the instance that the attribute was accessed through, - /// or None when the attribute is accessed through the owner - /// - /// always the owner class - /// - /// a CtorMapper (that borrows a reference to this python type and the - /// ClassObject's ConstructorBinder) wrapper. - /// - /// - /// Python 2.6.5 docs: - /// object.__get__(self, instance, owner) - /// Called to get the attribute of the owner class (class attribute access) - /// or of an instance of that class (instance attribute access). - /// owner is always the owner class, while instance is the instance that - /// the attribute was accessed through, or None when the attribute is accessed through the owner. - /// This method should return the (computed) attribute value or raise an AttributeError exception. - /// - public static NewReference tp_descr_get(BorrowedReference op, BorrowedReference instance, BorrowedReference owner) - { - var self = (ConstructorBinding?)GetManagedObject(op); - if (self == null) - { - Exceptions.SetError(Exceptions.AssertionError, "attempting to access destroyed object"); - return default; - } - - // It doesn't seem to matter if it's accessed through an instance (rather than via the type). - /*if (instance != IntPtr.Zero) { - // This is ugly! PyObject_IsInstance() returns 1 for true, 0 for false, -1 for error... - if (Runtime.PyObject_IsInstance(instance, owner) < 1) { - return Exceptions.RaiseTypeError("How in the world could that happen!"); - } - }*/ - return new NewReference(op); - } - - /// - /// Implement explicit overload selection using subscript syntax ([]). - /// - /// - /// ConstructorBinding.GetItem(PyObject *o, PyObject *key) - /// Return element of o corresponding to the object key or NULL on failure. - /// This is the equivalent of the Python expression o[key]. - /// - public static NewReference mp_subscript(BorrowedReference op, BorrowedReference key) - { - var self = (ConstructorBinding)GetManagedObject(op)!; - if (!self.type.Valid) - { - return Exceptions.RaiseTypeError(self.type.DeletedMessage); - } - Type tp = self.type.Value; - - Type[]? types = Runtime.PythonArgsToTypeArray(key); - if (types == null) - { - return Exceptions.RaiseTypeError("type(s) expected"); - } - //MethodBase[] methBaseArray = self.ctorBinder.GetMethods(); - //MethodBase ci = MatchSignature(methBaseArray, types); - ConstructorInfo ci = tp.GetConstructor(types); - if (ci == null) - { - return Exceptions.RaiseTypeError("No match found for constructor signature"); - } - var boundCtor = new BoundContructor(tp, self.ctorBinder, ci); - return boundCtor.Alloc(); - } - - /// - /// ConstructorBinding __repr__ implementation [borrowed from MethodObject]. - /// - public static NewReference tp_repr(BorrowedReference ob) - { - var self = (ConstructorBinding)GetManagedObject(ob)!; - if (self.repr is not null) - { - return new NewReference(self.repr); - } - MethodBase[] methods = self.ctorBinder.GetMethods(); - - if (!self.type.Valid) - { - return Exceptions.RaiseTypeError(self.type.DeletedMessage); - } - string name = self.type.Value.FullName; - var doc = ""; - foreach (MethodBase t in methods) - { - if (doc.Length > 0) - { - doc += "\n"; - } - string str = t.ToString(); - int idx = str.IndexOf("("); - doc += string.Format("{0}{1}", name, str.Substring(idx)); - } - using var docStr = Runtime.PyString_FromString(doc); - if (docStr.IsNull()) return default; - self.repr = docStr.MoveToPyObject(); - return new NewReference(self.repr); - } - } - - /// - /// Implements a Python type that constructs the given Type given a particular ContructorInfo. - /// - /// - /// Here mostly because I wanted a new __repr__ function for the selected constructor. - /// An earlier implementation hung the __call__ on the ContructorBinding class and - /// returned an Incref()ed self.pyHandle from the __get__ function. - /// - [Serializable] - internal class BoundContructor : ExtensionType - { - private Type type; // The managed Type being wrapped in a ClassObject - private ConstructorBinder ctorBinder; - private ConstructorInfo ctorInfo; - private PyObject? repr; - - public BoundContructor(Type type, ConstructorBinder ctorBinder, ConstructorInfo ci) - { - this.type = type; - this.ctorBinder = ctorBinder; - ctorInfo = ci; - } - - /// - /// BoundContructor.__call__(PyObject *callable_object, PyObject *args, PyObject *kw) - /// - /// PyObject *callable_object - /// PyObject *args - /// PyObject *kw - /// A reference to a new instance of the class by invoking the selected ctor(). - public static NewReference tp_call(BorrowedReference op, BorrowedReference args, BorrowedReference kw) - { - var self = (BoundContructor)GetManagedObject(op)!; - // Even though a call with null ctorInfo just produces the old behavior - /*if (self.ctorInfo == null) { - string msg = "Usage: Class.Overloads[CLR_or_python_Type, ...]"; - return Exceptions.RaiseTypeError(msg); - }*/ - // Bind using ConstructorBinder.Bind and invoke the ctor providing a null instancePtr - // which will fire self.ctorInfo using ConstructorInfo.Invoke(). - object? obj = self.ctorBinder.InvokeRaw(null, args, kw, self.ctorInfo); - if (obj == null) - { - // XXX set an error - return default; - } - // Instantiate the python object that wraps the result of the method call - // and return the PyObject* to it. - return CLRObject.GetReference(obj, ReflectedClrType.GetOrCreate(self.type)); - } - - /// - /// BoundContructor __repr__ implementation [borrowed from MethodObject]. - /// - public static NewReference tp_repr(BorrowedReference ob) - { - var self = (BoundContructor)GetManagedObject(ob)!; - if (self.repr is not null) - { - return new NewReference(self.repr); - } - string name = self.type.FullName; - string str = self.ctorInfo.ToString(); - int idx = str.IndexOf("("); - str = string.Format("returns a new {0}{1}", name, str.Substring(idx)); - using var docStr = Runtime.PyString_FromString(str); - if (docStr.IsNull()) return default; - self.repr = docStr.MoveToPyObject(); - return new NewReference(self.repr); - } - } -} diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 479e7a5d5..5cf845155 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -76,6 +76,15 @@ internal ExceptionClassObject(Type tp) : base(tp) } return Runtime.PyString_FromString(message); } + + public override bool Init(BorrowedReference obj, BorrowedReference args, BorrowedReference kw) + { + if (!base.Init(obj, args, kw)) return false; + + var e = (CLRObject)GetManagedObject(obj)!; + + return Exceptions.SetArgsAndCause(obj, (Exception)e.inst); + } } /// @@ -149,7 +158,7 @@ internal static void Shutdown() /// __getattr__ implementation, and thus dereferencing a NULL /// pointer. /// - internal static void SetArgsAndCause(BorrowedReference ob, Exception e) + internal static bool SetArgsAndCause(BorrowedReference ob, Exception e) { NewReference args; if (!string.IsNullOrEmpty(e.Message)) @@ -167,8 +176,7 @@ internal static void SetArgsAndCause(BorrowedReference ob, Exception e) { if (Runtime.PyObject_SetAttrString(ob, "args", args.Borrow()) != 0) { - args.Dispose(); - throw PythonException.ThrowLastAsClrException(); + return false; } } @@ -178,6 +186,8 @@ internal static void SetArgsAndCause(BorrowedReference ob, Exception e) using var cause = CLRObject.GetReference(e.InnerException); Runtime.PyException_SetCause(ob, cause.Steal()); } + + return true; } /// diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index c71529628..2ed9d7970 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -122,6 +122,28 @@ internal void Load(BorrowedReference ob, Dictionary? context) protected virtual Dictionary? OnSave(BorrowedReference ob) => null; protected virtual void OnLoad(BorrowedReference ob, Dictionary? context) { } + /// + /// Initializes given object, or returns false and sets Python error on failure + /// + public virtual bool Init(BorrowedReference obj, BorrowedReference args, BorrowedReference kw) + { + // this just calls obj.__init__(*args, **kw) + using var init = Runtime.PyObject_GetAttr(obj, PyIdentifier.__init__); + Runtime.PyErr_Clear(); + + if (!init.IsNull()) + { + using var result = Runtime.PyObject_Call(init.Borrow(), args, kw); + + if (result.IsNull()) + { + return false; + } + } + + return true; + } + protected static void ClearObjectDict(BorrowedReference ob) { BorrowedReference type = Runtime.PyObject_TYPE(ob); diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index c51ce1a22..7558269b4 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -197,41 +197,25 @@ public static void tp_free(NewReference tp) /// public static NewReference tp_call(BorrowedReference tp, BorrowedReference args, BorrowedReference kw) { - IntPtr func = Util.ReadIntPtr(tp, TypeOffset.tp_new); - if (func == IntPtr.Zero) + IntPtr tp_new = Util.ReadIntPtr(tp, TypeOffset.tp_new); + if (tp_new == IntPtr.Zero) { return Exceptions.RaiseTypeError("invalid object"); } - using NewReference obj = NativeCall.Call_3(func, tp, args, kw); + using NewReference obj = NativeCall.Call_3(tp_new, tp, args, kw); if (obj.IsNull()) { return default; } - BorrowedReference objOrNull = CallInit(obj.Borrow(), args, kw); - return new NewReference(objOrNull, canBeNull: true); - } - - private static BorrowedReference CallInit(BorrowedReference obj, BorrowedReference args, BorrowedReference kw) - { - using var init = Runtime.PyObject_GetAttr(obj, PyIdentifier.__init__); - Runtime.PyErr_Clear(); - - if (!init.IsNull()) - { - using var result = Runtime.PyObject_Call(init.Borrow(), args, kw); + var type = GetManagedObject(tp)!; - if (result.IsNull()) - { - return default; - } - } - - return obj; + return type.Init(obj.Borrow(), args, kw) + ? obj.Move() + : default; } - /// /// Type __setattr__ implementation for reflected types. Note that this /// is slightly different than the standard setattr implementation for diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 1c5da07c5..95c3aaa4a 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -55,14 +55,14 @@ internal void AddMethod(MethodBase m) /// Given a sequence of MethodInfo and a sequence of types, return the /// MethodInfo that matches the signature represented by those types. /// - internal static MethodInfo? MatchSignature(MethodInfo[] mi, Type[] tp) + internal static MethodBase? MatchSignature(MethodBase[] mi, Type[] tp) { if (tp == null) { return null; } int count = tp.Length; - foreach (MethodInfo t in mi) + foreach (MethodBase t in mi) { ParameterInfo[] pi = t.GetParameters(); if (pi.Length != count) @@ -89,7 +89,7 @@ internal void AddMethod(MethodBase m) /// return the MethodInfo that represents the matching closed generic. /// If unsuccessful, returns null and may set a Python error. /// - internal static MethodInfo? MatchParameters(MethodInfo[] mi, Type[]? tp) + internal static MethodInfo? MatchParameters(MethodBase[] mi, Type[]? tp) { if (tp == null) { @@ -128,7 +128,7 @@ internal void AddMethod(MethodBase m) /// Given a sequence of MethodInfo and two sequences of type parameters, /// return the MethodInfo that matches the signature and the closed generic. /// - internal static MethodInfo? MatchSignatureAndParameters(MethodInfo[] mi, Type[] genericTp, Type[] sigTp) + internal static MethodInfo? MatchSignatureAndParameters(MethodBase[] mi, Type[] genericTp, Type[] sigTp) { if (genericTp == null || sigTp == null) { @@ -364,7 +364,7 @@ public MismatchedMethod(Exception exception, MethodBase mb) /// If not null, only bind to that method. /// If not null, additionally attempt to bind to the generic methods in this array by inferring generic type parameters. /// A Binding if successful. Otherwise null. - internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodInfo[]? methodinfo) + internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) { // loop to find match, return invoker w/ or w/o error var kwargDict = new Dictionary(); @@ -873,7 +873,7 @@ protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference ar to.Append(')'); } - internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodInfo[]? methodinfo) + internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, MethodBase? info, MethodBase[]? methodinfo) { // No valid methods, nothing to bind. if (GetMethods().Length == 0) @@ -943,20 +943,20 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a // we return the out parameter as the result to Python (for // code compatibility with ironpython). - var mi = (MethodInfo)binding.info; + var returnType = binding.info.IsConstructor ? typeof(void) : ((MethodInfo)binding.info).ReturnType; if (binding.outs > 0) { - ParameterInfo[] pi = mi.GetParameters(); + ParameterInfo[] pi = binding.info.GetParameters(); int c = pi.Length; var n = 0; - bool isVoid = mi.ReturnType == typeof(void); + bool isVoid = returnType == typeof(void); int tupleSize = binding.outs + (isVoid ? 0 : 1); using var t = Runtime.PyTuple_New(tupleSize); if (!isVoid) { - using var v = Converter.ToPython(result, mi.ReturnType); + using var v = Converter.ToPython(result, returnType); Runtime.PyTuple_SetItem(t.Borrow(), n, v.Steal()); n++; } @@ -972,7 +972,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a } } - if (binding.outs == 1 && mi.ReturnType == typeof(void)) + if (binding.outs == 1 && returnType == typeof(void)) { BorrowedReference item = Runtime.PyTuple_GetItem(t.Borrow(), 0); return new NewReference(item); @@ -981,7 +981,7 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a return new NewReference(t.Borrow()); } - return Converter.ToPython(result, mi.ReturnType); + return Converter.ToPython(result, returnType); } } diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs index 8dcd985d0..d9bf3aec6 100644 --- a/src/runtime/methodbinding.cs +++ b/src/runtime/methodbinding.cs @@ -1,11 +1,12 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Reflection; namespace Python.Runtime { - using MaybeMethodInfo = MaybeMethodBase; + using MaybeMethodInfo = MaybeMethodBase; /// /// Implements a Python binding type for CLR methods. These work much like /// standard Python method bindings, but the same type is used to bind @@ -42,7 +43,9 @@ public static NewReference mp_subscript(BorrowedReference tp, BorrowedReference return Exceptions.RaiseTypeError("type(s) expected"); } - MethodInfo? mi = MethodBinder.MatchParameters(self.m.info, types); + MethodBase? mi = self.m.IsInstanceConstructor + ? self.m.type.Value.GetConstructor(types) + : MethodBinder.MatchParameters(self.m.info, types); if (mi == null) { return Exceptions.RaiseTypeError("No match found for given type params"); @@ -63,10 +66,9 @@ PyObject Signature infos = infos.Where(info => info.DeclaringType == type).ToArray(); // this is a primitive version // the overload with the maximum number of parameters should be used - MethodInfo primary = infos.OrderByDescending(i => i.GetParameters().Length).First(); + MethodBase primary = infos.OrderByDescending(i => i.GetParameters().Length).First(); var primaryParameters = primary.GetParameters(); PyObject signatureClass = Runtime.InspectModule.GetAttr("Signature"); - var primaryReturn = primary.ReturnParameter; using var parameters = new PyList(); using var parameterClass = primaryParameters.Length > 0 ? Runtime.InspectModule.GetAttr("Parameter") : null; diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index afbcaf631..397547616 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -5,7 +5,7 @@ namespace Python.Runtime { - using MaybeMethodInfo = MaybeMethodBase; + using MaybeMethodInfo = MaybeMethodBase; /// /// Implements a Python type that represents a CLR method. Method objects @@ -18,7 +18,7 @@ namespace Python.Runtime internal class MethodObject : ExtensionType { [NonSerialized] - private MethodInfo[]? _info = null; + private MethodBase[]? _info = null; private readonly List infoList; internal string name; internal readonly MethodBinder binder; @@ -27,13 +27,13 @@ internal class MethodObject : ExtensionType internal PyString? doc; internal MaybeType type; - public MethodObject(Type type, string name, MethodInfo[] info, bool allow_threads = MethodBinder.DefaultAllowThreads) + public MethodObject(Type type, string name, MethodBase[] info, bool allow_threads = MethodBinder.DefaultAllowThreads) { this.type = type; this.name = name; this.infoList = new List(); binder = new MethodBinder(); - foreach (MethodInfo item in info) + foreach (MethodBase item in info) { this.infoList.Add(item); binder.AddMethod(item); @@ -45,7 +45,9 @@ public MethodObject(Type type, string name, MethodInfo[] info, bool allow_thread binder.allow_threads = allow_threads; } - internal MethodInfo[] info + public bool IsInstanceConstructor => name == "__init__"; + + internal MethodBase[] info { get { diff --git a/src/runtime/operatormethod.cs b/src/runtime/operatormethod.cs index aad3f013f..035198f3e 100644 --- a/src/runtime/operatormethod.cs +++ b/src/runtime/operatormethod.cs @@ -86,7 +86,7 @@ public static bool IsOperatorMethod(MethodBase method) return OpMethodMap.ContainsKey(method.Name) || ComparisonOpMap.ContainsKey(method.Name); } - public static bool IsComparisonOp(MethodInfo method) + public static bool IsComparisonOp(MethodBase method) { return ComparisonOpMap.ContainsKey(method.Name); } @@ -170,7 +170,7 @@ public static string ReversePyMethodName(string pyName) /// /// The operator method. /// - public static bool IsReverse(MethodInfo method) + public static bool IsReverse(MethodBase method) { Type primaryType = method.IsOpsHelper() ? method.DeclaringType.GetGenericArguments()[0] @@ -179,10 +179,10 @@ public static bool IsReverse(MethodInfo method) return leftOperandType != primaryType; } - public static void FilterMethods(MethodInfo[] methods, out MethodInfo[] forwardMethods, out MethodInfo[] reverseMethods) + public static void FilterMethods(MethodBase[] methods, out MethodBase[] forwardMethods, out MethodBase[] reverseMethods) { - List forwardMethodsList = new List(); - List reverseMethodsList = new List(); + var forwardMethodsList = new List(); + var reverseMethodsList = new List(); foreach (var method in methods) { if (IsReverse(method)) diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs index c75d38574..20939f4c6 100644 --- a/src/runtime/overload.cs +++ b/src/runtime/overload.cs @@ -35,7 +35,7 @@ public static NewReference mp_subscript(BorrowedReference tp, BorrowedReference return Exceptions.RaiseTypeError("type(s) expected"); } - MethodInfo? mi = MethodBinder.MatchSignature(self.m.info, types); + MethodBase? mi = MethodBinder.MatchSignature(self.m.info, types); if (mi == null) { var e = "No match found for signature"; diff --git a/src/testing/constructortests.cs b/src/testing/constructortests.cs index 4dc7f04d8..732692b3a 100644 --- a/src/testing/constructortests.cs +++ b/src/testing/constructortests.cs @@ -38,6 +38,16 @@ public StructConstructorTest(Guid v) } } + public struct GenericStructConstructorTest where T : struct + { + public T Value; + + public GenericStructConstructorTest(T value) + { + this.Value = value; + } + } + public class SubclassConstructorTest { @@ -66,4 +76,11 @@ public MultipleConstructorsTest(string s, params Type[] tp) type = tp; } } + + public class DefaultConstructorMatching + { + public double a; + public DefaultConstructorMatching() { a = 1; } + public DefaultConstructorMatching(double a) { this.a = a; } + } } diff --git a/tests/test_class.py b/tests/test_class.py index f961b3975..f63f05f4d 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -108,19 +108,21 @@ def test_subclass_with_various_constructors(): class SubClass(ClassCtorTest2): def __init__(self, v): - ClassCtorTest2.__init__(self) - self.value = v + ClassCtorTest2.__init__(self, v) + self.value2 = v inst = SubClass('test') assert inst.value == 'test' + assert inst.value2 == 'test' class SubClass2(ClassCtorTest2): def __init__(self, v): - ClassCtorTest2.__init__(self) - self.value = v + ClassCtorTest2.__init__(self, v) + self.value2 = v inst = SubClass2('test') assert inst.value == 'test' + assert inst.value2 == 'test' def test_struct_construction(): @@ -128,9 +130,9 @@ def test_struct_construction(): from Python.Test import Point - p = Point() - assert p.X == 0 - assert p.Y == 0 + # no default constructor, must supply arguments + with pytest.raises(TypeError): + p = Point() p = Point(0, 0) assert p.X == 0 diff --git a/tests/test_constructors.py b/tests/test_constructors.py index c305377f3..8e7ef2794 100644 --- a/tests/test_constructors.py +++ b/tests/test_constructors.py @@ -2,6 +2,8 @@ """Test CLR class constructor support.""" +import pytest + import System @@ -34,6 +36,11 @@ def test_struct_constructor(): assert ob.value == guid +def test_datetime(): + inst = System.DateTime(2021, 12, 29) + assert inst.Year == 2021 + + def test_subclass_constructor(): """Test subclass constructor args""" from Python.Test import SubclassConstructorTest @@ -48,8 +55,17 @@ class Sub(System.Exception): def test_multiple_constructor(): from Python.Test import MultipleConstructorsTest - import System # Test parameterless ob = MultipleConstructorsTest() assert ob.value == "" + + +def test_default_constructor_fallback(): + from Python.Test import DefaultConstructorMatching + + ob = DefaultConstructorMatching(2) + assert ob.a == 2 + + with pytest.raises(TypeError): + ob = DefaultConstructorMatching("2") diff --git a/tests/test_generic.py b/tests/test_generic.py index 9e1f1226b..e03ac57af 100644 --- a/tests/test_generic.py +++ b/tests/test_generic.py @@ -177,10 +177,22 @@ def test_generic_reference_type(): def test_generic_value_type(): """Test usage of generic value type definitions.""" + from System import Int32 + from Python.Test import GenericStructConstructorTest + + ob = GenericStructConstructorTest[Int32](42) + assert ob.Value == 42 + + +def test_nullable(): + """Test usage of Nullable[T] (special runtime handling).""" inst = System.Nullable[System.Int32](10) assert inst.HasValue assert inst.Value == 10 + with pytest.raises(TypeError): + inst = System.Nullable[System.Int32]() + def test_generic_interface(): # TODO NotImplemented