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

Issues in Default Interface Methods #406

Closed
gafter opened this issue Apr 4, 2017 · 142 comments
Closed

Issues in Default Interface Methods #406

gafter opened this issue Apr 4, 2017 · 142 comments
Assignees
Labels
Milestone

Comments

@gafter
Copy link
Member

gafter commented Apr 4, 2017

The following notes are intended to prime the pump for LDM discussion as soon as the LDM has time to take up these issues.

Issues in Default Interface Methods

protected, private protected, and protected internal access (closed)

We tentatively decided to permit protected members in interfaces, admitting we need to specify the meaning of protected, internal, and related access modes. The draft spec permits these but does not describe the meaning. We also need to ensure the runtime agrees to implement them, if needed.

We may need to use protected members in interfaces for explicit interface implementations so that we can generate code for base(T).M() that has access to those explicit implementations: the only way to ensure that we invoke an accessible method is to make such methods protected from the IL point of view.

Here is what runtime spec was saying about accessibility of interface members prior to this feature:

I.8.9.4 Interface type definition
However, since accessibility attributes are relative to the implementing type rather
than the interface itself, all members of an interface shall have public accessibility

We were pretty comfortable with allowing all accessibility attributes that do not include protected access because those were pretty well understood and do not depend on inheritance relationship between types:

I.8.5.3.2 Accessibility of members and nested types
family – accessible to referents that support the same type (i.e., an exact type and all of
the types that inherit from it). For verifiable code (see §I.8.8), there is an additional
requirement that can require a runtime check: the reference shall be made through an
item whose exact type supports the exact type of the referent. That is, the item whose
member is being accessed shall inherit from the type performing the access.

As it turns out the runtime specification for the meaning of inheritance explicitly describes its meaning for interfaces:

I.8.9.8 Type inheritance
Inheritance of types is another way of saying that the derived type guarantees support for all of the type
contracts of the base type. In addition, the derived type usually provides additional functionality or specialized
behavior. A type inherits from a base type by implementing the type contract of the base type. An interface type
implements zero or more other interfaces...

This provides a clear meaning for protected in the runtime specification, which we could mirror in the language specification.

Resolution: We will support these protection levels in interfaces. A protected member in an interface is accessible in all interfaces that derive from it. A protected member in object is not accessible in an interface. A protected member in an interface is accessible through a base(I) qualifier in a class or struct type that implements the interface.


reabstraction (closed)

We specified our intent to permit reabstraction (See https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md#reabstraction and https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-03-21.md#reabstraction) and it is required for Java interop (See https://grokbase.com/t/openjdk/lambda-dev/11cvy137rq/what-does-reabstraction-mean) which is one of our key scenarios. Consequently the only specified form for reabstraction is

    void I.M();

The abstract modifier is currently explicitly forbidden for explicit implementations. Do we intend to also (for clarity) permit something like

    abstract void I.M();

Or is the former form sufficient?

We also need to ensure the runtime agrees to implement reabstraction, if needed.

Resolution: We allow reabstraction, and the abstract modifier is required (and permitted) on an abstract implementation. https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md#reabstraction


explicit interface abstract overrides in classes (closed)

Should we support explicit interface abstract overrides in classes?

Resolution: No. A class must implement every interface method, directly or indirectly. It may, as ever, do so using an abstract method of the class. https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md#explicit-interface-abstract-overrides-in-classes


Is object.MemberwiseClone() accessible in an interface? (closed)

Resolution: No. Protected members of object are not accessible in interfaces. https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md#is-objectmemberwiseclone-accessible-in-an-interface


Confirm: static int P { get; set; } is an auto-property with a compiler-generated backing field? (closed)

Resolution: Yes. https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md#is-static-int-p--get-set--is-an-auto-property-with-a-compiler-generated-backing-field


Confirm: partial on a method declaration implies private, and no access modifier is permitted? (closed)

The existing language specification says that partial methods are implicitly private and that no access modifier is permitted. This item is for the LDM to either confirm that position when they are declared in an interface, permit private on them, require private on them, or something else.

Resolution: Yes, confirmed. https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md#confirm-that-partial-implies-private-and-no-access-modifier-is-permitted


static fields (closed)

Open Issue: Are static fields and the like permitted in interfaces?

Resolution: Yes.


operators (closed)

Closed Issue: Should operator declarations be permitted in an interface? Probably not conversion operators, but what about others?

Decision: See https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-06-27.md for details.


new (closed)

Closed Issue: Should new be permitted on interface member declarations that hide members from base interfaces?

Resolution: new is supported on interface members for a long time. It simply suppresses a warning. See https://github.com/dotnet/csharplang/blob/master/spec/interfaces.md#interface-methods


const (closed)

Open Issue: Should const declarations be permitted in an interface?

Resolution: Yes.


System.Runtime.CompilerServices.RuntimeFeature.DefaultInterfaceImplementation (closed)

Is that the best name for the CLR feature? The CLR feature does much more than just that (e.g. relaxes protection constraints, supports overrides in interfaces, etc). Perhaps it should be called something like "concrete methods in interfaces", or "traits"?

As it stands right now the name of the member is DefaultImplementationsOfInterfaces.

Decision: The LDM doesn't care what its name is.


base(Type).Member for class types (closed)

Open Issue: Confirm base(Type).Member permitted for class types.

We think we approved using the new base(Type) syntax where Type is a class type (e.g. to skip a base and invoke your base's base), but we should explicitly confirm that. Also we should confirm that base(Type).M() might refer to a non-virtual member M. Also we should confirm that there is an accessibility requirement: the M found by this lookup must be accessible where the invocation occurs (i.e. the usual name-lookup constraint).

Decision: Yes.

@qrli
Copy link

qrli commented Apr 5, 2017

For the virtual issue: It is essentially to choose between being consistent with interface style or class style. If we were to use this feature lightly, where we would typically only add a few default implementations, then the interface style makes more sense. But if we were to put a lot of logic (including private methods) into interfaces, the class style makes more sense. IMO, the later case will happen more as time goes.

Another idea (maybe already raised by others) is to actually put the class stuff into a class, e.g.:

interface I
{
  void M();

  default class C : I
  {
    public virtual void M() { ... }
  }
}

So no need for a lot of new rules to learn and remember.

@qrli
Copy link

qrli commented Apr 5, 2017

For struct: I don't think the boxing need to be solved here. It is a more general issue with struct, and we have lived with it til now. The only issue is I expect

var s = default(S);
s.M(); // error: 'S' does not contain a member 'M'

to work. But if it is hard, I can also live with 1. Forbid a struct from inheriting a default implementation

@gafter
Copy link
Member Author

gafter commented Apr 5, 2017

I've added more open issues to the OP, starting with "Base interface invocations"

@orthoxerox
Copy link

Open Issue: What is the syntax for a base member invocation?

IFoo.base.M() works, but makes it look like you're calling the base of IFoo, not IFoo as the base type. I propose to use base<IFoo>.M().

@orthoxerox
Copy link

Open Issue: Should a concrete method without implementation be implicitly virtual?

The question is confusing (or I am too tired), but I think it's supposed to say "with an implementation". I think requiring an explicit modifier on all default methods is the better option. A "plain old method" would then be sealed without an override. "Implicitly virtual" makes default methods work like regular interface methods, but unlike class methods. "Implicitly plain old" makes them work like class methods, but unlike regular interface methods. Both options are potentially confusing.

Requiring a modifier forces the programmer to make an explicit choice.

@HaloFour
Copy link
Contributor

HaloFour commented Apr 5, 2017

@orthoxerox

That syntax feels very Java. Maybe base(IFoo).M ()?

@gafter gafter changed the title 2017-04-05 Issues in Default Interface Methods Issues in Default Interface Methods Apr 5, 2017
@gafter
Copy link
Member Author

gafter commented Apr 6, 2017

Added some resolutions from today's LDM, including

Closed Issue: Should a concrete method (with implementation) be implicitly virtual? [YES]

@gafter
Copy link
Member Author

gafter commented Apr 6, 2017

I like the syntax base<IFoo>.M()

@alrz
Copy link
Member

alrz commented Apr 6, 2017

Why this ((IFoo)this).M(); can't work though? It worked for classes so far.

@yaakov-h
Copy link
Member

yaakov-h commented Apr 6, 2017

Wouldn't that just cause a stack overflow, if it's virtual?

@qrli
Copy link

qrli commented Apr 7, 2017

@alrz Because you want to invoke the base interface's default implementation rather than the current override implementation.

@alrz
Copy link
Member

alrz commented Apr 7, 2017

Nevermind, apparently it doesn't work that way.

I see base(T) has been chosen cause it works like default(T). That's great.

@jveselka
Copy link

jveselka commented Apr 9, 2017

Regarding base interface invocation, I'm not a fan base<IFoo>.M() syntax. There is no generic involved so why make it look like one? Its especially weird with generic interface base<IEnumerable<int>>.Count().

base(IFoo).M() looks much more in-home in C# next to typeof(), default() and nameof()

Regarding diamond inheritance

One override M1 is considered more specific than another override M2 if M1 is declared on type T1, M2 is declared on type T2, and either

  1. T1 contains T2 among its direct or indirect interfaces, or
  2. T2 is an interface type but T1 is not an interface type.

I don't think that the second rule is useful. It's better to require derived class to implement explicit override.

Regarding Binary Compatibility issues, they are very similar to concerns raised in #409. Following existing C# logic for base invocation, Binary Compatibility 1 should (although probably unintuitively) run I1.M() and Binary Compatibility 2 should run I2.M().

@jnm2
Copy link
Contributor

jnm2 commented Apr 9, 2017

Can I put a request in here for calling this(IInterface).ExplicitGetterOnlyAutoProperty = value (non-virtual) and base(IInterface).ReimplementedMethod(), in C# 7 classes implementing C# 7 interfaces? These were always pain points.

Oops, wrong thread. Moved to #52 (comment)

@gafter
Copy link
Member Author

gafter commented Apr 9, 2017

@zippec

I don't think that the second rule is useful. It's better to require derived class to implement explicit override.

That would make it an incompatible change to add default methods to interfaces, because it would change the behavior of this program

interface IA
{
    void M();
}
interface IB : IA
{
}
class Base : IA
{
    void IA.M() { WriteLine("Base"); }
}
class Derived : Base, IB // allowed?
{
    static void Main()
    {
        Ia a = new Derived();
        a.M();
    }
}

when a default method implementation is added like this

interface IA
{
    void M();
}
interface IB : IA
{
    override void M() { WriteLine("IB"); }
}
class Base : IA
{
    void IA.M() { WriteLine("Base"); }
}
class Derived : Base, IB // allowed?
{
    static void Main()
    {
        Ia a = new Derived();
        a.M();           // what does it do?
    }
}

Since we don't want the addition of default methods to an interface to affect the meaning of existing correct programs that do not use default methods, we need the second rule.

See the open question "Diamond inheritance and classes".

@gafter
Copy link
Member Author

gafter commented Apr 19, 2017

The following draft notes from 2017-04-19 LDM have been partially reflected in the open issue list and specification:


DIM Event accessors

Today, syntactically, either both or neither accessor can have an implementation.

Should we allow just one to be specified? Overridden?

Conclusion

If you have only one, you probably have a bug. Let's not allow it. Not blocked for future idiots to ...

DIM Reabstraction

Yes, adding a body to an interface member declaration shouldn't break C.

DIM Sealed override

Should it be allowed? would it prevent overrides in classes or only in interfaces?

It seems odd to prevent either. Also, it is weird in connection with diamond inheritance: what if one branch is sealed?

Conclusion

Let's not allowed sealed on overrides in interfaces. The only use of sealed on interface members is to make them non-virtual in their initial declaration.

DIM sealed members (not on list)

Some folks in the community find it weird, and that they look too much like things that can be implemented in classes.

Conclusion

We think it is going to be useful, but will come back to it. This is a mental model tripping block.

DIM not quite implementing a member (not int list)

You have a member and implement an interface. The interface adds a new member with a default implementation, that looks like your method but doesn't quite make it an implementation. Bug? Intentional? We can't provide a warning, because it would assume it was a bug.

Conclusion

Can't do anything about this.

DIM implementing inaccessible interface members (not in list)

The way the runtime works today, a class member can happily implement an interface member that isn't accessible! That's not likely to be dependent on today (no language will generate that interface), but we need to decide what semantics to have here.

We could continue to only have public members in interfaces be virtual. But if we want protected, internal and private, we should probably have it so they can only be implemented by classes that can see them. But this means that interfaces can prevent other assemblies from implementing them! This may be a nice feature - it allows closed sets of implementations.

Conclusion

This is still open, but our current stake in the ground is we should allow non-public virtual interface members, but disallow overriding or implementing them in places where they are not accessible.

DIM Implementing a non-public interface member (not in list)

Would we allow them to be implemented implicitly? If so, what is required of the accessibility of the implementing method?:

  • Must be public
  • Must be the same accessibility
  • Must be at least as accessible

Conclusion

For now, let's not allow any of these. We can relax as we think through it.

@BreyerW
Copy link

BreyerW commented Apr 19, 2017

I have a question about unfortunate interaction with default members vs structs: How this (will) work with generics constrained to interface with default members? Lets me show example:

interface IDumb{

public virtual void Talk(){
Console.WriteLine("I'm here!");
}

}
struct DumbStruct : IDumb{

}
....
public void ConsumeDumb<T>(T item) where T : IDumb
{
item.Talk(); 
}
....
ConsumeDumb(new DumbStruct()); //print "I'm here!" or throw error?

The thing is, as far as i understand, generic constraints avoid boxing on value types and because of that default members should work flawlessly. But i would like to be sure.

If thats true then i vote for 2->3 ------------------------------>1 in that order (from best to worst) because default members will still be useful on structs even if you decide to leave it as-is (without forbidding nor providing code generator).

Otherwise i still vote on 2 but 3 and 1 become effectively same.

@MI3Guy
Copy link

MI3Guy commented Apr 19, 2017

@gafter

With regard to non-public overrides, isn't this what the strict modifier in IL is supposed to control? I was under the impression that without that any inaccessible method could be overridden at the IL level.

@gafter
Copy link
Member Author

gafter commented Apr 19, 2017

How this (will) work with generics constrained to interface with default members?

That is the essence of the open question. Inside the interface's default method, the this parameter is of an interface type. How could that be unless the receiver was boxed? If it was boxed, then the caller had no way to avoid boxing.

@MI3Guy I don't know anything about that. If your point is that our choice on how this works may affect the IL we elect to emit, I believe that.

@MI3Guy
Copy link

MI3Guy commented Apr 19, 2017

I can't say that I've ever actually heard anyone talk about the strict Method Attribute, but it shows up in the CLR spec (II.15.4.2.2, not sure if there's a better reference) and that's how I understood what it is supposed to do.

@Thaina
Copy link

Thaina commented Apr 26, 2017

There are many things extension method would not have a problem with

For instance. default method for struct could make it as generic constraint interface

public static class Ext
{
    public static void M<T>(this T obj) where T : ISomeThing
    {}
}

struct S : ISomeThing {}

S s;
s.M(); // not boxed

@paulomorgado
Copy link

Regarding base interface member access, what's wrong with this?

interface I0
{
   void M() { Console.WriteLine("I0"); }
}
interface I1 : I0
{
   override void M() { Console.WriteLine("I1"); }
}
interface I2 : I0
{
   override void M() { Console.WriteLine("I2"); }
}
interface I3 : I1, I2
{
   // an explicit override that invoke's a base interface's default method
   void I0.M() { I2.M(); }
}

I2.base looks like accessing the base of I2 and the use of <> without generics being involved seems awkward.

@alrz
Copy link
Member

alrz commented May 7, 2017

@paulomorgado I read in a design note that base(T).M() is the winner as it works like default(T).

@HaloFour
Copy link
Contributor

@timcassell

Not as a part of the DIM proposal. The methods are not virtual, cannot be overridden and would require an implementation. They'd be invoked as IDoSomething.DoSomething(), although maybe the compiler would allow invoking them from implementing types as it does now with static methods on classes. They'd mostly be useful for factory methods.

Type-classes, shapes, extension interfaces, whatever, etc., may eventually provide a way to define static methods that need to be implemented by a target type, but they are still under design.

@raffaeler
Copy link

From this document: https://github.com/dotnet/roslyn/blob/master/docs/Language%20Feature%20Status.md it looks like Preview 5 and VS16.2 Preview 1 already have all the features implemented.
But I still can't use the base(IA).Method() syntax from any derived interface or implementing class.
Is the 'base' thing already implemented?
TIA

@ufcpp
Copy link

ufcpp commented May 25, 2019

See #2485

Conclusion
Cut base() syntax for C# 8. We intend to bring this back in the next major release.

@raffaeler
Copy link

Thank you @ufcpp the information is fragmented and I didn't see that.
What's the current reccomended way to make a concrete interface method emerge in a class implementation?
Scenario:

  • at a certain point you add a method+implementation to an existing interface
  • later on, you decide you want certain classes expose the default implementation when using the class reference as opposed to the interface reference.
    The obvious method would be to add the following to the class:
public string Method() => ((IA)this).Method();

But it looks horrible to me :)
Any other better way before vNext?

Thanks

@orthoxerox
Copy link

Will protected interface methods be allowed, but unusable, in C# 8?

@jnm2
Copy link
Contributor

jnm2 commented Aug 22, 2019

I'm hitting the same thing. I'd expect them to just be in scope and also to be unambiguously callable via this(IFoo).Bar();.

@orthoxerox
Copy link

My specific use case looks like this:

public interface ICloneable<T> where T : ICloneable<T>
{
  T Clone();
}

// the assembly that contains the domain logic

public interface IFooState : ICloneable<IFooState>
{
  public string FooID {get; set;}
  public Bar Bar {get; set;}
  
  protected void CopyTo(IFooState newState)
  {
	newState.FooID = this.FooID;
	newState.Bar = Bar;
  }
}

// a different assembly that persists or proxies FooState objects

public class FooState : SomeBaseClass, IFooState
{
  public string FooID {get; set;}
  public Bar Bar {get; set;}
  
  public IFooState Clone() => ((IFooState)this).CopyTo(new FooState());
}

@gafter
Copy link
Member Author

gafter commented Aug 22, 2019

There is no this(IFoo) syntax. There is also no base(IFoo) syntax yet. The best you can do is something like this:

public interface I1
{
    protected void M1() {}
}

public class C : I1 {
    public void M()
    {
        M1Helper(this);
    }

    private static void M1Helper<T>(T self) where T: C, I1
    {
        self.M1(); // this works
    }
}

@DavidArno
Copy link

There is also no base(IFoo) syntax yet

Well you'd best get a move on and add it. The whole world (well me at least, speaking on behalf of the whole world without consulting them) is expecting a completed v8 to be released next month along with Core 3.0, so we are all going to be bitterly disappointed when this doesn't happen 😉

@raffaeler
Copy link

@DavidArno see #406 (comment)
This feature requires changes in the CLR first. Not sure they have already been made or not.
Anyway, given we are close to the release version, it is definitely too late to add anything.
Just forget it for the moment ;-)

P.S. I don't work in the team and am interested in this feature as well, as you may have read in this thread.

@gafter
Copy link
Member Author

gafter commented Aug 24, 2019

Core 3.0 will not have runtime changes necessary to support the base(T) syntax, so that syntax is not being added to C# 8.0.

@gafter
Copy link
Member Author

gafter commented Sep 12, 2019

All of the open language issues described here were resolved in https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-03-27.md and I have updated the issue list above to show the resolution.

@gafter
Copy link
Member Author

gafter commented Sep 13, 2019

Reopening so a spec can be updated with these resolutions.

@gafter gafter reopened this Sep 13, 2019
@raffaeler
Copy link

@gafter Your workaround works because there is no M1 declared in class C.
Is there any workaround for calling a base interface implementation when the class need to override it need to obtain the base result first?
I continue to get StackOverflowEx of course ...

@gafter
Copy link
Member Author

gafter commented Sep 23, 2019

@raffaeler No, there isn't. We are working on a possible addition to C# in 9.0 that would permit it. See #2337. Until then you are out of luck unless you can modify the source to expose the functionality, perhaps as a static or sealed method.

@douglasg14b
Copy link

douglasg14b commented Sep 26, 2019

@orthoxerox It looks like they are allowed, but are unusable.

I can declare an internal or protected interface member with a default implementation, but the implementing class cannot access or override it.

Shouldn't that result in a compiler error if it can't be used?

@raffaeler
Copy link

While I miss the base() feature, if you use them for implementing traits, the current implementation is enough.

@douglasg14b
Copy link

douglasg14b commented Sep 26, 2019

@raffaeler I was tinkering around with it, and the restriction that properties can either have a public setter, or no setter is a bit of damper on traits. To be clear, the compiler allows for it, but the implementing class can't seem to implement the property.

Traits would be a godsend for something like DDD, but at the same time setters need to be private or protected, and the traits methods should be able to set public properties on the trait itself. But without a setter, the default method implementations cannot.

Overall, this is exciting, but there are some oddities between what the compiler allows and what actually functions. Which has led to a few disappointments.

@raffaeler
Copy link

@douglasg14b From various comments on this repo and on the Roslyn one I understand that this feature is just the beginning to open to traits topic (which is very wide).
Since version 6 the trend of the language spec has changed in favor of progressive adjustements.
This repo is the right place to propose a new feature or discussing an existing one. The one talking about base() looks appropriate for this specific problem.
In any case, traits are usable and IMO this is the best reason for default interface members to be available right now.

@gafter
Copy link
Member Author

gafter commented Oct 7, 2019

All of these individual issues are closed (decided by LDM). When it has been confirmed that they are reflected in the specification this github issue can be closed.

@gafter
Copy link
Member Author

gafter commented Nov 4, 2019

These have been reflected in the spec, with the exception of base(T) which has been pushed to C# 9.0.

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

No branches or pull requests