Repository navigation
[Version 9.0] Feature support for more partial methods - #1468
BillWagner wants to merge 15 commits into
Conversation
5e7986c to
9e0a0f2
Compare
9e0a0f2 to
13eca45
Compare
|
This is ready for a first look from the committee. I addressed all the comments from #991. However, #991 (comment) is an instance where the spec language is correct, and the roslyn implementation has a parsing error. See dotnet/roslyn#81947. Secondly, I made more substantial edits to the original PR. The draft retained a single clause for Partial methods. Looking ahead at the future versions (for the standard), I split this into three sections: General, restricted partial methods, and unrestricted partial methods. I think that makes the text easier to read, and provides the framework for future versions where there are more unrestricted partial methods. Discussion question: I used the terms in the current PR of "restricted" and "unrestricted". I'm not convinced those are the best terms. It might be better to use "optional partial methods", and "required partial methods", respectively. I'm not sold on those either. Other thoughts? |
df85388 to
5839c04
Compare
jskeet
left a comment
There was a problem hiding this comment.
Overall - either optional and required are the wrong way round here, or I think we should find another pair of terms. Sorry to throw a spanner in the works!
fbe81a5 to
c403cee
Compare
…review commits Delta: 9 commits (16fa8b4..c403cee) Original: 3563471 (alpha-v9) / 3eed3e3 (pr-1468) Conflicts resolved: 2 in classes.md - Conflict 1: Replaced old 'two kinds' paragraph with new 'classified as follows' intro - Conflict 2: Replaced old 'restricted partial method' restrictions with 'default arguments' bullet (restrictions moved to §optional-partial-methods)
c403cee to
84fff65
Compare
84fff65 to
db72bd2
Compare
|
An earlier version of this feature is already present on |
b637493 to
e116922
Compare
|
This is ready for a first review at the next committee meeting. @Nigel-Ecma I think the ANTLR tests are failing because this PR allows new partial declarations. |
@BillWagner – A quick glance suggests the two tests fail because the structure of existing syntax rules has changed, i.e. As far as my quick glance goes you’ve done this so that ref-methods can be partial as well. There is nothing wrong with this per se, it is a choice, but if you want to achieve the same result without breaking anything just add HTH |
Nigel-Ecma
left a comment
There was a problem hiding this comment.
Some suggested small changes, including a grammar rewrite previously suggested in this comment
| > *Example*: | ||
| > | ||
| > <!-- Example: {template:"standalone-lib-without-using", name:"PartialMethods2", replaceEllipsis:true, customEllipsisReplacements: ["", "return true;", "i = 10;"]} --> | ||
| > ```csharp | ||
| > // part containing defining partial method declarations | ||
| > partial class C | ||
| > { | ||
| > partial void M1(); // implementation optional | ||
| > private partial void M2(); // required, impl. required | ||
| > protected partial bool M3(); // required, impl. required | ||
| > public partial void M4(out int i); // required, impl. required | ||
| > } | ||
| > | ||
| > // part containing implementing partial method declarations | ||
| > partial class C | ||
| > { | ||
| > private partial void M2() { ... } | ||
| > protected partial bool M3() { ... } | ||
| > public partial void M4(out int i) { ... } | ||
| > } | ||
| > ``` | ||
| > | ||
| > *end example* |
There was a problem hiding this comment.
I suggest adding a M5 which uses extern, thus showing an implementing declaration with a semicolon rather than a block.
82d6e56 to
a28ffad
Compare
fix md formatting revert to previous text handle imp/exp private allow partial with ref_kind returns change link's target name Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Also, edit after moving text around. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Clean up a bit of language and flow. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Jon Skeet <[email protected]> Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Investigate and respond to feedback from reviews. Implement the proposed name change to "optional" and "required" Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Jon Skeet <[email protected]> Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
- Add that return types must match. - Add rule on type parameter constraints. - Add text for extern implementations. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Review and make two small updates. Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
Co-authored-by: Copilot <[email protected]> Copilot-Session: 21570b74-c6f5-4fc5-ab6d-7546918d6ddb
a28ffad to
d5d2748
Compare
Co-authored-by: Nigel-Ecma <[email protected]>
Address the remaining feedback on partial methods
This PR replaces #991
There are several unaddressed comments on #991: