-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Parse partial events and constructors #76860
base: features/PartialEventsCtors
Are you sure you want to change the base?
Parse partial events and constructors #76860
Conversation
91ccc2a
to
ba8ca9d
Compare
Be sure to include the new feature in https://github.com/dotnet/roslyn/blob/main/docs/Language%20Feature%20Status.md |
@@ -1379,7 +1379,7 @@ private void ParseModifiers(SyntaxListBuilder tokens, bool forAccessors, bool fo | |||
{ | |||
case DeclarationModifiers.Partial: | |||
var nextToken = PeekToken(1); | |||
if (this.IsPartialType() || this.IsPartialMember()) | |||
if (this.IsPartialType() || this.IsPartialMember(allowPartialCtor: !forTopLevelStatements)) |
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.
document. why is this gated on that?
@@ -1632,26 +1632,28 @@ private bool IsPartialType() | |||
return false; | |||
} | |||
|
|||
private bool IsPartialMember() | |||
private bool IsPartialMember(bool allowPartialCtor) |
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.
my preference is to not have ambient state. that makes parsing (esp. incremental parsing) more difficult. Is it possible to just answer this uniformly regardless on ambient state? It might mean parsing something out which needs a binding error later, but that's fine.
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 don't like that either, but I need to avoid breaks in cases like this:
System.Console.Write(F().GetType().Name);
partial F() => new(); // don't want to parse this as a constructor in top-level statements
class @partial;
this.PeekToken(2).Kind == SyntaxKind.OpenParenToken) | ||
{ | ||
return true; | ||
} |
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.
my preference is to always return true here. Why would we not want that?
{ | ||
if (this.CurrentToken.ContextualKind == SyntaxKind.PartialKeyword) | ||
{ | ||
if (this.IsPartialType() || this.IsPartialMember()) | ||
if (this.IsPartialType() || this.IsPartialMember(allowPartialCtor: false)) |
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.
doc why partial cosntructors are not allowd 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.
Done with pass.
We should bring to ldm if we can just make partial a keyword in the next version. Like we've done for many other cases at this point. There's a simple workaround (using @partial). |
Sounds good to me. Added the question: dotnet/csharplang#9085 |
Test plan: #76859