8000 1783 Implement Interface And Inherit Class by rmadsen-ks · Pull Request #2028 · pythonnet/pythonnet · GitHub
[go: up one dir, main page]

Skip to content

1783 Implement Interface And Inherit Class #2028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
1783 Implement Interface And Inherit Class
Added support for multiple inheritance when inheriting from one base class and/or multiple interfaces.

Added a unit test verifying that it works with a simple class and interface. This unit test would previously have failed since multiple types are in the class super class list.
  • Loading branch information
rmadsen-ks committed Feb 2, 2023
commit 46de53e22c90fa461e6b14705d62372a61d53b6d
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
- Fixed error occuring when inheriting a class containing a virtual generic method.
- Make a second call to `pythonnet.load` a no-op, as it was intended.

- Added support for multiple inheritance when inheriting from a class and/or multiple interfaces.

## [3.0.1](https://github.com/pythonnet/pythonnet/releases/tag/v3.0.1) - 2022-11-03

### Added
Expand Down
1 change: 1 addition & 0 deletions src/python_tests_runner/PythonTestRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static IEnumerable<string[]> PythonTestCases()
// Add the test that you want to debug here.
yield return new[] { "test_indexer", "test_boolean_indexer" };
yield return new[] { "test_delegate", "test_bool_delegate" };
yield return new[] { "test_subclass", "test_implement_interface_and_class" };
}

/// <summary>
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1836,6 +1836,12 @@ internal static void SetNoSiteFlag()
return *Delegates.Py_NoSiteFlag;
});
}

internal static uint PyTuple_GetSize(BorrowedReference tuple)
{
IntPtr r = Delegates.PyTuple_Size(tuple);
return (uint)r.ToInt32();
}
}

internal class BadPythonDllException : MissingMethodException
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/StateSerialization/MaybeType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal struct MaybeType : ISerializable
const string SerializationName = "n";
readonly string name;
readonly Type type;

public string DeletedMessage
{
get
Expand Down Expand Up @@ -61,4 +61,4 @@ public void GetObjectData(SerializationInfo serializationInfo, StreamingContext
serializationInfo.AddValue(SerializationName, name);
}
}
}
}
19 changes: 7 additions & 12 deletions src/runtime/TypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ static PyTuple GetBaseTypeTuple(Type clrType)
return new PyTuple(bases);
}

internal static NewReference CreateSubType(BorrowedReference py_name, BorrowedReference py_base_type, BorrowedReference dictRef)
internal static NewReference CreateSubType(BorrowedReference py_name, IList<ClassBase> py_base_type, IList<Type> interfaces, BorrowedReference dictRef)
{
// Utility to create a subtype of a managed type with the ability for the
// a python subtype able to override the managed implementation
Expand Down Expand Up @@ -415,17 +415,12 @@ internal static NewReference CreateSubType(BorrowedReference py_name, BorrowedRe
}

// create the new managed type subclassing the base managed type
if (ManagedType.GetManagedObject(py_base_type) is ClassBase baseClass)
{
return ReflectedClrType.CreateSubclass(baseClass, name,
ns: (string?)namespaceStr,
assembly: (string?)assembly,
dict: dictRef);
}
else
{
return Exceptions.RaiseTypeError("invalid base class, expected CLR class type");
}
var baseClass = py_base_type.FirstOrDefault();

return ReflectedClrType.CreateSubclass(baseClass, interfaces, name,
ns: (string?)namespaceStr,
assembly: (string?)assembly,
dict: dictRef);
}

