Skip to content

Remove needsResolution hack #1531

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

Merged
merged 2 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru
- `PythonException.Restore` no longer clears `PythonException` instance.
- Replaced the old `__import__` hook hack with a PEP302-style Meta Path Loader
- BREAKING: Names of .NET types (e.g. `str(__class__)`) changed to better support generic types
- BREAKING: overload resolution will no longer prefer basic types. Instead, first matching overload will
be chosen.

### Fixed

Expand Down
13 changes: 7 additions & 6 deletions src/embed_tests/TestOperator.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using NUnit.Framework;

using Python.Runtime;
using Python.Runtime.Codecs;

using System;
using System.Linq;
using System.Reflection;

Expand Down Expand Up @@ -212,21 +214,19 @@ public OperableObject(int num)
return (a.Num >= b);
}

public static bool operator >=(OperableObject a, PyObject b)
public static bool operator >=(OperableObject a, (int, int) b)
{
using (Py.GIL())
{
// Assuming b is a tuple, take the first element.
int bNum = b[0].As<int>();
int bNum = b.Item1;
return a.Num >= bNum;
}
}
public static bool operator <=(OperableObject a, PyObject b)
public static bool operator <=(OperableObject a, (int, int) b)
{
using (Py.GIL())
{
// Assuming b is a tuple, take the first element.
int bNum = b[0].As<int>();
int bNum = b.Item1;
return a.Num <= bNum;
}
}
Expand Down Expand Up @@ -421,6 +421,7 @@ public void ForwardOperatorOverloads()
[Test]
public void TupleComparisonOperatorOverloads()
{
TupleCodec<ValueTuple>.Register();
string name = string.Format("{0}.{1}",
typeof(OperableObject).DeclaringType.Name,
typeof(OperableObject).Name);
Expand Down
25 changes: 5 additions & 20 deletions src/runtime/methodbinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth
pi = pi.Take(1).ToArray();
}
int outs;
var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList,
needsResolution: _methods.Length > 1, // If there's more than one possible match.
outs: out outs);
var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, outs: out outs);
if (margs == null)
{
var mismatchCause = PythonException.FetchCurrent();
Expand Down Expand Up @@ -612,7 +610,6 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
IntPtr args, int pyArgCount,
Dictionary<string, IntPtr> kwargDict,
ArrayList defaultArgList,
bool needsResolution,
out int outs)
{
outs = 0;
Expand Down Expand Up @@ -653,7 +650,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
}

bool isOut;
if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut))
if (!TryConvertArgument(op, parameter.ParameterType, out margs[paramIndex], out isOut))
{
return null;
}
Expand Down Expand Up @@ -681,16 +678,15 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
/// </summary>
/// <param name="op">Pointer to the Python argument object.</param>
/// <param name="parameterType">That parameter's managed type.</param>
/// <param name="needsResolution">If true, there are multiple overloading methods that need resolution.</param>
/// <param name="arg">Converted argument.</param>
/// <param name="isOut">Whether the CLR type is passed by reference.</param>
/// <returns>true on success</returns>
static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResolution,
static bool TryConvertArgument(IntPtr op, Type parameterType,
out object arg, out bool isOut)
{
arg = null;
isOut = false;
var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution);
var clrtype = TryComputeClrArgumentType(parameterType, op);
if (clrtype == null)
{
return false;
Expand All @@ -710,25 +706,14 @@ static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResoluti
/// </summary>
/// <param name="parameterType">The parameter's managed type.</param>
/// <param name="argument">Pointer to the Python argument object.</param>
/// <param name="needsResolution">If true, there are multiple overloading methods that need resolution.</param>
/// <returns>null if conversion is not possible</returns>
static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool needsResolution)
static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument)
{
// this logic below handles cases when multiple overloading methods
// are ambiguous, hence comparison between Python and CLR types
// is necessary
Type clrtype = null;
IntPtr pyoptype;
if (needsResolution)
{
// HACK: each overload should be weighted in some way instead
pyoptype = Runtime.PyObject_Type(argument);
if (pyoptype != IntPtr.Zero)
{
clrtype = Converter.GetTypeByAlias(pyoptype);
}
Runtime.XDecref(pyoptype);
}

if (clrtype != null)
{
Expand Down
13 changes: 13 additions & 0 deletions src/testing/conversiontest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,17 @@ public override string ToString()
return value;
}
}

public class MethodResolutionInt
{
public IEnumerable<byte> MethodA(ulong address, int size)
{
return new byte[10];
}

public int MethodA(string dummy, ulong address, int size)
{
return 0;
}
}
}
14 changes: 13 additions & 1 deletion tests/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest

import System
from Python.Test import ConversionTest, UnicodeString
from Python.Test import ConversionTest, MethodResolutionInt, UnicodeString
from Python.Runtime import PyObjectConversions
from Python.Runtime.Codecs import RawProxyEncoder

Expand Down Expand Up @@ -681,3 +681,15 @@ def CanEncode(self, clr_type):
l = ob.ListField
l.Add(42)
assert ob.ListField.Count == 1

def test_int_param_resolution_required():
"""Test resolution of `int` parameters when resolution is needed"""

mri = MethodResolutionInt()
data = list(mri.MethodA(0x1000, 10))
assert len(data) == 10
assert data[0] == 0

data = list(mri.MethodA(0x100000000, 10))
assert len(data) == 10
assert data[0] == 0