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: base(T) phase two #2337

Open
3 of 5 tasks
gafter opened this issue Mar 12, 2019 · 31 comments
Open
3 of 5 tasks

Champion: base(T) phase two #2337

gafter opened this issue Mar 12, 2019 · 31 comments
Assignees
Milestone

Comments

@gafter
Copy link
Member

gafter commented Mar 12, 2019

On 2019-02-27 the LDM met to make some decisions about the default interface methods feature.

[Plan A]: We decided that the best design for the base(I).M feature would be that we lookup M in the interface type I, and the result must be accessible. Moreover, there is a requirement that if the found member is abstract, the type I must have a unique most specific (concrete) implementation in the type I. The compiler would emit IL that identifies the method found, and the type I, and the runtime would select the most specific (concrete) implementation in the type I and invoke it (or throw an exception if there is no unique most specific implementation at runtime). There is no IL defined today that would serve this purpose, so we would have to design it. (One option would be to generate a "constrained" prefix with the interface type I). The IL should not assume or require that the implementing method is accessible to the caller (e.g. it could be private), but it does require that the declared method named in the IL is accessible.

Unfortunately, we do not believe we have the resources to design and implement the feature in this form (plan A) before we'd like to deliver its first implementation (hopefully in C# 8.0). So initially, we'll ship a language feature that is more constrained. This is our plan B for C# 8.0:

[Plan B]: We lookup M in the interface type I, and the result must be accessible. Moreover, there is a requirement that the found member must have an implementation directly in the type I, and that implementation method must be accessible. The compiler will produce IL that directly (non-virtually) invokes that method.

We expect to ship plan B in C# 8.0.

This issue is for moving to plan A in a subsequent language version.

Update: LDM 04-29-2019 cut base() from C# 8.0.

@gafter gafter self-assigned this Mar 12, 2019
@YairHalberstadt
Copy link
Contributor

Would it be possible to add an example of what would be possible under plan A, but is not under plan B? Thanks

@Joe4evr
Copy link
Contributor

Joe4evr commented Mar 13, 2019

If I understand correctly:

public interface IFoo
{
    int M();
}
public interface IBar : IFoo
{
    int IFoo.M() => 42;
}
public interface IBaz : IFoo, IBar
{
    int X1() => base(IBar).M(); //Plan B
    int X2() => base(IFoo).M(); //requires Plan A
}

@YairHalberstadt
Copy link
Contributor

@Joe4evr
Was that meant to be IBaz : IBar?

@Joe4evr
Copy link
Contributor

Joe4evr commented Mar 13, 2019

Not necessarily. Under Plan A, the method implementation could come from somewhere else and would be looked up at runtime. But I'll adjust anyway.

@YairHalberstadt
Copy link
Contributor

In that case what's the difference between calling base(IFoo).M and call M directly? A case when IBar or IBaz hides M?

@gafter
Copy link
Member Author

gafter commented Mar 13, 2019

@Joe4evr Not quite

public interface I1
{
    int M();
}
public interface I2 : I1
{
    int I1.M() => 42;
}
public interface I3 : I2
{
}
public interface C : I3
{
    int X1() => base(I2).M(); // Plan B
    int X2() => base(I3).M(); // requires Plan A
}

@gafter gafter added this to the 9.0 candidate milestone Apr 29, 2019
@Thaina
Copy link

Thaina commented Aug 15, 2019

Sorry that it might be too late but could we change syntax to base<T> instead of base(T)

I still feel awkward to have type in parentheses and thinking that syntax related with type should always be <T> if possible

@HaloFour
Copy link
Contributor

@Thaina
Copy link

Thaina commented Aug 15, 2019

@HaloFour Are there anywhere I could look for reason behind this decision?

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 16, 2019

https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-06-14.md#syntax

@Thaina
Copy link

Thaina commented Aug 16, 2019

@Joe4evr Thank you very much

Well, actually I wish that typeof sizeof and default should changed to use <T> too instead or additionally, and we would always use <T> as a type related argument everywhere

@gafter
Copy link
Member Author

gafter commented Sep 30, 2019

There is a thread starting at #406 (comment) with some users noting how they would really like to use this functionality. I'm posting this here in case it is useful in prioritizing our candidates for C# 9.0.

@gafter
Copy link
Member Author

gafter commented Oct 25, 2019

There is a draft specification for base(T) at #2910

@gafter
Copy link
Member Author

gafter commented Oct 29, 2019

Prioritization of runtime support for this is being tracked internally at https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1009879

@IS4Code
Copy link

IS4Code commented Jan 16, 2020

This seems to be especially useful for structs, as calling an explicit interface implementation via a base lookup would remove the need for boxing to the specific interface type, and thus also allow modifications of the original value done by the called method.

@gafter
Copy link
Member Author

gafter commented Jan 16, 2020

@IllidanS4 that is not how interface methods work. They always have a reference-typed this.

@Thaina
Copy link

Thaina commented Jan 16, 2020

@gafter So you mean DIM will always box struct?

@gafter
Copy link
Member Author

gafter commented Jan 16, 2020

Yes. We tried to design it so it would not, but it would have made things much more complicated.

@Thaina
Copy link

Thaina commented Jan 16, 2020

Thank you, well, yet another reason I should try to avoid DIM as much as possible

@gafter
Copy link
Member Author

gafter commented Jan 16, 2020

@Thaina how do you imagine this boxing would occur without the base(T) feature?

@Thaina
Copy link

Thaina commented Jan 16, 2020

@gafter Or did you mean normal DIM will not box struct, only when we did override DIM and then try to call the original interface DIM?

@YairHalberstadt
Copy link
Contributor

how do you imagine this boxing would occur without the base(T) feature?

When an interface is used as a type constraint...

@IS4Code
Copy link

IS4Code commented Jan 16, 2020

@gafter I was referring to this case:

struct Struct : IEnumerable<object>
{
    public IEnumerator<object> GetEnumerator()
    {
        yield break;
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        yield break;
    }

    public IEnumerator GetNonGenericEnumerator()
    {
        return ((IEnumerable)this).GetEnumerator(); //boxes
        return base(IEnumerable).GetEnumerator(); //calls IEnumerable.GetEnumerator directly
    }
}

Seems like that's what should happen, as base(T), as I understand it, should call the method on this but perform the lookup on a base class or interface. At this place, the compiler should know perfectly which method to call, and if that is not the case, it seems to me it would create some confusion, if it is hiding the boxing operation.

Of course one could argue that this code should be designed the other way around, but I've also run into this situation with partial structs where the auto-generated part expected the struct to implement a particular set of generic interfaces, differing only in the return type for one method, I couldn't simply create a new custom-named method for every one of them.

@gafter
Copy link
Member Author

gafter commented Jan 16, 2020 via email

@IS4Code
Copy link

IS4Code commented Jan 16, 2020

My mistake, I just forgot that interfaces can contain implementation. But still, if the interface doesn't provide the implementation, this construct could be used to refer to the implementation present on the current type, couldn't it?

@gafter
Copy link
Member Author

gafter commented Jan 16, 2020

No

@JinShil
Copy link

JinShil commented Mar 4, 2020

The default interface members proposal had an example that allowed for overriding default implementations:

using static System.Console;

interface IA
{
    void M() 
    {
        WriteLine("IA.M"); 
    }
}
interface IB : IA
{
    override void IA.M()    // The modifier 'override' is not valid for this item
    {
        WriteLine("IB.M");
    }
}
interface IC : IA
{
    override void IA.M()    // The modifier 'override' is not valid for this item
    { 
        WriteLine("IC.M"); 
    }
}

Currently that results in compiler errors as shown above. Did the ability to override implementaions get cut from C#8 along with base(T) or are the two on different implementation paths?

@gafter
Copy link
Member Author

gafter commented Mar 5, 2020

@JinShil Drop the override keyword. I think that should work.

@MadsTorgersen MadsTorgersen modified the milestones: 9.0 candidate, 10.0 candidate Apr 22, 2020
@MadsTorgersen MadsTorgersen modified the milestones: 10.0 candidate, X.0 candidate Sep 9, 2020
@JinShil
Copy link

JinShil commented Feb 24, 2022

Just a friendly ping on this issue. Looking at the list of upcoming C# 11 features, they aren't nearly as valuable, and even somewhat superficial, compared to the ability to call base default interface methods. Default interface methods will always be incomplete without this feature, and those creating complex hierarchies, and wishing to maximize code reuse, will always run into this limitation. Due to this limitation I have begun using source generators as a substitute. Please finish this implementation.

@zvrba
Copy link

zvrba commented Jul 5, 2024

Any progress on this? I stumbled upon another issue that'd be solved by base invocation: #8269

@westy
Copy link

westy commented Oct 7, 2024

Agree that this is badly needed, since could be a massive source of bugs in years to come when interfaces are used without detailed knowledge of their implementation!

/subscribed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants