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

Champion "Covariant Return Types" (VS 16.8, .NET 5) #49

Open
1 of 5 tasks
gafter opened this issue Feb 9, 2017 · 123 comments
Open
1 of 5 tasks

Champion "Covariant Return Types" (VS 16.8, .NET 5) #49

gafter opened this issue Feb 9, 2017 · 123 comments
Assignees
Labels
Design Review Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Milestone

Comments

@gafter
Copy link
Member

gafter commented Feb 9, 2017

@gafter gafter self-assigned this Feb 9, 2017
@gafter gafter added this to the X.0 candidate milestone Feb 22, 2017
@Joe4evr
Copy link
Contributor

Joe4evr commented Jun 23, 2017

This would apply to methods and properties, and be supported in classes and interfaces.

The part about support in interfaces, does this mean something like the following?

public interface IFoo { }
public interface IBar
{
    IFoo Get();
}
public interface IBaz : IFoo { }
public interface IQux : IBar
{
    override IBaz Get(); //or would it be 'override IBaz IBar.Get();'?
}

I guess it's lucky that discussions around Default Interface Implementations have pushed back a little bit on using override as a keyword for just that. Even if it wasn't, there probably could've been a way for the two to co-exist, but if you're talking about adding one new thing to interfaces, it would be good to consider the other things that are proposed to be added to interfaces, and make sure that one design choice doesn't preclude any other features entirely.

@JonHanna
Copy link

I find myself wishing C# supported covariant returns very often, and often with code that isn't particularly "clever".

@TylerBrinkley
Copy link

Covariant returns would be very useful for typed cloning methods, preventing the need to manually create unique protected cloning methods.

@ghost
Copy link

ghost commented May 25, 2018

@gafter @Joe4evr @JonHanna @TylerBrinkley @srivatsn
Or better:

public interface IQux : IBar
{
    this Get();
}

or better #1566 :

public interface IQux : IBar
{
    [IBaz] Get(); 
}


@ghost
Copy link

ghost commented May 25, 2018

Why didn't this happen yet?
It seems popular enough.

@HaloFour
Copy link
Contributor

@MohammadHamdyGhanem

Why didn't this happen yet?
It seems popular enough.

Because you haven't written it yet.

Seriously, there are a million "good" ideas, modifying the language is a very expensive process and the team has finite resources. Things take time.

@ghost
Copy link

ghost commented May 25, 2018

@HaloFour

Because you haven't written it yet.

If I can write "it", I would rather write a new S# (Smart Sharp) language, where I do whatever I want :).

Anyway, may God help the team :)

@yaakov-h
Copy link
Member

Would this need CLR changes, or compiler changes only?

@BreyerW
Copy link

BreyerW commented Jun 10, 2018

According to linked proposal document it is purely compiler magic. It is mentioned that compiler will spit two methods instead of one. One virtual override and another who will call this override and cast result to more specific instance so nothing fancy. Though for that reason alternative mentioned in proposal SEEMS to be better for performance but that will come at the cost of ugliness in code (method duplications).

I wonder if both couldnt be combined into one proposal? So compiler will produce same code to alternative but source code will look like original proposal.

@NickRedwood
Copy link

For a long time I didn't realise that the problems I had with many of my inheritance/interface heirarchies nearly always came back to this common problem - the lack of return type covariance to allow overriding methods to narrow their return types. This then leads to duplication of methods and properties to satisfy both the base and the subclass contracts, which is a whole lot of general messyness.

So I am very much in favour of this proposal.

@YairHalberstadt
Copy link
Contributor

@gafter
I think this feature is an extremely important one, and would be willing to put some work towards implementing it. I know @ldematte did some work on this, and I can build on his proposal.

If you think this is likely to succeed, I will go ahead. Some pointers on the process I will need to take would be helpful.

@ldematte, I assume you are no longer working on this, but if you are, please let me know, and we can discuss if I can help. Also, if you've achieved anything useful so far, it would be great if I could build on that.

Also @sharwell and @HaloFour were arguing on how best to implement this, and no decision seemed to be made. Should I just go for this with Method Shadowing, or do we need to finish that discussion first?

@gafter
Copy link
Member Author

gafter commented Sep 12, 2018

I think this will require some (perhaps substantial) design work before the desired/best implementation techniques are decided/agreed.

@YairHalberstadt
Copy link
Contributor

@gafter
What would that involve? Would a proposal containing C# and target IL for a variety of examples be sufficient?

@gafter
Copy link
Member Author

gafter commented Sep 13, 2018

@YairHalberstadt That would help. It would also help to understand how this would affect any tools that consume IL, including compilers for other languages.

@YairHalberstadt
Copy link
Contributor

Ok. I will get started on a design proposal.

@ldematte
Copy link

To save you a little time, fell free to recycle the work I did when I was looking at feasibility.
There is already a mock C# syntax (which is really simple), target IL compiled with ILASM and tested with the C# and VB compilers. And with ILASM/DASM and reflector.
A tool that needed modification for sure was Visual Studio: intellisense got confused, producing strage/non intuitive suggestions, but it worked (no crashes).
Do you have a link at my work/gists?

@ldematte
Copy link

@YairHalberstadt
Copy link
Contributor

Thanks Idematte. I'll definitely use whatever I can from your work.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Sep 14, 2018

A tool that needed modification for sure was Visual Studio: intellisense got confused, producing strage/non intuitive suggestions, but it worked (no crashes).

Can you be more specific? What strange/unintuitive suggestions are you referring to? Thanks!

@ldematte
Copy link

Honestly, I can't recall.
Mind you, the test was done a couple of years ago, meaning VisualStudio 2015, I don't know how VS2017 could react. I probably still have the dlls though, I can try them again (or recreate them from .il)

@ldematte
Copy link

I have just tried it with VS2017 and it works flawlessly, proposing them as overloads (even it is just the return type changing).
I still have a test project with a compiled dll in my dropobox if you want to try it out.

@YairHalberstadt
Copy link
Contributor

YairHalberstadt commented Sep 14, 2018 via email

@ldematte
Copy link

https://www.dropbox.com/sh/hw0sgrae83i93lm/AAC_YXcNxjY3q70JExCoAWfOa?dl=0

There is also a draft proposal I forgot I wrote :)
And a console application with some performance tests too (the stub was designed to work efficiently across multiple subclasses)

@fandrei
Copy link

fandrei commented Oct 28, 2024

@CyrusNajmabadi

Just as I can use expressions to build up arbitrarily complex patterns (including collections), I want the same ability to break apart arbitrary graphs (including collections) with patterns.

Are you sure that this is what end users need, rather than something that you C# team developers need?

@mikernet
Copy link

Pattern matching, especially when combined with collection pattern matching, is incredibly powerful and succinct. It can easily turn 20 lines of difficult to follow code into a simple switch expression with a few lines. I use it all the time in normal app development.

@mikernet
Copy link

mikernet commented Oct 28, 2024

There are lots of examples in the search Cyrus Jared provided above. It's not hard to skim over the ones that aren't relevant and find good examples in those search results.

@Charlieface
Copy link

Can the mods please delete the above discussion so we can focus on the matter at hand:
Covariant returns in interfaces, an extremely useful feature that would substatially simplify many hierarchies that currently use generics or downcasting.
What actually needs to happen (physically) to get this implemented? How much is the CLR subsystem affected (different archs etc)? How much is Roslyn affected? How much is the reflection API affected?

@fandrei
Copy link

fandrei commented Oct 28, 2024

@mikernet

It's not hard to skim over the ones that aren't relevant and find good examples

It's also easy to find some pretty awful examples.

It can easily turn 20 lines of difficult to follow code into a simple switch expression with a few lines.

I'd love to see an example, but it looks like we digressed too much. Probably better to move this discussion to somewhere else.

@mikernet
Copy link

Does it even need to be implemented? You can achieve similar results with DIMs on interfaces. Where is the lack of this feature overly burdensome?

interface IBase
{
    object GetValue();
}

interface ISub : IBase
{
    new string GetValue();
    object IBase.GetValue() => GetValue();
}

class C : ISub
{
    public string GetValue() => "Wooo";
}

@mikernet
Copy link

It was a huge pain with classes because you couldn't override a base abstract method and provide a more specific one with the same name. It's not an issue in interfaces.

@fandrei
Copy link

fandrei commented Oct 28, 2024

So, a few examples of people asking for a solution of this "unimportant" problem:

#4372
#2844
#296
#2589
#930

@fandrei
Copy link

fandrei commented Oct 28, 2024

I think that simply auto-adding stub methods would be enough. Unlikely that changing runtime is really needed.

@Charlieface
Copy link

@mikernet
That's a pretty hacky workaround, which requires duplicating every such function, along with its XML doc. And it's got some weird corners where you can can work around the expected results, by reimplementing the DIM.

interface IBase
{
    object GetValue();
}

interface ISub : IBase
{
    new string GetValue();
    object IBase.GetValue() => GetValue();
}

class Evil : ISub
{
    public string GetValue() => "Foo";
    object IBase.GetValue() => "Bar";
}

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi

Just as I can use expressions to build up arbitrarily complex patterns (including collections), I want the same ability to break apart arbitrary graphs (including collections) with patterns.

Are you sure that this is what end users need, rather than something that you C# team developers need?

Nope. I'm not sure. But what of it? Ultimately, certainty is not the goal.

@CyrusNajmabadi
Copy link
Member

There are lots of examples in the search Cyrus provided above.

That was Jared. But we are often easily mistaken.

@CyrusNajmabadi
Copy link
Member

I think that simply auto-adding stub methods would be enough. Unlikely that changing runtime is really needed.

This is possible today with source generators. If this is enough, if advise you to just implement that as you have a solution today (which could work across all version I if c#), versus waiting years for us to do it.

@mikernet
Copy link

@Charlieface I don't see it as hacky, it's relatively simple. I don't really see the problem, but that's just me. Even if override was provided, you would still be able to work around the expected results:

interface IBase
{
    object GetValue();
}

interface ISub : IBase
{
    override string GetValue();
}

class Evil : ISub, IBase
{
    public string GetValue() => "Foo";
    object IBase.GetValue() => "Bar";
}

@fandrei
Copy link

fandrei commented Oct 29, 2024

@CyrusNajmabadi gosh. I wasn't looking for _any_solution, there is a number of obvious ones. I just want this thing to be fixed finally.

@HaloFour
Copy link
Contributor

@mikernet

It was a huge pain with classes because you couldn't override a base abstract method and provide a more specific one with the same name. It's not an issue in interfaces.

If you really want to blow minds about the whole "consistency" argument, .NET has always allowed for "explicit" overrides, C# has just never supported them. Covariant returns might have not been necessary at all if it did. 😁

@CyrusNajmabadi
Copy link
Member

@jaredpar it also matches a lot of the code that constructs strings, and the code that could be easily rewritten in a different way,

Why does that matter? Lots of code can be rewritten in a different way. Indeed, that applies to most code.

I wanted to ask if you have a way to know how much different features are actually used, using a real C# parser, but it looks like you don't.

If that's what you want to know, then ask that.

Anyway, I don't remember many people asking about list pattern matching in C# on StackOverflow

I don't see how that's relevant. We don't design asking ourselves how much a particular person remembered feedback on StackOverflow. We design based on what we think will provide the most value to the ecosystem as a whole. This will always be subjective, and it's entirely true that for every single release there will be people that disagree with the decisions made. This - applies to us and literally ever mainstream language ever created.

I'm not sure what you're hoping to gain here. We thought investing in patterns was worthwhile. Nothing has changed our opinions here. Similarly, investing in covariant returns for interface implementation has not been worthwhile.

These are our choices for the past, and relitigating them won't change them. Am you can do is try to work with us on future decisions. You're interested in certain things. The best of option for you is to make the best cases for investment there, and see if the community and designers agree with you.

@CyrusNajmabadi
Copy link
Member

along with its XML doc.

Inheritdoc is a thing.

which requires duplicating every such function

Here's the thing. You consider that not acceptable. But we may disagree. Ultimately, we have to choose which tiny handful of features we pick. And if we think the workarounds are not so bad, then c'est la vie.

Language design and scenario prioritization age ultimately entirely subjective processes. Your subjective assessment differs from ours. That's life.

--

Reminder, my subjective opinion differs from many on the team. I likely have more rejected proposals than nearly anyone else out there. That's not something that bothers me. It just means figuring out better how to convince people and make good specifications for the ecosystem as a whole.

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi gosh. I wasn't looking for _any_solution, there is a number of obvious ones. I just want this thing to be fixed finally.

Then find a way for this to be viewed as not important than all the other things that we and the ecosystem want "fixed finally" :-)

I'm ok with the status quo. If you're not, then I wish you best of luck on what to do about that.

@Charlieface
Copy link

@mikernet
No, that would be impossible, because they are the same "slot". CLR uses a slot-style vtable, so an override takes over the slot of the base member, which cannot be further overridden separately from the derived's member.

@HaloFour
Yes, it annoys me that a feature that VB had from the start is still unavailable in C#. (I'm referring to explicit overrides rather than covariant returns, which VB still doesn't allow due to CLR blocking). In VB you can rename an overridden function, or implement multiple base/interface functions with a single one.

@CyrusNajmabadi

Inheritdoc is a thing.

I think inheritdoc won't work with a new function, you'd still need to copy it once for each duplicated function.

@CyrusNajmabadi
Copy link
Member

I think inheritdoc won't work with a new function, you'd still need to copy it once for each duplicated function.

You can just place it wherever you want, including on new functions.

@fandrei
Copy link

fandrei commented Oct 29, 2024

@CyrusNajmabadi

Then find a way for this to be viewed as not important than all the other things that we and the ecosystem want "fixed finally" :-)

You probably wanted to say "more important". I already listed some of the issues created for this problem before.

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi

Then find a way for this to be viewed as not important than all the other things that we and the ecosystem want "fixed finally" :-)

You probably wanted to say "more important". I already listed some of the issues created for this problem before.

Phone autocorrect strikes again

@CyrusNajmabadi
Copy link
Member

already listed some of the issues created for this problem before.

Great. We'll consider this when evaluating our backlog.

@Charlieface
Copy link

Great. We'll consider this when evaluating our backlog.

How often does that evaluation happen? I struggled to get any clear info on it re a different issue.
Like is there an approx cycle to it, or once per langver etc?

@CyrusNajmabadi
Copy link
Member

Great. We'll consider this when evaluating our backlog.

How often does that evaluation happen? I struggled to get any clear info on it re a different issue.
Like is there an approx cycle to it, or once per langver etc?

It's fairly continuous. Occasionally we take a little break to do nothing but reevaluation.

@mikernet
Copy link

mikernet commented Oct 29, 2024

No, that would be impossible, because they are the same "slot". CLR uses a slot-style vtable, so an override takes over the slot of the base member, which cannot be further overridden separately from the derived's member.

I highly doubt that is how this would work. If you look at my code, you'll see that the class states that it implements both interfaces, so it can specify how it implements the base interface member:

class Evil : ISub, **IBase**

It's kind of similar to how you can do:

public class MyList<T> : List<T>, ICollection
{
    public int Count => -999;
    // or: int ICollection.Count => -999;
}

I doubt that this feature would be designed such that you would be blocked from redeclaring an interface implementation.

@mikernet
Copy link

I think inheritdoc won't work with a new function, you'd still need to copy it once for each duplicated function.

inheritdoc has a cref attribute that can point at anything you want - a method in an entirely different class, a base implementation, etc. It doesn't need to point at something that it is actually inheriting.

@mikernet
Copy link

@dotnet dotnet locked and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Review Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Projects
None yet
Development

No branches or pull requests