@filmor FYI. Clearly I need to fix runtime issues, do some cleanup and move things around but this is my general approach.

Thanks for working on this! This could simplify compatibility with different Python versions a lot 👍

@filmor I don't think this alone will rid us of TypeOffset. tp_dict for instance does not appear to be covered in PyType_FromSpec and we use that offset in many places.

This is nevertheless a huge step forward. If we can minimise the offsets in active use, we can probably maintain the respective offset info with a small script. By the way, this PEP tracks more efforts that will help us in this regard:https://www.python.org/dev/peps/pep-0620/

@filmor@lostmsu I'm getting an error that I am puzzled by.

When I run this code

import clrimport Systemclass Sub(System.Exception): passx=Sub()

or even replace the last line by
x = Sub.__new__(Sub)

It crashes inside of CPython with this message:

The last place in pythonnet when it reaches this point is in the CLRObject constructor at this line
IntPtr dict = Marshal.ReadIntPtr(py, ObjectOffset.TypeDictOffset(tp));

I've been debugging this for several hours now and I wish I could say I gained some more insight but I really haven't. What I did find is that the System.Exception MetaType construction seems to not go through the code path which I've modified.

Perhaps a more minor issue is this one - I am not sure if it is related:

>>> import System__main__:1: DeprecationWarning: builtin type PropertyObject has no __module__ attribute__main__:1: DeprecationWarning: builtin type EventObject has no __module__ attribute__main__:1: DeprecationWarning: builtin type FieldObject has no __module__ attribute

This is nevertheless a huge step forward. If we can minimise the offsets in active use, we can probably maintain the respective offset info with a small script. By the way, this PEP tracks more efforts that will help us in this regard:https://www.python.org/dev/peps/pep-0620/

@filmor I'll watch that PEP. It looks like its for 3.10 which isn't that far away

@koubaa the assert actually looks like it comes from .NETDebug.Assert call. Are you sure this is actually from CPython?

@lostmsu yes I'm sure. I addedDebug.Assert to try to better understand what's happening. In fact, the dialog which comes from the latter looks different.

@koubaa have you tried this code (Sub class) onmaster without your change? Does it work? If it does not, can you contribute a test (mark it disabled/inconclusive)? Then I can try to work on it outside of this PR.

The overload ofCreateType you modified only works for metatypes defined inPython.Runtime (e.g. children ofManagedType). All other CLR types are handled via

It is probably related to myshenanigans with a similar problem. You might want to ask a similar question on StackOverflow about the relationship betweenPyType_FromSpec andtp_basicsize. The documentation is surprisingly scarce.

I just checked on Win64 Python's reflection ofSystem.Exception takes 80 bytes ==ExceptionOffset.Size(), which is as expected. However, theSub class takes 88 bytes, which is not. We need an explanation as to why Python adds extra bytes when deriving fromSystem.Exception. Depending on it we should either change theAssert condition, or change our code to ensure Python does not need to add anything when deriving fromSystem.Exception.

I just checked and the reflectedSystem.Exception type hastp_dictoffset > 0, so that's not it.

@lostmsu Actually I discovered this issue from an existing test case:test_constructors.py test_subclass_constructor()

I thought I could do one overload at a time so it would be easier to bisect issues, do you think that it is wrong to expect this to work?

I'll look intotp_basicsize. I have recently been poking around in CPython source so I'll try to see if I can figure out what the relationship is.

The larger type object gets created forSub here:

We need an explanation as to why Python adds extra bytes when deriving from System.Exception

/* PyException_HEAD defines the initial segment of every exception class. */
#define PyException_HEAD PyObject_HEAD PyObject *dict;
PyObject *args; PyObject *traceback;
PyObject *context; PyObject *cause;
char suppress_context;

https://github.com/python/cpython/blob/4e02981de0952f54bf87967f8e10d169d6946b40/Include/pyerrors.h#L10-L14

@amos402 my understanding is that the class hierarchy looks like this:Sub ->System.Exception ->builtins.BaseException ->builtins.object. The question is why the first-> adds 8 bytes, not the last one. The last one works as intended.

I just checked onmaster, and the same assert actually fails there, so this is not caused by the@koubaa 's change in this PR, and can be evaluated separately.

@lostmsu on master, its a pythonnet assertion (you can ignore the dialogs and the python interpreter is still up). With my branch, its a crash in native code. Perhaps@amos402 's fix will also fix things in my branch, I can try to cherry-pick the relevant changes interop.cs and see what happens

@lostmsu@amos402 I confirm that with Amos's interop changes I do not have any crash anymore. I submitted a PR to land amos' change separate from his soft shutdown branch and have that commit cherry-picked in this branch so that I can continue to work on this.

@lostmsu@amos402 I am looking at this again this weekend. I need time to understand the changes from soft-shutdown here...

@koubaa , if you are not actively working on this, I might pick it up over the holidays.

@lostmsu no I'm not actively working on it anymore. I resolved some of the merge conflicts locally from the soft shutdown PR but haven't had much time to figure out how to get this working.

Actually, I also decided against it for now, as I don't have any idea how to clear slots for types creates withFromSpec, if we removeTypeOffset.

Closing for now, as this has been temporarily abandoned