Skip to content

Commit

Permalink
Merge pull request #65480 from CyrusNajmabadi/tryParsing
Browse files Browse the repository at this point in the history 
Move context-sensitive parsing to context-free parsing.
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi committed Nov 18, 2022
2 parents ed76a90 + dc8e601 commit ccf9a90
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 128 deletions.
156 changes: 74 additions & 82 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ internal partial class LanguageParser : SyntaxParser

private int _recursionDepth;
private TerminatorState _termState; // Resettable
private bool _isInTry; // Resettable
private bool _checkedTopLevelStatementsFeatureAvailability; // Resettable

// NOTE: If you add new state, you should probably add it to ResetPoint as well.
Expand Down Expand Up @@ -2824,45 +2823,6 @@ private bool IsNoneOrIncompleteMember(SyntaxKind parentKind, SyntaxList<Attribut
return false;
}

private static bool ContainsErrorDiagnostic(GreenNode node)
{
// ContainsDiagnostics returns true if this node (or any descendants) contain any sort of error. However,
// GetDiagnostics() only returns diagnostics at that node itself. So we have to explicitly walk down the
// tree to find out if the diagnostics are error or not.

// Quick check to avoid any unnecessary work.
if (node.ContainsDiagnostics)
{
var stack = ArrayBuilder<GreenNode>.GetInstance();
try
{
stack.Push(node);

while (stack.Count > 0)
{
var current = stack.Pop();
if (!current.ContainsDiagnostics)
continue;

foreach (var diagnostic in current.GetDiagnostics())
{
if (diagnostic.Severity == DiagnosticSeverity.Error)
return true;
}

foreach (var child in current.ChildNodesAndTokens())
stack.Push(child);
}
}
finally
{
stack.Free();
}
}

return false;
}

private bool ReconsideredTypeAsAsyncModifier(ref SyntaxListBuilder modifiers, ref TypeSyntax type, ref ResetPoint afterTypeResetPoint,
ref ExplicitInterfaceSpecifierSyntax explicitInterfaceOpt, ref SyntaxToken identifierOrThisOpt,
ref TypeParameterListSyntax typeParameterListOpt)
Expand Down Expand Up @@ -8840,10 +8800,6 @@ private bool IsPossibleStatement(bool acceptAccessibilityMods)
case SyntaxKind.IdentifierToken:
return IsTrueIdentifier();

case SyntaxKind.CatchKeyword:
case SyntaxKind.FinallyKeyword:
return !_isInTry;

// Accessibility modifiers are not legal in a statement,
// but a common mistake for local functions. Parse to give a
// better error message.
Expand Down Expand Up @@ -8938,74 +8894,76 @@ private ContinueStatementSyntax ParseContinueStatement(SyntaxList<AttributeListS

private TryStatementSyntax ParseTryStatement(SyntaxList<AttributeListSyntax> attributes)
{
var isInTry = _isInTry;
_isInTry = true;
Debug.Assert(this.CurrentToken.Kind is SyntaxKind.TryKeyword or SyntaxKind.CatchKeyword or SyntaxKind.FinallyKeyword);

// We are called into on try/catch/finally, so eating the try may actually fail.
var @try = this.EatToken(SyntaxKind.TryKeyword);

BlockSyntax block;
BlockSyntax tryBlock;
if (@try.IsMissing)
{
block = _syntaxFactory.Block(
attributeLists: default, this.EatToken(SyntaxKind.OpenBraceToken), default(SyntaxList<StatementSyntax>), this.EatToken(SyntaxKind.CloseBraceToken));
// If there was no actual `try`, then we got here because of a misplaced `catch`/`finally`. In that
// case just synthesize a fully missing try-block. We will already have issued a diagnostic on the
// `try` keyword, so we don't need to issue any more.

Debug.Assert(@try.ContainsDiagnostics);
Debug.Assert(this.CurrentToken.Kind is SyntaxKind.CatchKeyword or SyntaxKind.FinallyKeyword);

tryBlock = missingBlock();
}
else
{
var saveTerm = _termState;
_termState |= TerminatorState.IsEndOfTryBlock;
block = this.ParsePossiblyAttributedBlock();
tryBlock = this.ParsePossiblyAttributedBlock();
_termState = saveTerm;
}

var catches = default(SyntaxListBuilder<CatchClauseSyntax>);
FinallyClauseSyntax @finally = null;
SyntaxListBuilder<CatchClauseSyntax> catchClauses = default;
FinallyClauseSyntax finallyClause = null;
try
{
bool hasEnd = false;

if (this.CurrentToken.Kind == SyntaxKind.CatchKeyword)
{
hasEnd = true;
catches = _pool.Allocate<CatchClauseSyntax>();
catchClauses = _pool.Allocate<CatchClauseSyntax>();
while (this.CurrentToken.Kind == SyntaxKind.CatchKeyword)
{
catches.Add(this.ParseCatchClause());
catchClauses.Add(this.ParseCatchClause());
}
}

if (this.CurrentToken.Kind == SyntaxKind.FinallyKeyword)
{
hasEnd = true;
var fin = this.EatToken();
var finBlock = this.ParsePossiblyAttributedBlock();
@finally = _syntaxFactory.FinallyClause(fin, finBlock);
finallyClause = _syntaxFactory.FinallyClause(
this.EatToken(),
this.ParsePossiblyAttributedBlock());
}

if (!hasEnd)
if (catchClauses.IsNull && finallyClause == null)
{
block = this.AddErrorToLastToken(block, ErrorCode.ERR_ExpectedEndTry);
if (!ContainsErrorDiagnostic(tryBlock))
tryBlock = this.AddErrorToLastToken(tryBlock, ErrorCode.ERR_ExpectedEndTry);

// synthesize missing tokens for "finally { }":
@finally = _syntaxFactory.FinallyClause(
SyntaxToken.CreateMissing(SyntaxKind.FinallyKeyword, null, null),
_syntaxFactory.Block(
attributeLists: default,
SyntaxToken.CreateMissing(SyntaxKind.OpenBraceToken, null, null),
default(SyntaxList<StatementSyntax>),
SyntaxToken.CreateMissing(SyntaxKind.CloseBraceToken, null, null)));
finallyClause = _syntaxFactory.FinallyClause(
SyntaxFactory.MissingToken(SyntaxKind.FinallyKeyword),
missingBlock());
}

_isInTry = isInTry;

return _syntaxFactory.TryStatement(attributes, @try, block, catches, @finally);
return _syntaxFactory.TryStatement(attributes, @try, tryBlock, catchClauses, finallyClause);
}
finally
{
if (!catches.IsNull)
{
_pool.Free(catches);
}
if (!catchClauses.IsNull)
_pool.Free(catchClauses);
}

BlockSyntax missingBlock()
=> _syntaxFactory.Block(
attributeLists: default,
SyntaxFactory.MissingToken(SyntaxKind.OpenBraceToken),
statements: default,
SyntaxFactory.MissingToken(SyntaxKind.CloseBraceToken));
}

private bool IsEndOfTryBlock()
Expand Down Expand Up @@ -14046,15 +14004,13 @@ private void LeaveQuery()
return new ResetPoint(
base.GetResetPoint(),
_termState,
_isInTry,
_syntaxFactoryContext.IsInAsync,
_syntaxFactoryContext.QueryDepth);
}

private void Reset(ref ResetPoint state)
{
_termState = state.TerminatorState;
_isInTry = state.IsInTry;
_syntaxFactoryContext.IsInAsync = state.IsInAsync;
_syntaxFactoryContext.QueryDepth = state.QueryDepth;
base.Reset(ref state.BaseResetPoint);
Expand All @@ -14069,20 +14025,17 @@ private void Release(ref ResetPoint state)
{
internal SyntaxParser.ResetPoint BaseResetPoint;
internal readonly TerminatorState TerminatorState;
internal readonly bool IsInTry;
internal readonly bool IsInAsync;
internal readonly int QueryDepth;

internal ResetPoint(
SyntaxParser.ResetPoint resetPoint,
TerminatorState terminatorState,
bool isInTry,
bool isInAsync,
int queryDepth)
{
this.BaseResetPoint = resetPoint;
this.TerminatorState = terminatorState;
this.IsInTry = isInTry;
this.IsInAsync = isInAsync;
this.QueryDepth = queryDepth;
}
Expand All @@ -14104,5 +14057,44 @@ internal TNode ConsumeUnexpectedTokens<TNode>(TNode node) where TNode : CSharpSy
node = this.AddTrailingSkippedSyntax(node, trailingTrash.Node);
return node;
}

private static bool ContainsErrorDiagnostic(GreenNode node)
{
// ContainsDiagnostics returns true if this node (or any descendants) contain any sort of error. However,
// GetDiagnostics() only returns diagnostics at that node itself. So we have to explicitly walk down the
// tree to find out if the diagnostics are error or not.

// Quick check to avoid any unnecessary work.
if (node.ContainsDiagnostics)
{
var stack = ArrayBuilder<GreenNode>.GetInstance();
try
{
stack.Push(node);

while (stack.Count > 0)
{
var current = stack.Pop();
if (!current.ContainsDiagnostics)
continue;

foreach (var diagnostic in current.GetDiagnostics())
{
if (diagnostic.Severity == DiagnosticSeverity.Error)
return true;
}

foreach (var child in current.ChildNodesAndTokens())
stack.Push(child);
}
}
finally
{
stack.Free();
}
}

return false;
}
}
}
51 changes: 33 additions & 18 deletions src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4132,24 +4132,39 @@ public static int Main()
";
// Extra Errors
ParseAndValidate(test,
// (12,13): error CS1524: Expected catch or finally
// }
Diagnostic(ErrorCode.ERR_ExpectedEndTry, "}"),
// (11,21): error CS1031: Type expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_TypeExpected, "throw"),
// (11,21): error CS1026: ) expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_CloseParenExpected, "throw"),
// (11,21): error CS1002: ; expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_SemicolonExpected, "throw"),
// (11,80): error CS1002: ; expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_SemicolonExpected, ")"),
// (11,80): error CS1513: } expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_RbraceExpected, ")"));
// (11,21): error CS1031: Type expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_TypeExpected, "throw"),
// (11,21): error CS1026: ) expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_CloseParenExpected, "throw"),
// (11,21): error CS1002: ; expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_SemicolonExpected, "throw"),
// (11,80): error CS1002: ; expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_SemicolonExpected, ")"),
// (11,80): error CS1513: } expected
// sizeof (throw new RecoverableException("An exception has occurred"));
Diagnostic(ErrorCode.ERR_RbraceExpected, ")"));
}

[Fact]
public void ParseTryWithoutCatchesOrFinally()
{
var test = @"
public class mine
{
void M()
{
try { }
}
}
";
ParseAndValidate(test,
// (6,15): error CS1524: Expected catch or finally
// try { }
Diagnostic(ErrorCode.ERR_ExpectedEndTry, "}").WithLocation(6, 15));
}

[WorkItem(906299, "DevDiv/Personal")]
Expand Down
32 changes: 4 additions & 28 deletions src/Compilers/CSharp/Test/Syntax/Parsing/StatementAttributeParsingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2835,13 +2835,7 @@ void Goo()
Diagnostic(ErrorCode.ERR_ExpectedEndTry, "}").WithLocation(6, 15),
// (6,21): error CS1003: Syntax error, 'try' expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_SyntaxError, "finally").WithArguments("try").WithLocation(6, 21),
// (6,21): error CS1514: { expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_LbraceExpected, "finally").WithLocation(6, 21),
// (6,21): error CS1513: } expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_RbraceExpected, "finally").WithLocation(6, 21));
Diagnostic(ErrorCode.ERR_SyntaxError, "finally").WithArguments("try").WithLocation(6, 21));

N(SyntaxKind.CompilationUnit);
{
Expand Down Expand Up @@ -2931,13 +2925,7 @@ void Goo()
Diagnostic(ErrorCode.ERR_AttributesNotAllowed, "[A]").WithLocation(6, 17),
// (6,21): error CS1003: Syntax error, 'try' expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_SyntaxError, "finally").WithArguments("try").WithLocation(6, 21),
// (6,21): error CS1514: { expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_LbraceExpected, "finally").WithLocation(6, 21),
// (6,21): error CS1513: } expected
// try { } [A] finally { }
Diagnostic(ErrorCode.ERR_RbraceExpected, "finally").WithLocation(6, 21));
Diagnostic(ErrorCode.ERR_SyntaxError, "finally").WithArguments("try").WithLocation(6, 21));
}

[Fact]
Expand Down Expand Up @@ -3035,13 +3023,7 @@ void Goo()
Diagnostic(ErrorCode.ERR_ExpectedEndTry, "}").WithLocation(6, 15),
// (6,21): error CS1003: Syntax error, 'try' expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_SyntaxError, "catch").WithArguments("try").WithLocation(6, 21),
// (6,21): error CS1514: { expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_LbraceExpected, "catch").WithLocation(6, 21),
// (6,21): error CS1513: } expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_RbraceExpected, "catch").WithLocation(6, 21));
Diagnostic(ErrorCode.ERR_SyntaxError, "catch").WithArguments("try").WithLocation(6, 21));

N(SyntaxKind.CompilationUnit);
{
Expand Down Expand Up @@ -3131,13 +3113,7 @@ void Goo()
Diagnostic(ErrorCode.ERR_AttributesNotAllowed, "[A]").WithLocation(6, 17),
// (6,21): error CS1003: Syntax error, 'try' expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_SyntaxError, "catch").WithArguments("try").WithLocation(6, 21),
// (6,21): error CS1514: { expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_LbraceExpected, "catch").WithLocation(6, 21),
// (6,21): error CS1513: } expected
// try { } [A] catch { }
Diagnostic(ErrorCode.ERR_RbraceExpected, "catch").WithLocation(6, 21));
Diagnostic(ErrorCode.ERR_SyntaxError, "catch").WithArguments("try").WithLocation(6, 21));
}

[Fact]
Expand Down

0 comments on commit ccf9a90

Please sign in to comment.