Skip to content
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

fix: genericmethodinvoker now correctly handles generic methods with overloads #6844

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Orleans.CodeGenerator.Compatibility;
using Orleans.CodeGenerator.Model;
using Orleans.CodeGenerator.Utilities;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
Expand Down Expand Up @@ -150,6 +149,9 @@ private MemberDeclarationSyntax[] GenerateInvokeMethods(GrainInterfaceDescriptio
allParameters.Add(TypeOfExpression(typeParameter.ToTypeSyntax()));
}

// PR: 6844 (fix generic overload selector in GenericMethodInvoker)
allParameters.AddRange(parameters.Select(p => TypeOfExpression(p.Symbol.Type.ToTypeSyntax())));

allParameters.AddRange(parameters.Select(p => GetParameterForInvocation(p.Symbol, p.Name)));

args =
Expand Down
1 change: 1 addition & 0 deletions src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<TargetFrameworks>netcoreapp3.1</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Common" Version="3.4.0" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsLoggingVersion)" />
</ItemGroup>
Expand Down
159 changes: 118 additions & 41 deletions src/Orleans.Core/CodeGeneration/GenericMethodInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;

using Orleans.Internal;
using Orleans.Runtime;
using Orleans.Serialization;
Expand All @@ -18,14 +19,18 @@ namespace Orleans.CodeGeneration
/// </remarks>
public class GenericMethodInvoker : IEqualityComparer<object[]>
{
private static readonly ConcurrentDictionary<Type, MethodInfo> BoxMethods = new ConcurrentDictionary<Type, MethodInfo>();
private static readonly ConcurrentDictionary<Type, MethodInfo> BoxMethods =
new ConcurrentDictionary<Type, MethodInfo>();

private static readonly Func<Type, MethodInfo> CreateBoxMethod = GetTaskConversionMethod;

private static readonly MethodInfo GenericMethodInvokerDelegateMethodInfo =
TypeUtils.Method((GenericMethodInvokerDelegate del) => del.Invoke(null, null));

private static readonly ILFieldBuilder FieldBuilder = new ILFieldBuilder();

private readonly MethodInfo genericMethodInfo;
private readonly Type grainInterfaceType;
private readonly string methodName;
private readonly int typeParameterCount;
private readonly ConcurrentDictionary<object[], GenericMethodInvokerDelegate> invokers;
private readonly Func<object[], GenericMethodInvokerDelegate> createInvoker;
Expand All @@ -47,34 +52,61 @@ public class GenericMethodInvoker : IEqualityComparer<object[]>
public GenericMethodInvoker(Type grainInterfaceType, string methodName, int typeParameterCount)
{
this.grainInterfaceType = grainInterfaceType;
this.methodName = methodName;
this.typeParameterCount = typeParameterCount;
this.invokers = new ConcurrentDictionary<object[], GenericMethodInvokerDelegate>(this);
this.genericMethodInfo = GetMethod(grainInterfaceType, methodName, typeParameterCount);
this.createInvoker = this.CreateInvoker;
}

/// <summary>
/// Invoke the defined method on the provided <paramref name="grain"/> instance with the given <paramref name="arguments"/>.
/// </summary>
/// <param name="grain">The grain.</param>
/// <param name="arguments">The arguments to the method with the type parameters first, followed by the method parameters.</param>
/// <param name="arguments">The arguments to the method with the type parameters first, followed by the method parameters types, and finally the parameter values..</param>
/// <returns>The invocation result.</returns>
public Task<object> Invoke(IAddressable grain, object[] arguments)
{
var invoker = this.invokers.GetOrAdd(arguments, this.createInvoker);
return invoker(grain, arguments);
var argc = (arguments.Length - typeParameterCount) / 2;

// As this is on a hot path, avoid allocating (LINQ) as much as possible
var argv = arguments.AsSpan();

// generic parameter type(s) + argument type(s) -- this is our invokers' cache-key
var types = argv.Slice(0, typeParameterCount + argc);

// argument values to be passed
var argValues = argv.Slice(typeParameterCount + argc, argc);

var invoker = this.invokers.GetOrAdd(types.ToArray(), this.createInvoker);

return invoker(grain, argValues.ToArray());
}

/// <summary>
/// Creates an invoker delegate for the type arguments specified in <paramref name="arguments"/>.
/// </summary>
/// <param name="arguments">The arguments.</param>
/// <param name="arguments">The method arguments, including one or more type parameter(s) at the head of the array..</param>
/// <returns>A new invoker delegate.</returns>
private GenericMethodInvokerDelegate CreateInvoker(object[] arguments)
{
// First, create the concrete method which will be called.
// obtain the generic type parameter(s)
var typeParameters = arguments.Take(this.typeParameterCount).Cast<Type>().ToArray();
var concreteMethod = this.genericMethodInfo.MakeGenericMethod(typeParameters);

// obtain the method argument type(s)
var parameterTypes = arguments
.Skip(typeParameterCount)
.Cast<Type>()
.ToArray();

// get open generic method for this arity/parameter combination
var openGenericMethodInfo = GetMethod(
grainInterfaceType,
methodName,
typeParameters,
parameterTypes);

// close the generic type
var concreteMethod = openGenericMethodInfo.MakeGenericMethod(typeParameters);

// Next, create a delegate which will call the method on the grain, pushing each of the arguments,
var il = new ILDelegateBuilder<GenericMethodInvokerDelegate>(
Expand All @@ -93,8 +125,9 @@ private GenericMethodInvokerDelegate CreateInvoker(object[] arguments)
{
il.LoadArgument(1); // Load the argument array.

// Skip the type parameters and load the particular argument.
il.LoadConstant(i + this.typeParameterCount);
// load the particular argument.
il.LoadConstant(i);

il.LoadReferenceElement();

// Cast the argument from 'object' to the type expected by the concrete method.
Expand Down Expand Up @@ -135,7 +168,8 @@ private static MethodInfo GetTaskConversionMethod(Type taskType)
var methods = typeof(OrleansTaskExtentions).GetMethods(BindingFlags.Static | BindingFlags.Public);
foreach (var method in methods)
{
if (method.Name != nameof(OrleansTaskExtentions.ToUntypedTask) || !method.ContainsGenericParameters) continue;
if (method.Name != nameof(OrleansTaskExtentions.ToUntypedTask) ||
!method.ContainsGenericParameters) continue;
return method.MakeGenericMethod(innerType);
}

Expand All @@ -154,65 +188,108 @@ bool IEqualityComparer<object[]>.Equals(object[] x, object[] y)
if (ReferenceEquals(x, null)) return false;
if (ReferenceEquals(null, y)) return false;

// Since this equality compararer only compares type parameters, ignore any elements after
// the defined type parameter count.
if (x.Length < this.typeParameterCount || y.Length < this.typeParameterCount) return false;

// Compare each type parameter.
for (var i = 0; i < this.typeParameterCount; i++)
{
if (x[i] as Type != y[i] as Type) return false;
}

return true;
return x.SequenceEqual(y);
}

/// <summary>
/// Returns a hash code for the type parameters in the provided argument list.
/// Returns a hash code for the provided argument list.
/// </summary>
/// <param name="obj">The argument list.</param>
/// <returns>A hash code.</returns>
int IEqualityComparer<object[]>.GetHashCode(object[] obj)
{
if (obj == null || obj.Length == 0) return 0;
if (obj.Length == 0) return 0;

unchecked
{
// Only consider the type parameters.
var result = 0;
for (var i = 0; i < this.typeParameterCount && i < obj.Length; i++)
foreach (var type in obj)
{
var type = obj[i] as Type;
if (type == null) break;

result = (result * 367) ^ type.GetHashCode();
}

return result;
}
}

/// <summary>
/// Returns the <see cref="MethodInfo"/> for the method on <paramref name="declaringType"/> with the provided name
/// and number of generic type parameters.
/// </summary>
/// <param name="declaringType">The type which the method is declared on.</param>
/// <param name="methodName">The method name.</param>
/// <param name="typeParameterCount">The number of generic type parameters.</param>
/// <param name="typeParameters">The generic type parameters to use.</param>
/// <param name="parameterTypes"></param>
/// <returns>The identified method.</returns>
private static MethodInfo GetMethod(Type declaringType, string methodName, int typeParameterCount)
private static MethodInfo GetMethod(
Type declaringType,
string methodName,
Type[] typeParameters,
Type[] parameterTypes
)
{
MethodInfo methodInfo = null;

var typeParameterCount = typeParameters.Length;

bool skipMethod = false;

var methods = declaringType.GetMethods(BindingFlags.Instance | BindingFlags.Public);
foreach (var method in methods)
foreach (var openMethod in methods)
{
if (!openMethod.IsGenericMethodDefinition) continue;

// same name?
if (!string.Equals(openMethod.Name, methodName, StringComparison.Ordinal)) continue;

// same type parameter count?
if (openMethod.GetGenericArguments().Length != typeParameterCount) continue;

// close the definition
MethodInfo closedMethod = openMethod.MakeGenericMethod(typeParameters);

// obtain list of closed parameters (no generic placeholders any more)
var parameterInfos = closedMethod.GetParameters();

// same number of params?
if (parameterInfos.Length == parameterTypes.Length)
{
for (int i = 0; i < parameterInfos.Length; ++i)
{
// validate compatibility - assignable/covariant array etc.
if (!parameterInfos[i].ParameterType.IsAssignableFrom(parameterTypes[i]))
{
skipMethod = true;
break;
}
}

if (skipMethod)
{
skipMethod = false;
continue;
}

// found compatible overload; return generic definition, not closed method
methodInfo = openMethod;
break;
}
} // next method


if (methodInfo is null)
{
if (!method.IsGenericMethodDefinition) continue;
if (!string.Equals(method.Name, methodName, StringComparison.Ordinal)) continue;
if (method.GetGenericArguments().Length != typeParameterCount) continue;
var signature = string.Join(",",
parameterTypes.Select(t => t.Name));

var typeParams = string.Join(",", typeParameters.Select(t => t.Name));

return method;
throw new ArgumentException(
$"Could not find exact match for generic method {declaringType}.{methodName}" +
$"<{typeParams}>({signature}).");
}

throw new ArgumentException(
$"Could not find generic method {declaringType}.{methodName}<{new string(',', typeParameterCount - 1)}>(...).");
return methodInfo;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@
<TargetFrameworks>netcoreapp3.1</TargetFrameworks>
<IsPackable>false</IsPackable>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<OrleansBuildTimeCodeGen>true</OrleansBuildTimeCodeGen>
oising marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="5.10.3" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftTestSdkVersion)" />
<PackageReference Include="Moq" Version="4.14.7" />
<PackageReference Include="xunit" Version="$(xUnitVersion)" />
<PackageReference Include="xunit.runner.visualstudio" Version="$(xUnitVersion)" />
</ItemGroup>
Expand All @@ -18,7 +21,9 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\..\src\BootstrapBuild\Orleans.CodeGenerator.MSBuild.Bootstrap\Orleans.CodeGenerator.MSBuild.Bootstrap.csproj" />
<ProjectReference Include="..\..\..\src\Orleans.CodeGenerator\Orleans.CodeGenerator.csproj" />
<ProjectReference Include="..\..\..\src\Orleans.Core.Abstractions\Orleans.Core.Abstractions.csproj" />
<ProjectReference Include="..\..\..\src\Orleans.Core\Orleans.Core.csproj" />
</ItemGroup>

Expand Down
Loading