Proposal: Update Type.GetMethod() overloads to simplify finding generic methods via reflection

[HaloFour]

Problem

Currently it is quite burdensome to resolve a specific MethodInfo for a generic method. That requires looping through all of the methods of the type and manually determining candidates based on name, parameter arity, type compatibility, etc. Such home-grown solutions are generally fragile, making assumptions regarding the specific overloads available for a given generic method at that point in time.

The problem with Type.GetMethod()

System.Type exposes various apis for retrieving methods by signature. In the end, all of them are wrappers around this single method:

public MethodInfo GetMethod(string name, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers)

This api was adequate in a pre-generics world. Once generics appeared however, the api has two serious defects:

1. You cannot specify the generic arity.

According to ECMA 335, the method signature includes the method's generic arity (generic parameters introduced by the method itself - not to be confused with generic parameters on the type declaring the method.) It is fully possible to have two methods on a type that differ only by generic arity. Despite this, there is no way to constrain the search to a specific generic arity.

2. No easy way to specify a generic parameter reference in a signature.

To disambiguate between overloads, the api takes an expected signature in the form of a Type[] array. The problem is that Type objects in Reflection represent "resolved types" rather than "signature types." Nevertheless, in a pre-generics world, this was a reasonably adequate way to represent a signature type. However, generics added a new signature type known as the generic parameter reference (ET_VAR/ET_MVAR) which throws a wrench into this scheme. Information-wise, an ET_MVAR is a mere integer (the position index of the generic parameter being referenced.)

Unfortunately, the Type model in Reflection has no direct analog to this simple construct. Instead, the only option available today to pass in a fully resolved generic parameter definition Type, which includes back references to MethodInfo that introduced it, a name, the constraints, a metadata token -- the works.

For generic method parameters, this creates a chicken and egg problem for the Type.GetMethod() api. To unambiguously retrieve a generic method such as this:

void Foo<T>(T t) {}

you to have pass in the fully resolved Type object for "T". But the only way to obtain that is by calling GetGenericArguments() on the MethodInfo for Foo<T>.

In other words, you must already have the method you're looking for in hand before you can call the Type.GetMethod() to find it.

Proposal Part A: Introduce a genericParameterCount parameter.

On Type, introduce the following public overloads:

public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types)
public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[] modifiers)
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, Type[] types, ParameterModifier[] modifiers);
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers);

and the protected overload (which all of the above funnel to):

protected virtual MethodInfo GetMethodImpl(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers) => throw new NotSupported;

genericParameterCount can be any non-negative integer and constrains the search to methods with that generic arity. (In particular, if genericParameterCount is 0, the search is constrained to non-generic methods.) The candidates are filtered for generic arity before the binder (if supplied) is invoked.

Proposal Part B: Provide a way to create a Type that represents a bare generic parameter reference and constructed types involving them.

On System.Type, introduce a new static method:

public static Type MakeGenericMethodParameter(int position);

This method returns a special "Signature Type" object that can be passed into Type.GetMethod()'s Type[] array to represent an ET_MVAR.

Sample Usage

Take this horror show of a type:

class Horror
{
     public void Moo(int x, int[] y) {}
     public void Moo<T>(T x, T[] y) {}
     public void Moo<T>(int x, int[] y) {}
     public void Moo<T,U>(T x, U[] y) {}  // <-- We want this one instantiated to "Moo<int, int>()"
     public void Moo<T,U>(int x, int[] y) {}
}

Here's how to pluck this out safely and unambiguously using the new api:

Type horror = typeof(Horror);
Type theT = Type.MakeGenericMethodParameter(0);
Type theU = Type.MakeGenericMethodParameter(1);

MethodInfo moo = horror.GetMethod("Moo", genericParameterCount: 2, new Type[] { theT, theU.MakeArrayType() });
MethodInfo mooOfIntInt = moo.MakeGenericMethod(typeof(int), typeof(int));

Type members implemented by Signature Types.

Signature Type objects will be very restricted in functionality with most of its members throwing NotSupportedExceptions. It's only purpose is to be used as a search pattern to discriminate between overloaded methods.

The following are the apis that actually do something on Signature Types:

Type Compositors:

public Type MakeArrayType();
public Type MakeArrayType(int rank);
public Type MakeByRefType();
public Type MakePointerType();

These create the appropriate constructed Type around the generic parameter reference. Types created this way have the same restrictions and also belong in the "signature type" universe.

In addition, MakeGenericType() on regular types will be enhanced to accept Signature Types. If one or more argument to MakeGenericType() is a Signature Type, the returned type will also be a Signature Type. For performance (and code simplicity reasons), no constraint checking will be done on any of the generic type arguments. The Signature Type will never be used to construct an actual Type, after all, it is simply a pattern used to disambiguate overloads.

Type Flavor Identifiers

public bool IsTypeDefinition;
protected bool HasElementTypeImpl();
protected bool IsArrayImpl();
public bool IsSZArray;
public bool IsVariableBoundArray;
protected bool IsByRefImpl();
public bool IsByRefLike;
protected bool IsPointerImpl();
public bool IsGenericType;
public bool IsGenericTypeDefinition;
public bool IsConstructedGenericType;
public bool ContainsGenericParameters;
public bool IsSignatureType;
public MemberTypes MemberType;

(while we don't need all of these, they are all trivial to implement so we might as well keep this set "safe to call.")

Type Dissectors:

public Type GetElementType();
public int GetArrayRank();
public Type GetGenericTypeDefinition();
public Type[] GenericTypeArguments { get; }
public Type[] GetGenericArguments();
public int GenericParameterPosition { get; }

This is the minimal set needed to do comparisons between signature types and the actual method parameter type.

Identity:

public bool Equals(object o);
public bool Equals(Type o);
public int GetHashCode();
public Type UnderlyingSystemType => this; // Only because Equals(Type) has a dependency on this

We won't override System.Object's implementations. In other words, these methods will work but have no particular utility value. At the moment, signature types are simply short-lived argument objects to Type.GetMethod(). If they never play a bigger role than that, we'll have saved ourselves from writing unnecessary memoization code and detailed equality routines. If they do become something more persistent in the future, we reserve the right to override and implement these to compute actual semantic equality.

Diagnostics:

public string Name { get; }
public string Namespace { get; }
public string FullName { get; }
public string AssemblyQualifiedName => null;
public string ToString();

For simple debugging/logging purposes, the Name property will emit strings like "!!0" and "!!1" (the long established ILASM syntax for ET_MVARS.)

Namespace will return null. FullName and AssemblyQualifiedName will both return null (as all open types do.)

Anything not mentioned above will throw a NotSupportedException("This operation is not supported on signature types."). If we find it useful to support additional apis later, this will leave that door open.

Signature Types passed to other apis that take Types

Signature Types are not recognized as "runtime-implemented Types" by apis that test for this ("if (type is RuntimeType)). Thus most apis will either throw at that point or hit a NotSupportedException eventually when it calls a non-supported member on type.

There are several reasons for this separation:

• The Types normally created by the runtime represent fully resolved types with assembly and container member info, not to mention executable IL, loader contexts and runtime application state (static fields.) Signature Types are simply search patterns that include none of those things, and potentially things that resolved types don't (for example, custom modifiers.) From a purist perspective, having the same object represent both is an overlap of concerns. From a pragmantic perspective, creating an entire new parallel Type class for Signature Types is probably going too far, given where .NET is at this point. However, we can at least minimize the issue by keeping the subset that doesn't fit into the "resolved type" model separate and restricted to the very limited purpose of passing them into Type.GetMethod() routines.

• Implementing these as "third party" types means we can have a simple non-invasive implementation of them that's sharable between CoreCLR and CoreRT. In particular, we don't have to figure out how to represent them in the unmanaged area of CoreCLR (the latter would drastically reduce the chances of finding anyone to step up and implement this...)

Items not in scope but possible future extensions

To keep the proposal to a reasonable size, these items are not in scope - however, we want to keep them in mind while designing this so as not to lock them out in the future.

A SignatureType for generic parameters on types (ET_VAR)

This is an obvious completeness addition. The usability difficulties that motivated this proposal don't apply to generic parameters on types so we don't need this right away. If this usage catches on, though, it would be good to have a consistent language for both types and method generics. This would imply adding the ability to pass Signature Types to the Type.GetConstructor and Type.GetProperty family of apis and the DefaultBinder methods that support them.

For now, this is being left out of scope to keep the api proposal down to size.

Third party binder/reimplementation support

If we want third party binders to be able to recognize and act on these signature types, we'll need to add more surface area to enable that. For now, I'm keeping this out of the scope of the proposal until we've had some experience with this.

Custom Modifiers

Another obvious use of the Signature Type concept would to be a MakeModifiedType(Type modifier, bool required) method on Type. This would make it possible to disambiguate methods overloaded on custom modifiers. Since this is a completely different usage scenario, it is not in scope here.

Summary of Additions

Two new api on System.Type:

public static Type MakeGenericMethodParameter(int position);
public virtual bool IsSignatureType => false;

Returns a Signature Type representing a reference to a generic parameter on the method. ArgumentException if position < 0.

New api overloads on System.Type:

public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types)
public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[] modifiers)
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, Type[] types, ParameterModifier[] modifiers);
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers);
protected virtual MethodInfo GetMethodImpl(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers) => throw new NotSupported;

New Parameter: int genericParameterCount
Behavior: Causes GetMethod() to disregard methods whose generic arity does not match the specified value. If genericParameterCount is 0, disregard all generic methods. This filtering is done before the binder, if supplied, is called.

Exceptions:
ArgumentException if genericParameterCount < 0

New behavior for System.Type.MakeGenericType() on regular Type objects:

If one or more arguments to MakeGenericType() is a Signature Type, the method switches to the new behavior and constructs a Signature Type. No constraint checking is done (even on the types that aren't signature types.)

Exceptions:
ArgumentNullException if any of the parameters are null.
ArgumentException if the number of supplied arguments does not match the number of generic parameters on the "this" Type.
InvalidOperationException if this.IsGenericTypeDefinition != true

New behavior for System.Type.GetMethod(...) and System.Type.DefaultBinder:

The Type[] passed to indicate the target method signature can now include Signature Types. Overload discrimination will proceed as if each generic parameter reference embedded within the Signature Type were replaced by the corresponding generic parameter definition introduced by the method being considered.

Reponses to previously asked questions

1. Can we change our implementation of GetMethod(...) to make it easier for instantiating generic methods?

Doing this now by adding a genericParameterCount argument that completes the "missing piece" of the signature specification.

2. Do we need new APIs?

Yes, when the generic method references its own generic type variables (ET_MVAR) in the signature. It is not practical to expect the caller to pass in a fully resolved generic parameter type since that requires him to have the very MethodInfo that he's looking for. Adding one new api to System.Type to create a lightweight Type object that wraps an index and nothing more.

3. Do we want to expose this policy at all or should the selection process be controlled by the consumer?

While I think the current GetMethod() apis have too much policy and complexity in them (binders), this proposal recognizes that these apis are well known and the proposed changes are straightforward extensions. More importantly, the changes introduce no new policy or fuzziness. "Disregards all methods that don't match the specified generic arity and does so before calling the binder" is crisp and objective. Similarly, the handing of signature types is "binding and overloading resolutions occurs as if you'd passed in the actual generic parameter from the method being considered." So we're not introducing new semantics there - only a saner way of invoking the existing semantics.

4. How do you conceive this proposal working for the overloads that takes the binder?

For part A, binders will receive only those candidates that match the supplied genericParameterCount.

For part B, binders (other than the DefaultBinder) will receive a Type array with signature types in them that they won't know what to do with. Applications calling these methods with custom binders will have to stick to passing in regular old Types. I think the non-custom binder case is common and useful enough not to block them on this. If we think the custom binder case that that important, the proposal can be extended if necessary to expose the necessary surface area for custom binders to recognize and act on signature types. (It'll be the internal surface area we implement for the DefaultBinder's use.)

[ghost]

I don't think ParameterModifier is needed. This is only relevant when invoking COM methods, which would not happen with generic methods. Similar argument for CallingConvention.

[HaloFour]

If those overloads of GetMethod couldn't apply to generic methods then I have no issue with removing them. I wasn't sure how those arguments were used and since I was basing my implementation effectively on the disassembly of RuntimeType I wanted the logic to be as consistent as possible with GetMethod.

That would give us the following overloads:

public MethodInfo GetGenericMethod(string name, Type[] genericArgs);
public MethodInfo GetGenericMethod(string name, Type[] genericArgs, BindingFlags bindingAttrs);
public MethodInfo GetGenericMethod(string name, Type[] genericArgs, Type[] types);
public MethodInfo GetGenericMethod(string name, Type[] genericArgs, BindingFlags bindingAttrs, Binder binder, Type[] types);

Unless you think that Binder is also unnecessary.

[ghost]

I suspect the Binder isn't necessary and would only be attractive if we actually implemented these on System.Type alongside the other Get apis (the implementation is cheap in that case and consistency is desirable.)

On the other hand, we may want to implement these as external extension methods in which case, I'd question whether we'd want anything other than ExactBinding semantics. Is all that obscure policy that allows imperfect specification of parameter types and the api "choosing the best" really necessary? (Yes, this kind of a belated rant against this sort of stuff being in the existing System.Type api...)

So we should at least ask ourselves if we have any reason other than consistency with existing Type api to want binder support and inexact parameter type matching support.

[HaloFour]

I'm a big fan of consistency. Much of the plumbing necessary to calculate candidates is already contained within RuntimeType and DefaultBinder so there isn't a lot left to implement. I'm sure it could be a lot cleaner than my own even discounting my reflection shenanigans.

If they were implemented as extension methods would they still ship in the same assembly and reuse the same plumbing?

[terrajobst]

Does this require a new API or could we fix the implementation of GetMethod to bind to to a generic method? Isn't the new method essentially just a shorthand for GetMethod("foo", ...).MakeGenericMethod(....)? And if not, could we change GetMethod so it is simply a shorthand?

[HaloFour]

@terrajobst

No, the methods above disambiguate generic methods by arity, enforce generic constraints and confirm the parameter types to the generic type arguments. The current API does none of those things which is why the prescribed manner to resolve generic methods is to use GetMethods and to interpret every MethodInfo manually.

The current API can't handle either use case in the proposal. It either returns null or throws.

[terrajobst]

@atsushikan

We don't believe we've enough information to make a call. Three questions:

• Can we change our implementation of GetMethod(...) to make it easier for instantiating generic methods?
• Do we need new APIs? If so, is the proposed shape good enough?
• Do we want to expose this policy at all or should the selection process be controlled by the consumer?

Feel free to sync with @karelz on the process.

@HaloFour

How do you conceive this proposal working for the overloads that takes the binder? Historically, this is where the complexity lies with applying overload resolution policies. Would you mind submitting some sample code on how this would work?

[karelz]

@atsushikan @DnlHarvey in future, please do the deeper first round of API reviews on your side (as area owners), before you pass it for API review. Please make sure you are on board with the API proposal, you can defend it and all questions for area owners are answered on the issue.
I'll be happy to provide more details offline if you think it will help. Thanks!

[HaloFour]

What is the status of this proposal?

@terrajobst

Can we change our implementation of GetMethod(...) to make it easier for instantiating generic methods?

I don't think you could without adding more overloads to accept the generic type arguments. Otherwise I don't see how you could disambiguate between generic methods that happened to accept the same parameter types, or even how you'd negotiate those parameter types.

It'd certainly be possible to add methods that didn't happen to have the name GetGenericMethod as proposed above, but when I wrote these extension methods for myself I wanted an overload that could accept just an array of generic type arguments without an array of parameter types.

Do we need new APIs?

I honestly can't fathom the fact that this remains a question. It's a massive gap in reflection and the result is that anyone trying to do something like dynamically construct expression trees have to roll their own code to manually resolve generic methods. For example, StackOverflow is loaded with examples of people trying to sort their queries by a property name. I can't imagine that this problem has not come up multiple times at Microsoft.

If so, is the proposed shape good enough?

The shape proposed above solves the problem and works well in production. I based my API and implementation on the reflection API of the .NET 4.0 runtime. If the state of the BCL has changed and some of those arguments are considered no longer necessary then I have no issue with their removal.

Do we want to expose this policy at all or should the selection process be controlled by the consumer?

If you are referring to overload resolution I'd think that the behavior should match that of GetMethod.

How do you conceive this proposal working for the overloads that takes the binder? Historically, this is where the complexity lies with applying overload resolution policies. Would you mind submitting some sample code on how this would work?

I'm not sure I understand the question. How do you handle overload resolution without the binder? The overloads that don't accept a binder just use the default, no? Again, I based my API on the overloads and functionality of .NET 4.0. I take no issue with simplifying the surface as long as it handles the job. The implementation that I've been using for years is included in the proposal.

[ghost]

It's a massive gap in reflection

I'd consider it a gap if there was no correct way to implement the functionality using the current api. (For example, IsSZArray and and GetForwardedTypes() were added to fill such gaps.)

Here, I believe we're talking about convenience methods to solve a common problem. Am I correct? Is there a specific correctness problem we'd be solving here? You mention the fragility problem (in that algorithms are often broken by someone adding a new overload to the type) but the existing Get overloads have that same issue. I think you generally have to opt for BindingFlags.ExactBinding to be sure.

Note that there was a correctness (or more precisely, a portably correctness) problem regarding overloads like this:

    Foo<T>(T x){}
    Foo<T>(int x}{}

The HasSameMetadataDefinitionAs api (https://github.com/dotnet/corefx/issues/5884) was recently added to plug that particular problem. In light of that api, do you still see a correctness issue that needs to be resolved?

[HaloFour]

@atsushikan

I'd consider it a gap if there was no correct way to implement the functionality using the current api.

I claim that it's not possible to correctly implement the functionality using the current API. Overload resolution and best match detection are handled by internal code within RuntimeType and Binder. I had to break out reflection to directly invoke the necessary methods. I don't see exact binding short circuiting that requirement in the framework source so eliminating Binder from the equation does nothing to remedy this.

Even ignoring that, I also claim that if a sufficiently large amount of code is required in order to implement that functionality and that doing so incorrectly will lead to fragile solutions specifically because people don't know how to implement this right that having a way to implement this is far from sufficient.

Generics aren't a third-party hack to the CLR, the reflection code to interrogate them shouldn't be either.

[ghost]

best match detection are handled by internal code within RuntimeType and Binder .

@HaloFour, @terrajobst

My own take is that the proposal would be more attractive without the "best match" part of this (i.e. the part that requires complicated policies to match up when the passed types aren't exact matches.)

The fact is, we don't really want that kind of policy-laden code in the low-level Type class. We made the mistake of adding them in the original framework and are now required to keep the ones we have for NS2.0 for compatibility sake. But I'm not keen on doubling down on that mistake by adding new ones.

Exact-binding overload resolution, on the other hand, is a much simpler problem (similar to passing BindingFlags.ExactBinding and no binder to the existing apis.) It also ensures that the code you write today isn't broken tomorrow by someone adding new overloads to the type.

[HaloFour]

@atsushikan

I am largely fine with that, but as far as I can tell even in that case the code that does the work of interrogating the overloads is still internal to mscorlib (RuntimeType.FilterApplyMethodBase).

To me it's more of a minor point but I do think it could cause confusion if the non-generic and generic versions of these methods behaved differently by default. Given that the code that exposes the overload resolution policy is already exposed and used in the same manner is there much harm in remaining consistent?

[ghost]

The advantage of pruning the scope of the problem to exact overload resolution means we open up the possibility of having these exposed on something like System.Reflection.Extensions which is what I'd prefer to having them as direct instance methods on Type. Exact overload resolution is also a simple enough problem that we don't need to leverage the FilterApplyMethodBase code. It also means we could have a common implementation between CoreCLR and CoreRT.

[HaloFour]

@atsushikan

Understood, and I'm personally fine with this being implemented via extension methods as long as they simplify the problem. I completely agree that exact binding makes sense for reflecting methods in general.

So, given that I guess it boils down to the following potential overloads:

public static class TypeGenericMethodExtensions {
    public static MethodInfo GetGenericMethod(this Type type, string name, Type[] genericArgs);
    public static MethodInfo GetGenericMethod(this Type type, string name, Type[] genericArgs, Type[] types);
}

Or, if you really want to overload GetMethod, you could put the generic type arguments first:

public static class TypeGenericMethodExtensions {
    public static MethodInfo GetMethod(this Type type, Type[] genericArgs, string name);
    public static MethodInfo GetMethod(this Type type, Type[] genericArgs, string name, Type[] types);
}

[ghost]

• I think the right naming would GetDeclaredMethod() or GetDeclaredGenericMethod(). The "Declared" is what we used on the TypeInfo methods to signal that we're only considering methods declared on that type (not base types) and that we're not discriminating on visibility/calling convention. I.e. the semantic equivalent of BindingFlags.Public|NonPublic|Instance|Static|DeclaredOnly|ExactBinding.

• It might be better if the apis not mix two concerns: finding the generic method and instantiating the generic method with a specific set of type parameters. This could be accomplished by replacing the "genericArgs" parameter with an "int genericArity" parameter. That would make the api own the task of finding the generic method. There's already an existing api (MakeGenericMethod) to do the instantiation.

This would also make it clear that the genericArgs type values are not being used to filter the list (only the generic arity as implied by genericArgs.Length)

[HaloFour]

@atsushikan

In that case it would make more sense for the extension methods to resolve off of TypeInfo rather than Type.

It does make sense to use generic arity but how would you resolve overloads based on the parameters in that case? How would you disambiguate the following two?

public class C {
    public static void M<T1, T2>(T1 x, T2 y) { }
    public static void M<T1, T2>(T2 x, T1 y) { }
}

[ghost]

In that case it would make more sense for the extension methods to resolve off of TypeInfo rather than Type .

No, TypeInfo is a vestigal class. It used to have a meaningful semantic in the .NETCore 1.0 refactoring but since we backed away from that refactoring, it sticks around only for compatibility and we expect it to fade away from consciousness over time.

It does make sense to use generic arity but how would you resolve overloads based on the parameters >in that case? How would you disambiguate the following two?

public class C {
    public static void M<T1, T2>(T1 x, T2 y) { }
    public static void M<T1, T2>(T2 x, T1 y) { }
}

Realistically, how would you disambiguate them even with a "genericTypeArgs" array? Sure, the "obvious" answer is to pass the Type objects for T1 and T2 in the array in the right order, but in order to get those Type objects, you already had to have the MethodInfo you were looking for in hand. It's a chicken and egg problem that makes the api borderline unusable for this scenario (otherwise, we could just use the classic apis to solve this.)

To disambiguate this sort of knot, you need a proper signature representation that Reflection just doesn't have today. I.e. something that represents a generic type parameter as simply an index, rather than the type-method-index tuple that Reflection surfaces them as today.

[HaloFour]

@atsushikan

No, TypeInfo is a vestigal class.

But you propose keeping the vestigial convention? That doesn't make a lot of sense, particularly if you aren't also adding GetDeclaredMethod extensions to Type proper. I'd still have to vote for consistency. But you could call it Frank as long as it solved the problem.

Realistically, how would you disambiguate them even with a "genericTypeArgs" array?

Pass types, either concrete or open. Sure, it mixes concerns, but it gets the job done. My implementation above has no problem distinguishing between those two overloads.

Just spitballin', but if you want to keep to arity but support disambiguating the above example you could expose a helper type with an indexer that can return placeholder generic type arguments:

var method1 = typeof(C).GetDeclaredMethod("M", 2, new Type[] { Generic.Arguments[0], Generic.Arguments[1] });
var method2 = typeof(C).GetDeclaredMethod("M", 2, new Type[] { Generic.Arguments[1], Generic.Arguments[0] });

[ghost]

TypeInfo wasn't about adding GetDeclared methods, it was about separating the notion of type references (Type) and type definitions (TypeInfo). The "Declared" naming is orthogonal to that - we don't need to throw it out just because the Type/TypeInfo split wasn't accepted by the developer base.

[ghost]

expose a helper type with an indexer that can return placeholder generic type arguments

Yes, if we wanted to stick to the idea of representing the signature as an array of Types, that's what we'd have to do - introduce a new flavor of Type (two actually, one for type variables and one for method variables) that represents a bare generic parameter position without any reference to its declaring entity.

Just to set realistic expectations, though, that that's an api proposition that's significantly larger than just the overloads you're proposing now. Defining the factory methods for those new flavors of Type is the easy part. We'd also have to nail down the behaviors of all the existing type apis on them (most of them don't make sense so they'd just throw, likely, but we'd have to be clear on what the behaviors are because we'd have to implement them separately on two different frameworks (CoreCLR and CoreRT.) We'd also want to nail down how they interact with the generic parameter types that exist today (the ones that represent the actual type variable definition, not just the reference) - how the type flavor predicates work (do they return true for IsGenericParameter or do we invent something new like IsGenericParameterReference)? And it's likely to trigger a discussion as to the ripple effects of using the Type object to represent both defined types and signature elements. (Are we overloading concerns there?)

(Of course, for a private implementation in a library, you could probably just hack up a type or even pass in some unrelated generic parameter type with the expectation that your overloads will only rely on the parameter position and whether DeclaringMethod returns null. But I don't think that hack would pass muster for a CoreFX api.)

[HaloFour]

@atsushikan

I'm not entirely sure how to respond to that. The exact naming of the API is of little concern to me. I'm more interested in solving the problem.

But I disagree with the notion that the naming is orthogonal between the two. As a part of the Type/TypeInfo split you guys decided to go with a different naming convention than what was established and that's perfectly fine. But if you are deprecating the API containing the "Declared" naming convention I don't see why you'd implement new API side-by-side with the existing API using the deprecated naming convention rather than the existing naming convention.

What I would do, personally, is go with the GetDeclaredMethod naming convention and implement them for both generic and non-generic methods. The GetDeclaredMethod methods would always apply exact binding rules, which gives you a clean and clear break from the legacy behavior and exposed method selection policies:

public static class DeclaredMethodTypeExtensions {
    public static MethodInfo GetDeclaredMethod(this Type type, string name);
    public static MethodInfo GetDeclaredMethod(this Type type, string name, Type[] types);
    public static MethodInfo GetDeclaredMethod(this Type type, string name, int genericArity);
    public static MethodInfo GetDeclaredMethod(this Type type, string name, int genericArity, Type[] types);
}

But I'll leave those exact details up to you guys. I will say that it's a lot of fun working through the surface details and I hope I'm not being too much of a pain. 😁

[ghost]

We couldn't use the existing naming convention without a BindingFlags parameter because we already have overloads that do that - except that they default to a different BindingFlags value (Public|Instance|Static or something.)

What I would do, personally, is go with the GetDeclaredMethod naming convention and implement them for both generic and non-generic methods.

Yes, this actually sounds ideal.

[HaloFour]

@atsushikan

Yes, if we wanted to stick to the idea of representing the signature as an array of Types, that's what we'd have to do

What reasonable alternatives would there be? Some surrogate type that can represent both a proper System.Type and a generic type argument? Either way it sounds like an explosive expansion on the potential API surface, but it should only have to be done once.

[ghost]

The only other alternative I see is a SignatureType class that represents.. signatures. That would be a clean separation of concerns but I also agree it's a major api expansion that isn't likely to fly. It defeats the convenience for the common case.

The way I see it, we now have two proposals:

1. The relative modest proposal (GetDeclaredMethod-style apis that support filtering on the generic arity.) Main justification is that the current apis do not support a way to disambiguate between methods that have same name, same method signature but different generic arities.

2. The far-reaching proposal (support Types that represent generic variable references) so that proposal 1 can reach its full usefulness and convenience. Note that once we add this, we'll probably face pressure to retroactively add recognitions of these to the various existing GetMethod() apis.

Proposal 2 will take considerably longer to design and implement and a harder acceptance bar. Proposal 1 would probably take less, but there's a chance it might not get accepted without Proposal 2.

I'd like to let this bake now for about a couple of weeks and give people a chance to chime in. (a lot of us our heads down on some tight schedules right now hence the extra buffer time.)

[HaloFour]

@atsushikan

I don't think that proposal 1 is worth considering. Many of the methods on System.Linq.Queryable have overloads with the same generic arity but differing numbers of arguments, e.g. Queryable.OrderBy. In my opinion resolving the methods on Queryable are right at the top of the list of common use cases for the purposes of manually constructing expression trees.

[ghost]

Proposal 1 includes the overloads with the "Type[] types" for disambiguation on parameter types.

Or are you saying "Proposal 1 without Proposal 2 won't be any use to you"?

[HaloFour]

@atsushikan

Or are you saying "Proposal 1 without Proposal 2 won't be any use to you"?

Correct. I think that without some form of proposal 2 that this API would not be very helpful when working with System.Linq.Queryable. What's worse is that they're often overloaded with other generic container types, e.g.:

public static IQueryable<TResult> Select<TSource, TResult>(
	this IQueryable<TSource> source,
	Expression<Func<TSource, TResult>> selector
);

public static IQueryable<TResult> Select<TSource, TResult>(
	this IQueryable<TSource> source,
	Expression<Func<TSource, int, TResult>> selector
);

So if the signature must be represented by some kind of surrogate that surrogate will have to deal with such type descriptions. So would some kind of Type-based placeholder.

All of this is so much easier when dealing with concrete generic type arguments rather than arity.

[ghost]

I see... so the MakeGenericMethod step is not a post-step but a pre-step before applying the parameter type disambiguation - which compares against the substituted types, not the formal types.

A question: to simplify the implementation (so it doesn't have to re-implement constraint checking and so we can eliminate "prospective" generic instantiations which are .Net Native unfriendly), would it be acceptable to implement the parameter type checking by substituting the types manually (without checking constraints) and then call MakeGenericMethod only once an unambiguous match has been found? (At that point, if you've violated constraints, you'll get the exception from MakeGenericMethod.)

[HaloFour]

@atsushikan

That works well for me. I've only ever used this implementation on the full framework so I can't speak to how well it plays with .NET Native but eliminating those steps makes sense.

[ghost]

Came up with a strawman implementation:

https://github.com/AtsushiKan/test/blob/master/MakeGenericMethod.cs

I ended up naming it MakeGenericMethod() as that's what it really is - a variant of MethodInfo.MakeGenericMethod() with an alternate way of specifying the method to specialize. You specify the declaring type, the method name and the generic type arguments to specialize over. Optionally, you can pass an expected parameter type array to disambiguate overloads. The tricky part is that the parameter type array contains the expected post-specialization parameter types. This is for expediency as the declared parameter types will often contain open types (in particular, the method's own generic type formals) and those Type objects are inconvenient to obtain.

My take on it is:

1. This is a useful toolbox routine - something I could easily see using in our own corefx tests or sticking into the Common directory inside corefx.

2. It's easily and portably implementable.

3. I'm not convinced, though, that it's an api. This thing says "helper" to me, rather than enabling core functionality for a broad use case. Chances are, specific use cases are going to want to tweak the shape. For example, some might want to specify a BindingFlags argument rather than hard coding it, or for convenience, make the helper itself generic rather than passing the generic type arguments as a separate array. Other scenarios might prefer to have "no match found" result in an exception rather than a null return.

Leaving this open for some further baking.

[HaloFour]

@atsushikan

Came up with a strawman implementation.

That's a nice and simple implementation.

I'm not convinced, though, that it's an api. This thing says "helper" to me, rather than enabling core functionality for a broad use case.

What is the fate of "helpers" as opposed to APIs?

[ghost]

My feeling is that it won't pass api-review. See this comment, for example:

https://github.com/dotnet/corefx/issues/461#issuecomment-70434257

"In particular, we need to be careful that we don't turn core types (such as Array and String) into a grab bag of potentially useful APIs."

This is indeed potentially useful but it's also something that can be easily written using existing apis (even more compactly without the necessary ceremony of argument validation and stuff.)

@terrajobst , @stephentoub - any thoughts?

[HaloFour]

@atsushikan

I assume that your example is written specifically for .NET Core 2.0? It won't compile on either .NET 4.7 or .NET Core 1.1 due to a myriad of missing methods. I am trying to tweak it to work on .NET 4.7 but some of the properties you reference are internal.

This is indeed potentially useful but it's also something that can be easily written using existing apis

I find this incredibly frustrating. Doing this right is far from easy and apparently can't be done using the APIs as they exist today. Both of our examples are walls of text relying on internals to actually work. Even if .NET Core 2.0 and .NET 4.next include those members in an accessible manner it still requires a very good understanding of CLR type metadata to handle correctly. Being able to interrogate generic methods shouldn't be orders of magnitude more difficult than interrogating non-generic methods.

[ghost]

Yes, it's built on .NET Core 2.0. IsSzArray, IsVariableBoundArray and IsTypeDefinition are the new apis. (These apis have already been approved and added so they're not "internals.") IsTypeDefinition is not crucial to this (it can be handled as the default case), and IsSZArray is often emulated by this extension method:

IsSZArray(this Type type) => type.IsArray && type.Name.EndsWith("[]")

while IsVariableBoundArray is literally IsArray && !IsSZArray

I'll acknowledge that as someone who's been neck deep in Reflection code for years, my definition of "easy" may be biased.

[ghost]

Also, this version still can't resolve all ambiguities - consider this case:

   void Foo<T>(T x) {}
   void Foo<T>(int x) {}

when you're instantiating on "int". All overload rules are permissive enough that you're always going to have trouble if you do the signature comparion post-substitution. You lose information during that substitution that's needed in some cases.

If we're going to add new apis, we should do it right and make sure we nail the problems of the old one completely this time. I think that does mean we have to create a way to represent generic parameter references in Types and do the signature comparisons pre-substitution. I'll see if I can free up some time next week to play around with that concept. It may not be as bad as it sounds.

[ghost]

I've updated the top comment with my counter-proposal. Based on some prototyping, I think this is a proposal that can be easily implemented in a day or two, has an implementation that's 90% sharable between CoreCLR and CoreRT and is foundationally and conceptually sound. This is something I could take to api-review.

[HaloFour]

I think it sounds pretty good. Having the generic type parameters represented by special instances of System.Type should help to simplify resolution.

I am curious about the third-party type binder situation. I've only ever dealt with Type.DefaultBinder. Are you aware of instances where a custom binder has been used? I assume that any such binder would have to be updated to support these new signature types otherwise they'll likely thrown when trying to interrogate the type.

[ghost]

Are you aware of instances where a custom binder has been used?

No, I haven't seen any such instance. I did note that as a possible extension point - if it actually turned out to be important to combine this feature with custom binders, we could expose some additional surface area to let them recognize and deal with signature types. (Basically, we'd expose the helpers I have in mind that would let the default binder deal with them.) I wouldn't be surprised if it never comes up, though. I don't see much interest in custom binders out there.

[ghost]

Re-labeled as "api-ready-for-review". Now the waiting game starts...

[terrajobst]

Video

This

public static Type MakeGenericMethodParameterSignatureType(int position);

should be

public static Type MakeGenericMethodParameter(int position);

We should also rename genericArity to genericParameterCount:

public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types)
public MethodInfo GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[] modifiers)
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, Type[] types, ParameterModifier[] modifiers);
public MethodInfo GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers);
protected virtual MethodInfo GetMethodImpl(string name, int genericParameterCount, BindingFlags bindingAttr, Binder binder, CallingConventions callConvention, Type[] types, ParameterModifier[] modifiers) => throw new NotSupported;

We should also expose a query API that identifies the new positional generic method parameters. We propose to go with some more foundational concept that could equally apply to references to generic type parameters. In the spec above @atsushikan used the term signature type. So we settled on:

public virtual bool IsSignatureType => false;

[karelz]

FYI: The API review discussion was recorded if you're interested - see https://youtu.be/VppFZYG-9cA?t=1685 (1h - yeah, it's loooong discussion)

[ghost]

Starting implementation. Should be a week or three...

• Implement on Project N
• Wait a few days until the shared bits propagate to CoreCLR
• Implement the CoreCLR-specific parts
• Add tests in corefx and expose in contract.