Runtime: Add generic overloads for MethodInfo.CreateDelegate

[YairHalberstadt]

Currently usages of Delegate.CreateDelegate tend to be along these lines:

(Func<object>)Delegate.CreateDelegate(typeof(FuncObject), methodInfo)

For complicated delegates, this can be quite annoying and error prone.

it would be relatively simple to add generic overloads and it would make the API much nicer to use.

API suggestion

public partial class MethodInfo
{
    // Current
    public virtual Delegate CreateDelegate(Type delegateType);
    public virtual Delegate CreateDelegate(Type delegateType, object target);

    // New
    public T CreateDelegate<T>() where T : Delegate => (T)CreateDelegate(typeof(T));
    public T CreateDelegate<T>(object? target) where T : Delegate => (T)CreateDelegate(typeof(T), target);
}

The generic methods would not be virtual, as they're just syntactic sugar around the existing virtual.

[DaZombieKiller]

+1, I often find myself writing extension methods on MethodInfo for this purpose.

[bartonjs]

@jkotas I know that sometimes you're opposed to generic methods on core types. I don't quite understand the expense associated with them, so I'll just ask: How do you feel about this proposal?

[jkotas]

sometimes you're opposed to generic methods on core types

I have these concerns for frequently used types, like Dictionary. This is not used that frequently.

We have number of proposals to add convenience generic wrappers for non-generic APIs, e.g. similar issue / comment: https://github.com/dotnet/corefx/issues/27453#issuecomment-525448416 . We should figure how we are going to decide whether the convenience generic wrapper is worth it.

There are hundreds of methods in the surface that are potential candidates (basically, anything that has System.Type, System.Array or System.Delegate argument or return value is a candidate). Add generic wrapper for all of them? It does not sound right.

There are 12 variants of CreateDelegate in the netcoreapp surface. Which ones deserve the convenience wrapper? Unclear to me.

[YairHalberstadt]

There are hundreds of methods in the surface that are potential candidates (basically, anything that has System.Type, System.Array or System.Delegate argument or return value is a candidate). Add generic wrapper for all of them? It does not sound right.

Just to understand, assuming someone takes on the implementation, what are the disadvantages? Design, code review and API review should be minimal, considering the APIs are just straightforward pass throughs.

[jkotas]

assuming someone takes on the implementation, what are the disadvantages?

Platform bloat.

[YairHalberstadt]

There are hundreds of methods in the surface that are potential candidates (basically, anything that has System.Type, System.Array or System.Delegate argument or return value is a candidate). Add generic wrapper for all of them? It does not sound right.

I think those that return take a complicated type as a parameter, and return the same complicated type as a return value should be prioritised, as they make it much likelier to introduce an error.

There are 12 variants of CreateDelegate in the netcoreapp surface. Which ones deserve the convenience wrapper? Unclear to me.

If this is accepted in principle, I would be happy to see which, if any, are most commonly used in CoreFX.

[jkotas]

There are 12 variants of CreateDelegate in the netcoreapp surface. Which ones deserve the convenience wrapper? Unclear to me.

The two overloads on MethodInfo are the ones most convenient to use (they are instance methods), so they make the most sense to wrap for even more convenience. Unfortunately, it creates overload ambiguity.

public class MethodInfo
{
    public T CreateDelegate<T>() where T: delegate => (T)CreateDelegate(typeof(T));

    // This one is problematic because of it creates overload ambiguity with the existing non-generic CreateDelegate
    public T CreateDelegate<T>(object? target) where T: delegate => (T)CreateDelegate(typeof(T), target);
}

[GrabYourPitchforks]

How can we make progress on this? The API proposal is straightforward enough and normally it'd be actionable by us. But it sounds like there's concern that the proposal as currently stated is incomplete?

[jkotas]

@GrabYourPitchforks The proposal says to add generic overload to every CreateDelegate variant:

• It is not possible to add it everywhere because of it creates ambiguity. We need to figure how to deal with this ambiguity.
• It probably does not make sense to add it to every existing CreateDelegate variant. There is a lot of redundancy in the existing CreateDelegate variants. My proposal above was to add it for MethodInfo.CreateDelegate only.

[bartonjs]

Can you expand on the ambiguity, Jan?

Given my very clever class of:

```C#
public class Potato
{
public object Cook(Type t) => t;
public object Cook(Type t, object o) => Tuple.Create(t, o);
public T Cook() => default;
public T Cook(object o) => default;
}

I don't have compile-time or runtime ambiguity with these calls (intentionally using var to maximize ambiguity):

```C#
Potato p = new Potato();
var one = p.Cook<Action>();
var two = p.Cook<Action>(one);
var three = p.Cook(typeof(Action));
var four = p.Cook(typeof(Action), three);
var five = p.Cook(null);

Since the T isn't used in a parameter it isn't a candidate for generic inference, so there's no inference-based ambiguity (and even if there was, the non-generic method wins).

But if I'm confused and you're already suggesting that we do

```C#
public partial class MethodInfo
{
// Current
public virtual Delegate CreateDelegate(Type delegateType);
public virtual Delegate CreateDelegate(Type delegateType, object target);

// New
public T CreateDelegate<T>() => (T)CreateDelegate(typeof(T));
public T CreateDelegate<T>(object? target) => (T)CreateDelegate(typeof(T), target);

}
```

Then I'm on board.

Unless it's ambiguous in F# or VB?

[jkotas]

@bartonjs You are right. I am not sure what I was thinking about. So two new generic methods on MethodInfo is the plan then?

[bartonjs]

Added generic constraints (where T : Delegate) and updated the top post.

[GrabYourPitchforks]

Any issues with adding generic instance methods to a non-generic type? I recall some discussion in the past about trying to avoid such things.

[jkotas]

Virtual generic methods are problematic. Non-virtual generic instance methods are fine.

[terrajobst]

Video

• We discussed dropping the generic constraint (because it makes the new API less useful to code that is generic and not constrained, because the constraint is recent).
• We'll keep it for now and will drop it if we see issues with the constraint.

```C#
public partial class MethodInfo
{
// Current
public virtual Delegate CreateDelegate(Type delegateType);
public virtual Delegate CreateDelegate(Type delegateType, object target);

// New
public T CreateDelegate<T>() where T : Delegate => (T)CreateDelegate(typeof(T));
public T CreateDelegate<T>(object? target) where T : Delegate => (T)CreateDelegate(typeof(T), target);

}
```