Skip to content

Update methodbinder.cs #217

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

Update methodbinder.cs #217

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/runtime/methodbinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw,
if (clrtype != null)
{
bool typematch = false;
if (pi[n].ParameterType != clrtype)
if ((pi[n].ParameterType != clrtype) && !(pi[n].ParameterType.IsAssignableFrom(clrtype)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the != check redundant? It looks like checking !pi[n].ParameterType.IsAssignableFrom(clrtype) would be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a very good catch! But actually the logic is more complicated,
since sometimes multiple methods can match IsAssignableFrom criteria for
the overloaded arguments.

So in addition to type equality really need to loop through IsSubclassOf
and build a list sorted by proximity to the argument type.

On Thu, May 26, 2016 at 4:43 AM, Tony Roberts notifications@github.com
wrote:

In src/runtime/methodbinder.cs
#217 (comment):

@@ -344,7 +344,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw)
if (clrtype != null)
{
bool typematch = false;

  •                            if (pi[n].ParameterType != clrtype)

  •                            if ((pi[n].ParameterType != clrtype) && !(pi[n].ParameterType.IsAssignableFrom(clrtype)))

Isn't the != check redundant? It looks like checking
!pi[n].ParameterType.IsAssignableFrom(clrtype) would be sufficient.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/pythonnet/pythonnet/pull/217/files/5a8ae4f6b1a69ad96c17d894972556296b7fe513#r64718246

Copy link
Contributor

Choose a reason for hiding this comment

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

All I meant was that there can never be the case where pi[n].ParameterType == clrtype && !(pi[n].ParameterType.IsAssignableFrom(clrtype)), so this case is equivalent to checking that ParameterType is not assignable from clrtype.

Another way of saying this is that (ParameterType is not assignable from clrtype) implies (Parameter Type != clrtype). So, the expression as written is logically equivalent to simply (ParameterType is not assignable from clrtype).

{
IntPtr pytype = Converter.GetPythonTypeByAlias(pi[n].ParameterType);
pyoptype = Runtime.PyObject_Type(op);
Expand Down Expand Up @@ -603,4 +603,4 @@ internal Binding(MethodBase info, Object inst, Object[] args,
this.outs = outs;
}
}
}
}