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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: features/PartialEventsCtors
Choose a base branch
from
+1,497 −211
Open

Parse partial events and constructors #76860

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba8ca9d
Parse partial events and constructors
jjonescz Jan 22, 2025
941b824
Update pre-existing tests
jjonescz Jan 23, 2025
ced9913
Explain why partial ctors are sometimes disallowed
jjonescz Jan 23, 2025
504f58d
Parse `partial` constructors unconditionally
jjonescz Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 24 additions & 22 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

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?

{
// Standard legal cases.
modTok = ConvertToKeyword(this.EatToken());
Expand Down Expand Up @@ -1632,26 +1632,28 @@ private bool IsPartialType()
return false;
}

private bool IsPartialMember()
private bool IsPartialMember(bool allowPartialCtor)
Copy link
Member

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.

Copy link
Member Author

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;

{
// note(cyrusn): this could have been written like so:
//
// return
// this.CurrentToken.ContextualKind == SyntaxKind.PartialKeyword &&
// this.PeekToken(1).Kind == SyntaxKind.VoidKeyword;
//
// However, we want to be lenient and allow the user to write
// 'partial' in most modifier lists. We will then provide them with
// a more specific message later in binding that they are doing
// something wrong.
//
// Some might argue that the simple check would suffice.
// However, we'd like to maintain behavior with
// previously shipped versions, and so we're keeping this code.
Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.PartialKeyword);

// Check for:
// partial event
if (this.PeekToken(1).Kind == SyntaxKind.EventKeyword)
{
return true;
}

// Here we check for:
// Check for constructor:
// partial Identifier(
if (allowPartialCtor &&
this.PeekToken(1).Kind == SyntaxKind.IdentifierToken &&
this.PeekToken(2).Kind == SyntaxKind.OpenParenToken)
{
return true;
}
Copy link
Member

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?


// Check for method/property:
// partial ReturnType MemberName
Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.PartialKeyword);
using var _ = this.GetDisposableResetPoint(resetOnDispose: true);

this.EatToken(); // partial
Expand Down Expand Up @@ -5680,7 +5682,7 @@ private bool IsTrueIdentifier()
{
if (this.CurrentToken.Kind == SyntaxKind.IdentifierToken)
{
if (!IsCurrentTokenPartialKeywordOfPartialMethodOrType() &&
if (!IsCurrentTokenPartialKeywordOfPartialMemberOrType() &&
!IsCurrentTokenQueryKeywordInQuery() &&
!IsCurrentTokenWhereOfConstraintClause())
{
Expand Down Expand Up @@ -5727,7 +5729,7 @@ private SyntaxToken ParseIdentifierToken(ErrorCode code = ErrorCode.ERR_Identifi
// show the correct parameter help in this case. So, when we see "partial" we check if it's being used
// as an identifier or as a contextual keyword. If it's the latter then we bail out. See
// Bug: vswhidbey/542125
if (IsCurrentTokenPartialKeywordOfPartialMethodOrType() || IsCurrentTokenQueryKeywordInQuery())
if (IsCurrentTokenPartialKeywordOfPartialMemberOrType() || IsCurrentTokenQueryKeywordInQuery())
{
var result = CreateMissingIdentifierToken();
result = this.AddError(result, ErrorCode.ERR_InvalidExprTerm, this.CurrentToken.Text);
Expand All @@ -5754,11 +5756,11 @@ private bool IsCurrentTokenQueryKeywordInQuery()
return this.IsInQuery && this.IsCurrentTokenQueryContextualKeyword;
}

private bool IsCurrentTokenPartialKeywordOfPartialMethodOrType()
private bool IsCurrentTokenPartialKeywordOfPartialMemberOrType()
{
if (this.CurrentToken.ContextualKind == SyntaxKind.PartialKeyword)
{
if (this.IsPartialType() || this.IsPartialMember())
if (this.IsPartialType() || this.IsPartialMember(allowPartialCtor: false))
Copy link
Member

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.

{
return true;
}
Expand Down
30 changes: 30 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10588,5 +10588,35 @@ void M()
// [A(p)] void F() { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 21));
}

[Fact]
public void ReturningPartialType_InMethod()
{
var source = """
class @partial
{
static void Main()
{
System.Console.Write(F().GetType().Name);

partial F() => new();
}
}
""";
CompileAndVerify(source, expectedOutput: "partial").VerifyDiagnostics();
}

[Fact]
public void ReturningPartialType_TopLevel()
{
var source = """
System.Console.Write(F().GetType().Name);

partial F() => new();

class @partial;
""";
CompileAndVerify(source, expectedOutput: "partial").VerifyDiagnostics();
}
}
}
13 changes: 2 additions & 11 deletions src/Compilers/CSharp/Test/Syntax/Parsing/AsyncParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1567,9 +1567,6 @@ class C
{
async partial event
",
// (4,19): error CS1519: Invalid token 'event' in class, record, struct, or interface member declaration
// async partial event
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "event").WithArguments("event").WithLocation(4, 19),
// (4,24): error CS1031: Type expected
// async partial event
Diagnostic(ErrorCode.ERR_TypeExpected, "").WithLocation(4, 24),
Expand All @@ -1589,16 +1586,10 @@ async partial event
N(SyntaxKind.ClassKeyword);
N(SyntaxKind.IdentifierToken, "C");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.IncompleteMember);
{
N(SyntaxKind.AsyncKeyword);
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.IdentifierToken, "partial");
}
}
N(SyntaxKind.EventDeclaration);
{
N(SyntaxKind.AsyncKeyword);
N(SyntaxKind.PartialKeyword);
N(SyntaxKind.EventKeyword);
M(SyntaxKind.IdentifierName);
{
Expand Down
Loading
Loading