#2240 Recursion error when doing reversed python operations on C# types #2327
Conversation
1ae96dd to
c256746
Compare
|
Changes made... I traced it down to the nb_add, nb_mul, etc slots only having the forward operators defined/linked for C# types being used in Python, this somehow caused the RecursionError when any reverse operators are called as they are probably just pointing to random pointers.
It seems there were only catered for forward operators when both are the different (with overloaded operators, i.e. OwnInt - Int64 and Int64 - OwnInt). Thus all the code to detect whether a call is a reverse operator tested based on calling and passed types. Thus when called with the same types it could never detect it was a reversed operation even if it were defined. This needed some changes to the MethodBinding and ModelBinding to ensure the reversed calls are linked to the right C# operators and called with the arguments swapped. |
d4e1fe9 to
4ea33fe
Compare
4ea33fe to
5277ed9
Compare
src/runtime/MethodBinder.cs
Outdated
| /// <returns>A Binding if successful. Otherwise null.</returns> | ||
| internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw) | ||
| internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false) |
There was a problem hiding this comment.
Do you need to pass this parameter at all? Don't you have the field?
There was a problem hiding this comment.
Cleaned it up a bit. I missed that... The one method is static though so we will need to still pass it there...
3409d88 to
38fa8f2
Compare
38fa8f2 to
ed2505c
Compare
What does this fix? Explain your changes.
Reverse operators were not implemented correctly for C# types which resulted in Recursion errors when embedding C# in Python. Changed the model binding process to add reverse operators for all applicable types. Added a flag to MethodBinder that signals the operation is a reversed operation and that the arguments to the operator needs to be reversed.
Added tests and an wrapper object for int without overloaded operator methods and a wrapper codec to test the reverse operations (and forward operator on the same type for safety).
...
Does this close any currently open issues?
This should resolve #2240.
...
Any other comments?
...
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG