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

Convert delegate types with compatible signature automatically #2766

Closed
GSPP opened this issue Jun 24, 2015 · 34 comments
Closed

Convert delegate types with compatible signature automatically #2766

GSPP opened this issue Jun 24, 2015 · 34 comments

Comments

@GSPP
Copy link

GSPP commented Jun 24, 2015

.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.

@HaloFour
Copy link
Contributor

This has come up on a number of threads here. The only real problem is that the CLR provides no signature equivalence for delegate types and the only way to "convert" from one delegate to another is to create a new delegate of the new type that points to the Invoke method of the first delegate. That results in an additional delegate invocation which is not a negligible amount of overhead.

I'm not arguing against the feature, I think it would be a great idea, but that's really the one potential sticking point.

@axel-habermaier
Copy link

@HaloFour: Note that the compiler could be using the following trick:

Predicate<int> p = i => i % 2 == 0;

// Func<int, bool> f = p; could be compiled as:
var f = (Func<int, bool>)Delegate.CreateDelegate(typeof(Func<int, bool>), p.Target, p.Method);

In a quick'n'dirty micro benchmark, the Delegate.CreateDelegate solution is about 30% faster when invoking the delegate; it is, however, two orders of magnitude slower to create the delegate. So, the relevant questions are: Is there a way to improve the performance of Delegate.CreateDelegate? What's the ratio of delegate creation vs. delegate invocation?

@HaloFour
Copy link
Contributor

Also remember that if the source delegate has multiple targets that the Target and Method properties only return the values for the last target in the invocation list. My gut feeling is that any incurred conversion cost is better upfront rather than on invocation, assuming that the overhead is roughly similar which it is not.

Short of CLR equivalence I wonder if a more efficient call could be added to the Delegate class that could perform a conversion through iterating the invocation list and creating new delegate instances.

@MrJul
Copy link

MrJul commented Jun 24, 2015

It turns out the current CLR can already invoke delegates of different types. Here is my test.

I compiled the following C# code that defines a delegate type named Do, having a signature compatible with Action<int>:

using System;

namespace ConsoleApplication12 {

    public delegate void Do(int s);

    public static class Program {

        private static readonly Do _fn = i => Console.WriteLine("Hello! " + i);

        public static void Main() {
            CallDo(_fn);
        }

        private static void CallDo(Do fn) {
            Console.WriteLine("In CallDo, fn is a {0}", fn.GetType());
            fn(1);
        }

        private static void CallAction(Action<int> fn) {
            Console.WriteLine("In CallAction, fn is a {0}", fn.GetType());
            fn(2);
        }

    }

}

I decompiled the resulting .exe to IL using ildasm, replaced the call to CallDo with a call to CallAction, and compiled that back with ilasm.

//IL_0006:  call       void ConsoleApplication12.Program::CallDo(class ConsoleApplication12.Do)
IL_0006:  call       void ConsoleApplication12.Program::CallAction(class [mscorlib]System.Action`1<int32>)

As expected, peverify complains:

ConsoleApplication12.Program::Main][offset 0x00000006][found ref 'ConsoleApplication12.Do'][expected ref 'System.Action`1[System.Int32]'] Unexpected type on the stack.

However, the resulting executable runs fine!

In CallAction, fn is a ConsoleApplication12.Do
Hello! 2

This was executed on the CLR installed along with .NET 4.6 RC. I don't know if this works on Mono or CoreCLR since it's not a documented behavior, and there are probably edge cases. What's great to know is that delegate compatibility may be easier than expected if only the C# compiler and the verifier have to be modified, not the CLR.

@mburbea
Copy link

mburbea commented Jun 24, 2015

Yes, I would 100% love delegate structural compatibility.
Currently, VB offers this functionality by essentially creating a new delegate of your type pointing to the invoke method of the other delegate
e.g.

Action a=()=>Console.WriteLine("woah");
ThreadStart b= a.Invoke

@HaloFour
Copy link
Contributor

@MrJul That is interesting. If the CLR already doesn't care, even by accident, and only the verifier really needs to be modified to compare the signatures I imagine that would reduce the effort needed to support this considerably.

@HaloFour
Copy link
Contributor

@MrJul Just tested with Mono (running on a Mac), it also works fine.

@GSPP
Copy link
Author

GSPP commented Jun 24, 2015

I doubt this will be done by simply reinterpreting the object reference. You could do:

Func<int, bool> a = i => true;
Predicate b = a;
cw(b.GetType()); //prints Func

Not sure if that is a good thing. This is possible for compatible arrays already. See http://stackoverflow.com/questions/11976969/in-c-net-why-is-sbyte-the-same-as-byte-except-that-its-not

(sbyte[])(object)(new byte[0])

Compiles and runs.

This implicit conversion probably should create a new delegate with some framework support for making the conversion efficient. Maybe a JIT intrinsic.

@orthoxerox
Copy link

@MrJul have you tried replacing Func<char, short> with Func<short, char>? I am afraid this will work as well. And maybe Action<IFoo> with Action<IBar> if the object you're passing implements both interfaces. The compiler will have to tread a fine line when trying to determine the structural equality of two delegates.

@leppie
Copy link

leppie commented Jun 25, 2015

@axel-habermaier : That one fails hard for a DynamicMethod.

There is already a good conversion solution. Just pass the instance to the constructor of the desired type. Eg: new Func<int, bool>(new Predicate<int>(x => true)) (also does not blow up with DynamicMethod).

@HaloFour
Copy link
Contributor

@leppie That's just syntax candy for calling Delegate.CreateDelegate with the target being the original delegate and the method being the Invoke method of that delegate. That's what results in the delegate invocation overhead being doubled, invoking the first delegate has to then invoke the second delegate.

@leppie
Copy link

leppie commented Jun 25, 2015

@HaloFour Yeah, I know, but Delegate.CreateDelegate can't be used with delegates created by DynamicMethod that way on the MS runtime. It actually has no problem on Mono. The only safe way to handle all delegates is the way I shown.

@Pzixel
Copy link

Pzixel commented Apr 6, 2016

I think user shouldn't define its own delegate types, but just create aliases. So every delegate is interpreted as alias for Func<T1,T2..Tn>. Or at least just inherit it in some way.
So when you write

string delegate Foo(string bar);

If just creates an alias for Func<string,string> and is substituted everywhere with it. It's only useful to define custom parameters (like bar here), but with "inheritance" it's easy to solve - each "derived" delegate just has different name and list of parameter names.

@axel-habermaier
Copy link

@Pzixel: Unfortunately, it's not that easy. See dotnet/roslyn#10303 for details.

@HaloFour
Copy link
Contributor

HaloFour commented Apr 6, 2016

@Pzixel Even if C# could do that the CLR could not. Those different delegates are distinct types and incompatible with one another, even if their signature is identical. The CLR doesn't allow for one delegate type to be used in place of another, you have to have the one delegate actually invoke the other delegate, which adds overhead. Also, the Func<...> family of helper delegates only gives you support for simple functions. You can't use them to call void methods or any methods with ref or out parameters.

@Pzixel
Copy link

Pzixel commented Apr 6, 2016

@axel-habermaier thanks, i will examine this link now

@HaloFour ref and out have to be traited as different signatures (maybe) or something. It's just a concept, while treating refs and outs is implementation detail. The idea is that user delegates must be convertible each to another with same signature. Question here is "what is signature?", but it's a bit offtopic. Of course it leads to CLR cnanges, but there is small amount of things may be done with language change alone.

As bonus you receive free delegate type inferring, for example you can write
var isPositive = x => x > 0;
instead of
Func<int, bool> isPositive = x => x > 0;

@gafter
Copy link
Member

gafter commented Apr 21, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to dotnet/roslyn#18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this. You can accomplish the same thing today by writing

    match1 = match2.Invoke;

@gafter gafter closed this as completed Apr 21, 2017
@gafter
Copy link
Member

gafter commented Apr 21, 2017

Also, as observed in #149, it would be an incompatible change.

@masonwheeler
Copy link

@gafter The observation over there is incorrect. It would not be an incompatible change.

Specifically, by making this change, there is no code that currently builds that would 1) fail to build or 2) run with different semantics. All that would change is that some code that currently fails to build would now build and run.

@Pzixel
Copy link

Pzixel commented Apr 21, 2017

@gafter I don't agree. I commented my position in linked proposal.

@jnm2
Copy link
Contributor

jnm2 commented Apr 21, 2017

@gafter
Copy link
Member

gafter commented Apr 24, 2017

@masonwheeler Adding a conversion between types that already exist is always an incompatible change. Here is a demonstration of that fact:

using System;

class Program
{
    static void Main(string[] args)
    {
        A a = null;
        M(a, 0, 0);
    }

    static void M(A a, int x = 0, long y = 0) { }
    static void M(B b, long x = 0, int y = 0) { }
}

class A
{
    //public static implicit operator B(A self) => null;
}

class B
{
}

This program compiles. If you uncomment the conversion operator, it fails to compile.

@masonwheeler
Copy link

@gafter Sounds like a bug in overload resolution, then. A direct match should be treated as a higher priority than an implicit-conversion match.

