Skip to content

DLR-based overload resolution #1278

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

Closed

Conversation

lostmsu
Copy link
Member

@lostmsu lostmsu commented Nov 12, 2020

What does this implement/fix? Explain your changes.

This is an attempt to replace our custom (and in many cases borked) overload resolution with C# RuntimeBinder, that is used to implement C#'s dynamic keyword.

This should make our overload resolution nearly identical to C# overload resolution. It has full handling for: params, implicit conversions, type hierarchy, generics, overload prioritization, keyword arguments.

A bonus feature is that supporting newer C# features in method binding should be as easy as simply updating the Microsoft.CSharp NuGet.

Major consequences and further TODO

  1. During overload selection, there will be no more multiple attempts to marshal Python objects to C# (was: one per attempted overload). All parameters will be decoded exactly once, and then C# binder will be used to pick the correct overload.
  2. Because of 1, decoders should return object instances, that are compatible with expected .NET types. For example: decode of builtins.int should return an instance of PyInt, which then should implement static or dynamic (e.g. TryConvert) conversion to System.Int32 and/or System.Int64 in order to be used with void Foo(int arg).
  3. The mechanism to select a specific overload from Python would have to be reworked. We need a clear separation between explicitly specifying generic parameters vs explicitly specifying parameter types.

Does this close any currently open issues?

#1099 , specifically #1099 (comment)

Other comments

I am not sure how we could approach getting this done. Until the entirety of binding features are re-implemented with DLR, current tests will be failing. Perhaps a separate branch would be a good approach, where changes could be reviewed incrementally. @amos402 , @filmor , thoughts?

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@lostmsu
Copy link
Member Author

lostmsu commented Apr 8, 2021

Closing for now as it is still in development

@lostmsu lostmsu closed this Apr 8, 2021
@filmor
Copy link
Member

filmor commented Sep 27, 2021

@lostmsu Do you plan to pick this one up again?

@lostmsu
Copy link
Member Author

lostmsu commented Sep 27, 2021

@filmor not any time soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants