Basic operator overloads support - #907
Conversation
Sorry, something went wrong.
lostmsu
left a comment
There was a problem hiding this comment.
Completed initial pass with some comments.
Sorry, something went wrong.
| { | ||
| if (tp == null) | ||
| { | ||
| yield break; |
There was a problem hiding this comment.
I think this should throw instead, and the caller should explicitly handle null case.
Sorry, something went wrong.
| MethodInfo method; | ||
| try | ||
| { | ||
| method = t.MakeGenericMethod(tp); |
There was a problem hiding this comment.
You can yield return right here
Sorry, something went wrong.
| } | ||
| catch (ArgumentException) | ||
| { | ||
| method = null; |
There was a problem hiding this comment.
You can continue right here
Sorry, something went wrong.
| Type param = pi[n].ParameterType; | ||
|
|
||
| if (!param.IsGenericParameter && !IsNullableOf(sig, param) && | ||
| !param.IsAssignableFrom(sig)) |
There was a problem hiding this comment.
Can you explain what this method was supposed to be doing, and what your change enables it to do now? It is actually not clear what genericTp and sigTp represent from their names :(
Sorry, something went wrong.
|
|
||
| public static void FixupSlots(IntPtr pyType, Type clrType) | ||
| { | ||
| IntPtr tp_as_number = Marshal.ReadIntPtr(pyType, TypeOffset.tp_as_number); |
There was a problem hiding this comment.
This does not seem to be used anywhere
Sorry, something went wrong.
There was a problem hiding this comment.
😂opps, some methods are just used in my code, so it maybe not used here.
Sorry, something went wrong.
| { | ||
| throw new PythonException(); | ||
| } | ||
| OperatorMethod.FixupSlots(type, clrType); |
There was a problem hiding this comment.
Should not the fixup happen before PyType_Ready?
Sorry, something went wrong.
There was a problem hiding this comment.
It could be.
Sorry, something went wrong.
There was a problem hiding this comment.
@amos402 From Python doc:
Finalize a type object. This should be called on all type objects to finish their initialization. This function is responsible for adding inherited slots from a type’s base class. Return 0 on success, or return -1 and sets an exception on error.
Unfortunately, the description does not clarify if the behavior could change in the future :/ I'd try to move FixupSlots up, and see if it works that way.
Sorry, something went wrong.
| assert result[0] == 'one' | ||
| assert result[1] == 'two' | ||
| assert result[2] == 'three' | ||
| # Skip these temporally cause of the changes of array parameter calling |
There was a problem hiding this comment.
Is this change not ready?
.NET permits arrays to be passed in place of params object[] arguments. If it was allowed previously, but is not after your change, it removes a feature, which should not be done.
Sorry, something went wrong.
| return OpMethodMap.ContainsKey(methodName); | ||
| } | ||
|
|
||
| public static bool IsPyOperatorMethod(string pyMethodName) |
There was a problem hiding this comment.
This method seems to be unused.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks like this was used later for delegates support? joonhwan@b154475
Sorry, something went wrong.
|
Could be closed in favour of the updated version at #1324 |
Sorry, something went wrong.
What does this implement/fix? Explain your changes.
Support the operator for objects.
I found this code from my branch before, not only support the operator but also refactored the MethodBinder. @lostmsu I realized we just did the same thing. But a little different was I intended to support the method like foo(int x = 0, params object[] bar) can be call with foo(1,1,1), you can see the change from the pytest. I think we need to discuss how to merge them.
I develop it base on Python3, some test cases may fail on 2 now.
Does this close any currently open issues?
#906
Any other comments?
...
Checklist
Check all those that are applicable and complete.