@HaloFour
Copy link
Contributor

Neither of the overloads are a direct match which I imagine is why the implicit operator is considered. If the call was M(a, 0, 0L) then it wouldn't be ambiguous.

@masonwheeler
Copy link

@HaloFour Ouch! I missed that detail. :(

@benoit-sauropod
Copy link

How about allowing explicit (but cost-less) conversion between the compatible delegates? Would that be an incompatible change / could that break existing programs? Being able to convert between delegates without overhead would be a very useful feature for us.

@svick
Copy link
Contributor

svick commented Aug 15, 2019

@benoit-sauropod What cost are you worried about?

If you're worried about the overhead of invoking the converted delegate, then you can bypass that by using Delegate.CreateDelegate(targetType, source.Target, source.Method).

If you're worried about the overhead of creating the converted delegate, then the conversion using .Invoke is quite cheap. (And you can make the CreateDelegate approach cheaper by instead generating code that directly invokes the delegate constructor.)

If you're worried about both costs, then I don't know about a good solution that exists today.

@mburbea
Copy link

mburbea commented Aug 15, 2019

@svick, Delegate.CreateDelegate still is creating a delegate and is also doing a whole bunch of validations.
You can do the following, but I don't know if this is remotely a good idea.

Func<int, string> x = i => $"this worked?{i}";
Converter<int, string> y = Unsafe.As<Converter<int, string>>(x);
Console.WriteLine(y(3));
Console.ReadLine();

Which prints This worked? 3 as expected. There is no cost of creating the delegate.
Now it's of course ugly, but it shows that they truly are equivalent under the covers.

@svick
Copy link
Contributor

svick commented Aug 15, 2019

@mburbea

I think doing that is generally a bad idea, because you end up with a "confused" object. E.g.:

Console.WriteLine(y.GetType()); // System.Func`2[System.Int32,System.String]
Console.WriteLine(y is Converter<int, string>); // True
Console.WriteLine((object)y is Converter<int, string>); // False

Because of that, I think you have to create a new object when you want to convert to a different delegate type.

@Pzixel
Copy link

Pzixel commented Aug 15, 2019

I'd like to have an explicit cast, but C-like casts are very ugly and with current C# types resolver it's impossible to create a convinient extension method (Like Rust's Into::into for instance).

So it remains yet another bad C# feature.

@masonwheeler
Copy link

There's no good reason why Delegate.CreateDelegate is necessary.

I actually implemented this in the Boo compiler. If the signatures of the two delegates are identical, (and only if the signatures are identical!) it's perfectly safe for the compiler to implement assignment by calling the constructor on the delegate type, which takes two arguments: the target object (or null for a static method) and the addressof the method, which can be extracted from the source delegate. No need for the overhead of Delegate.CreateDelegate's validation, as the relevant information is statically known at compile-time. Just create the new delegate object and you're good to go.

For obvious reasons, this should only happen inside a compiler, rather than users doing this in source code.

@HaloFour
Copy link
Contributor

@masonwheeler

That doesn't work with multicast delegates. The Target property will only return the target of the last delegate in the invocation list. Delegate.CreateDelegate takes care of all of this for you.

@StealthMiniBoss
Copy link

Not every feature of C# has to be tied to the CLR. A typedef feature in C#--which is sorely needed anyway--would largely solve this problem.

While I'm at it, the common concept of backwards compatibility for programming languages is miserable. People commonly make an argument like, we can't add great feature X because it won't be backward compatible. This idea is overly restrictive, and basically means the language must continue to be miserable in certain ways indefinitely.

This problem is also easy to solve. One should simply be able to specify the language version valid for their source code. This could be a compiler option, or better, simply a statement at the beginning of the file, like "using language version x.y.z". Then the latest compiler can simply invoke older compilers as necessary. Compatibility between language versions only needs to be a the IML level. If you take this view of backwards compatibility, then you are free to make your language beautiful, and a pleasure to use.

Cheers.

@Pzixel
Copy link

Pzixel commented Aug 29, 2019

While I'm at it, the common concept of backwards compatibility for programming languages is miserable.

But it stays

This idea is overly restrictive, and basically means the language must continue to be miserable in certain ways indefinitely.

Yeah, if language have a flaw it will keep it until death, that's how things work. Some languages like Rust have epochs mechanism that allows it to deprecate some features, but C# has nothing alike.

This problem is also easy to solve. One should simply be able to specify the language version valid for their source code.

It's possible but the game doesn't worth the candle. Some people don't even consider this to be a problem.

@gafter gafter transferred this issue from dotnet/roslyn Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests