-
Notifications
You must be signed in to change notification settings - Fork 93
Adding support for default interface function members #681
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
Adding support for default interface function members #681
Conversation
|
Depending how PR #680 is resolved, some tweaks might be needed with this PR. As at 2025-05-03, that PR is still pending, and is tied to the review of this PR. |
84a1703 to
caba5c7
Compare
caba5c7 to
dc8d7a0
Compare
|
rebased on the latest draft-v8 on 09-23-2023 |
|
Should the references to "C# implementations targeting the CLI" be changed somehow because the ECMA standard CLI does not support default interface members? Possible answers include:
|
|
Re @KalleOlaviNiemitalo's concern about the Ecma CLI standard not supporting default interface function members, we previously made a related change to the draft-v8 Introduction, which in V7, said:
It now says:
I think that allows us to say nothing more for this feature with regard to the CLI. |
|
I don't recall why this is assigned to me. Is it for general review, or to answer the question about
In short, the |
Going from memory, but I believe as a general reviewer. |
|
closed and reopened to get all tests to run |
|
@RexJaeschke and @BillWagner could you look at resolving the conflict? I'm not sure whether we can realistically expect most members to have reviewed this before the meeting later today though. (I'll see whether I can find time, but I'm prioritizing #606.) |
22549fb to
efb69e5
Compare
@jskeet I did a full rebase. My main goal in prioritizing this for today's meeting is to get it assigned to someone for next meeting. I agree that there isn't time to get it reviewed thoroughly. |
|
@BillWagner Looking again, did we get as far as assigning it during the last meeting? We spent quite a bit of time talking about prose vs grammar for constraints, but we didn't record the next step... |
Respond to edits except for the rabbit hole opened by https://github.com/dotnet/csharpstandard/pull/681/files#r2386725345
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated for most review comments.
I did some investigation for https://github.com/dotnet/csharpstandard/pull/681/files#r2386725345
My notes are the last comment in that thread.
Update rules for interface member lookup and mapping (or implementing) an interface member. Add C# 10 note For our future selves.
bf3c327 to
acfdffe
Compare
|
The changes in the latest commit address the concepts in #681 (comment) The significant changes in the last commit:
|
| This clause augments the description of nested types in classes [§15.3.9](classes.md#1539-nested-types) for nested types declared in interfaces. | ||
| It is an error to declare a class type, struct type, or enum type within the scope of a type parameter that was declared with a *variance_annotation* ([§19.2.3.1](interfaces.md#19231-general)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may require changing the wording in §7.7.1 (Scopes / General). In the following, the outer T is hidden by the inner T.
interface IOuter<out T>
{
interface IInner<T>
{
class C { }
}
}§7.7.1 currently says:
The scope of a name is the region of program text within which it is possible to refer to the entity declared by the name without qualification of the name. Scopes can be nested, and an inner scope may redeclare the meaning of a name from an outer scope. (This does not, however, remove the restriction imposed by §7.3 that within a nested block it is not possible to declare a local variable or local constant with the same name as a local variable or local constant in an enclosing block.) The name from the outer scope is then said to be hidden in the region of program text covered by the inner scope, and access to the outer name is only possible by qualifying the name.
Within the definition of interface IInner<T> , it is not possible to refer to the T type parameter of IOuter. According to this wording then, the definition of class C is not in the scope of the outer T.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, this might be solved by adding "including any nested scopes" to the restriction here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed the changes as best I can, one commit at a time... but it's getting really tricky. (Things I've noticed in one commit have been fixed in a later one.)
I think we should aim to merge the PR as-is by the end of the upcoming meeting, with comments for things that still need resolving - it'll make it much easier to review. The downside is it'll be harder to see "the totality of changes for this feature" of course.
|
I agree with @jskeet:
@BillWagner – could you please tag the draft-v8 branch before & after you merge this so we can check out the two commits locally & diff them to see "the totality of changes for this feature"? Of course we can do such diffs anyway, the tags just save locating the commit UUIDs. |
Co-authored-by: Jon Skeet <[email protected]>
Co-authored-by: Nigel-Ecma <[email protected]>
Agreed to merge in 10/22 meeting.
We merged dotnet/csharpstandard#681 at the October meeting.
We merged dotnet/csharpstandard#681 at the October meeting.
Re the MS proposal: The main challenge when reading it was the (misleading) occurrence of the modifier
overridein numerous examples and narrative. As best as I can tell, this modifier is not actually permitted on any interface member. Instead, overriding is achieved via explicit implementation.There is one open question, which has to do with the topic "Base Interface Invocations," for which the decision "Decided on
base(N.I1<T>).M(s)" was made. However, as far as I can tell, support for this was not added to the V8 compiler. Can someone please confirm (or refute) that.BTW, I think the proposal is misnamed: it really applies to all interface function members (properties, indexers, and events as well), not just methods. (it also allows nested types.)
Note: No spec changes are needed to support entry point
Mainin an interface. Any non-generic type containing aMainmethod withvoid/intreturn and no/onestring[]parameter list is already permitted, which not only allowsMainin a struct or class, but now also in an interface.As of V7, the descriptions of the various member kinds has been located in classes.md, with augmenting and/or overriding text in structs.md, with pointers from structs.md back into classes.md, and that continues. However, now that many members kinds can have implementations in interfaces as well, I've added an intro para to the start of most member sections in classes.md, which contains forward pointers to that member kind's occurrence in structs.md and interfaces.md, as appropriate.
Fixes #982 (which reinstates
abstract override.)