Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Proposal: Convert delegate types with compatible signature automatically #149

Closed
iam3yal opened this issue Feb 19, 2017 · 16 comments
Closed

Comments

@iam3yal
Copy link
Contributor

iam3yal commented Feb 19, 2017

Original post: #3661

.NET has inherited a whole sea of delegate types from .NET 1.0. Thanks to Func and Action they are largely obsolete now. C# should allow implicit conversions between delegate types of compatible signature.

See https://lostechies.com/jimmybogard/2008/03/26/stop-creating-custom-delegate-types/

public delegate bool CustomMatchingFunction(string value);

public void Custom_delegates_are_bad()
{
    Predicate<string> match1 = value => value == "Blarg";
    Func<string, bool> match2 = value => value == "Blarg";
    CustomMatchingFunction match3 = value => value == "Blarg";

    match1 = match2;
    match1 = (Predicate<string>)match2;
    match1 = match3;
}

All of these assignments should just work. If there are objections to making this an implicit conversion an explicit one would work as well.

@jnm2
Copy link
Contributor

jnm2 commented Feb 19, 2017

The fields of the delegate should be identical so this should be cheap in most cases. Multicast would be less cheap.

@gafter
Copy link
Member

gafter commented Feb 20, 2017

Introducing conversions among types that can already exist would be a breaking change to the language, and would therefore be undertaken only if the benefits were overwhelming. I don't see that here.

@iam3yal
Copy link
Contributor Author

iam3yal commented Feb 20, 2017

@gafter Is it still a breaking change in the case where there's an explicit conversion? if so, can you please elaborate on that?

p.s. I wonder whether you refer to nominal vs structural typing in both of these cases of explicit and implicit conversion.

@gafter
Copy link
Member

gafter commented Feb 20, 2017

@eyalsk Why bother with an explicit conversion when you can already write

public void Custom_delegates_are_bad()
{
    Predicate<string> match1 = value => value == "Blarg";
    Func<string, bool> match2 = value => value == "Blarg";
    CustomMatchingFunction match3 = value => value == "Blarg";

    match1 = match2.Invoke;
    match1 = match3.Invoke;
}

@iam3yal
Copy link
Contributor Author

iam3yal commented Feb 20, 2017

@gafter You're right, thank you.

@iam3yal iam3yal closed this as completed Feb 20, 2017
@Pzixel
Copy link

Pzixel commented Apr 21, 2017

@gafter because in this case you cannot pass a delegate as method group if it doesn't exactly of type it is declared in calling method. For example, I really like Predicate<T>, because there is only one parameter and i'm fine, but a lot of people prefer Func<T, bool>. If I have a library that takes Predicate<T> and they have, for example, some filtering function which takes Func<T, bool> they should call my library like Filter(x=>myPassedFuncTBool(x)) even when signatures matches.

@jnm2
Copy link
Contributor

jnm2 commented Apr 21, 2017

Chaining delegate calls using .Invoke is quick and dirty, but if you want a higher-performance call at the price of a slower cast, here's what I've been using:

public static T Cast<T>(this Delegate @delegate) where T : class
{
    if (@delegate == null) return null;

    var multicastList = (@delegate as MulticastDelegate)?.GetInvocationList();
    if (multicastList != null)
    {
        switch (multicastList.Length)
        {
            case 0:
                return null;
            case 1:
                if (multicastList[0] != @delegate)
                    return multicastList[0].Cast<T>();
                break;
            default:
                var convertedItems = new Delegate[multicastList.Length];
                for (var i = 0; i < multicastList.Length; i++)
                    convertedItems[i] = (Delegate)(object)multicastList[i].Cast<T>();
                return (T)(object)Delegate.Combine(convertedItems);
        }
    }

    return (T)(object)Delegate.CreateDelegate(typeof(T), @delegate.Target, @delegate.Method, true);
}

Only thing that would be better is where T : delegate. Ahem...

@Pzixel
Copy link

Pzixel commented Apr 21, 2017

But we have a lot of reflection here, especially that you don't have compile-time checks, and still able to cast Action to Func<int, bool> and do others dirty things... No, we defenitly need compiler that can implicitely convert delegates with same signature.

@jnm2
Copy link
Contributor

jnm2 commented Apr 21, 2017

@Pzixel You do get compile-time checks when you chain delegate calls together via .Invoke. The compiler can't transparently cast one to the other though; it would still have to do all the work I posted in the code sample at runtime. It would be nice to have language support to do that work.

@Pzixel
Copy link

Pzixel commented Apr 21, 2017

@jnm2 that's exactly what I am talking about

@gafter
Copy link
Member

gafter commented Apr 24, 2017

@jnm2 @Pzixel You say language support is needed for the compiler to produce good code for d1 = d2.Invoke, but I don't see any language changes that would be needed.

@jnm2
Copy link
Contributor

jnm2 commented Apr 24, 2017

@gafter I was attempting to communicate the exact opposite.

  • You don't need language support to chain a string of delegate invocations together using .Invoke.
  • You do need language support if you want to cast one delegate to another so that there is only one invoke happening, and so that mutating the original delegate does not affect the new delegate. (As in Proposal: Convert delegate types with compatible signature automatically  #149 (comment). That's an extension method, not an explicit cast, nor is it very type-safe. No checking of signature compatibility at compile time.)

@gafter
Copy link
Member

gafter commented Apr 25, 2017

You do need language support if you want to cast one delegate to another so that there is only one invoke happening

I don't understand how the language comes into it. The compiler can generate whatever correct code is most efficient for the operation.

, and so that mutating the original delegate does not affect the new delegate.

Delegates are not mutable, so I'm not sure what this is referring to.

@jnm2
Copy link
Contributor

jnm2 commented Apr 25, 2017

@gafter The language comes into it if it allows the explicit conversion (D2)d1 which the compiler would implement via the given code. The language does not allow this today IIUC.

You're right, when it comes to immutability, reflection is negligible.

@alfredmyers
Copy link
Contributor

alfredmyers commented May 22, 2020

@eyalsk Why bother with an explicit conversion when you can already write

public void Custom_delegates_are_bad()
{
    Predicate<string> match1 = value => value == "Blarg";
    Func<string, bool> match2 = value => value == "Blarg";
    CustomMatchingFunction match3 = value => value == "Blarg";

    match1 = match2.Invoke;
    match1 = match3.Invoke;
}

Although this will do the trick, I wouldn't call it idiomatic. People are used to casting, but I'd guess not many people know of this Invoke "trick".
I wasn't able to find mention to delegate's Invoke in .NET API docs nor in C#'s docs on delegates.


Edited to add:

I found a mention to Invoke in the remarks for the Delegate class. Not what I would call discoverable...

@lostmsu
Copy link

lostmsu commented Jul 27, 2022

@gafter I was surprised to see this thread with no mention of event handlers. If two event handlers have matching (incl. variance) but different delegate types D1 and D2, you can't just do

event D1 D1Event {
  add { D2Event += value; } ...
}

because value (of type D1) is currently incompatible with OtherEvent. And because you need to preserve identity between add and remove, you would need to keep a list of wrappers for values assigned to D1Event and wrapped for D2Event in order to be able to unsubscribe later.

@jnm2 jnm2 converted this issue into discussion #6324 Jul 27, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

6 participants