internal static IntPtr WriteMethodDef(IntPtr mdef, IntPtr name, IntPtr func, PyMethodFlags flags, IntPtr doc)
Expand Down
10 changes: 7 additions & 3 deletions src/runtime/Types/ClassDerived.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ internal static NewReference ToPython(IPythonDerivedType obj)
/// </summary>
internal static Type CreateDerivedType(string name,
Type baseType,
IList<Type> typeInterfaces,
BorrowedReference py_dict,
string? namespaceStr,
string? assemblyName,
Expand All @@ -163,7 +164,9 @@ internal static Type CreateDerivedType(string name,
ModuleBuilder moduleBuilder = GetModuleBuilder(assemblyName, moduleName);

Type baseClass = baseType;
var interfaces = new List<Type> { typeof(IPythonDerivedType) };
var interfaces = new HashSet<Type> { typeof(IPythonDerivedType) };
foreach(var interfaceType in typeInterfaces)
interfaces.Add(interfaceType);

// if the base type is an interface then use System.Object as the base class
// and add the base type to the list of interfaces this new class will implement.
Expand Down Expand Up @@ -214,8 +217,9 @@ internal static Type CreateDerivedType(string name,
}
}

// override any virtual methods not already overridden by the properties above
MethodInfo[] methods = baseType.GetMethods();
// override any virtual not already overridden by the properties above
// also override any interface method.
var methods = baseType.GetMethods().Concat(interfaces.SelectMany(x => x.GetMethods()));
var virtualMethods = new HashSet<string>();
foreach (MethodInfo method in methods)
{
Expand Down
64 changes: 38 additions & 26 deletions src/runtime/Types/MetaType.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;

Expand Down Expand Up @@ -79,49 +82,56 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
BorrowedReference bases = Runtime.PyTuple_GetItem(args, 1);
BorrowedReference dict = Runtime.PyTuple_GetItem(args, 2);

// We do not support multiple inheritance, so the bases argument
// should be a 1-item tuple containing the type we are subtyping.
// That type must itself have a managed implementation. We check
// that by making sure its metatype is the CLR metatype.
// Extract interface types and base class types.
var interfaces = new List<Type>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, repeating the same base interface multiple times should be an error.

And it might be, add a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test added.

var baseType = new List<ClassBase>();

if (Runtime.PyTuple_Size(bases) != 1)
var cnt = Runtime.PyTuple_GetSize(bases);

for (uint i = 0; i < cnt; i++)
{
return Exceptions.RaiseTypeError("cannot use multiple inheritance with managed classes");
var base_type2 = Runtime.PyTuple_GetItem(bases, (int)i);
var cb2 = (ClassBase) GetManagedObject(base_type2);
if (cb2 != null)
{
if (cb2.type.Valid && cb2.type.Value.IsInterface)
interfaces.Add(cb2.type.Value);
else baseType.Add(cb2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, it is better to enforce the order to be BaseClass, IInterface1, IInterface2, etc.

I believe the current code has a bug down below because it uses base_type which is not the BaseClass if the order is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only added this functionality for types which goes through TypeManager.CreateSubType, so it should only affect that branch.

for code flows not invoking CreateSubType, the behavior willl be exactly the same as before.

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it is not a .NET class or interface? You can't simply ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added exceptions here in the latest version.

}

BorrowedReference base_type = Runtime.PyTuple_GetItem(bases, 0);
BorrowedReference mt = Runtime.PyObject_TYPE(base_type);

if (!(mt == PyCLRMetaType || mt == Runtime.PyTypeType))
// if the base type count is 0, there might still be interfaces to implement.
if (baseType.Count == 0)
{
return Exceptions.RaiseTypeError("invalid metatype");
baseType.Add(new ClassBase(typeof(object)));
}

// Ensure that the reflected type is appropriate for subclassing,
// disallowing subclassing of delegates, enums and array types.
// Multiple inheritance is not supported, unless the other types are interfaces
if (baseType.Count > 1)
{
return Exceptions.RaiseTypeError("cannot use multiple inheritance with managed classes");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you collected multiple base types, perhaps it would be useful to list them in the error message in case user did not notice which ones are classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll include this.

}

if (GetManagedObject(base_type) is ClassBase cb)
var cb = baseType[0];
try
{
try
{
if (!cb.CanSubclass())
{
return Exceptions.RaiseTypeError("delegates, enums and array types cannot be subclassed");
}
}
catch (SerializationException)
if (!cb.CanSubclass())
{
return Exceptions.RaiseTypeError($"Underlying C# Base class {cb.type} has been deleted");
return Exceptions.RaiseTypeError("delegates, enums and array types cannot be subclassed");
}
}
catch (SerializationException)
{
return Exceptions.RaiseTypeError($"Underlying C# Base class {cb.type} has been deleted");
}

BorrowedReference slots = Runtime.PyDict_GetItem(dict, PyIdentifier.__slots__);
if (slots != null)
{
return Exceptions.RaiseTypeError("subclasses of managed classes do not support __slots__");
}

// If __assembly__ or __namespace__ are in the class dictionary then create
// If the base class has a parameterless constructor, or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the base class has a parameterless constructor

??? What is this for? If that's right, it seems breaking to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget, but that does not actually seem to be included with these changes. It is in my fork though.

// if __assembly__ or __namespace__ are in the class dictionary then create
// a managed sub type.
// This creates a new managed type that can be used from .net to call back
// into python.
Expand All @@ -130,10 +140,12 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
using var clsDict = new PyDict(dict);
if (clsDict.HasKey("__assembly__") || clsDict.HasKey("__namespace__"))
{
return TypeManager.CreateSubType(name, base_type, clsDict);
return TypeManager.CreateSubType(name, baseType, interfaces, clsDict);
}
}

var base_type = Runtime.PyTuple_GetItem(bases, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bases here can be empty since you removed check above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can bases be length 0? This should only be called when a C# type is subclassed, right? In this case, it should at least be length 1.


// otherwise just create a basic type without reflecting back into the managed side.
IntPtr func = Util.ReadIntPtr(Runtime.PyTypeType, TypeOffset.tp_new);
NewReference type = NativeCall.Call_3(func, tp, args, kw);
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/Types/ReflectedClrType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@ internal void Restore(ClassBase cb)
TypeManager.InitializeClass(this, cb, cb.type.Value);
}

internal static NewReference CreateSubclass(ClassBase baseClass,
internal static NewReference CreateSubclass(ClassBase baseClass, IList<Type> interfaces,
string name, string? assembly, string? ns,
BorrowedReference dict)
{
try
{
Type subType = ClassDerivedObject.CreateDerivedType(name,
baseClass.type.Value,
interfaces,
dict,
ns,
assembly);
Expand Down
12 changes: 12 additions & 0 deletions src/testing/classtest.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections;

namespace Python.Test
Expand Down Expand Up @@ -59,4 +60,15 @@ public ClassCtorTest2(string v)
internal class InternalClass
{
}

public class SimpleClass
{
public static void TestObject(object obj)
{
if ((!(obj is ISayHello1 && obj is SimpleClass)))
{
throw new Exception("Expected ISayHello and SimpleClass instance");
}
}
}
}
9 changes: 7 additions & 2 deletions tests/test_subclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import System
import pytest
from Python.Test import (IInterfaceTest, SubClassTest, EventArgsTest,
FunctionsTest, IGenericInterface, GenericVirtualMethodTest)
FunctionsTest, IGenericInterface, GenericVirtualMethodTest, SimpleClass, ISayHello1)
from System.Collections.Generic import List


Expand Down Expand Up @@ -338,4 +338,9 @@ class OverloadingSubclass2(OverloadingSubclass):
obj = OverloadingSubclass2()
assert obj.VirtMethod[int](5) == 5


def test_implement_interface_and_class():
class DualSubClass0(ISayHello1, SimpleClass):
__namespace__ = "Test"
def SayHello(self):
return "hello"
obj = DualSubClass0()
0