From e32f3b646ee95dd1c8c1ef60e9f7ae64103d0f09 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 21:43:17 -0800 Subject: [PATCH 01/14] Extract out complex part of statement parsing into its own helper function. --- .../CSharp/Portable/Parser/LanguageParser.cs | 85 +++++++++++-------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index efbab3eab8e5c..0943425cbda4b 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6377,6 +6377,8 @@ private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() /// private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) { + StatementSyntax result; + switch (this.CurrentToken.Kind) { case SyntaxKind.FixedKeyword: @@ -6416,11 +6418,9 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) case SyntaxKind.ThrowKeyword: return this.ParseThrowStatement(); case SyntaxKind.UnsafeKeyword: - // Checking for brace to disambiguate between unsafe statement and unsafe local function - if (this.IsPossibleUnsafeStatement()) - { - return this.ParseUnsafeStatement(); - } + result = TryParseStatementStartingWithUnsafe(); + if (result != null) + return result; break; case SyntaxKind.UsingKeyword: return PeekToken(1).Kind == SyntaxKind.OpenParenToken ? this.ParseUsingStatement() : this.ParseLocalDeclarationStatement(); @@ -6431,37 +6431,9 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) case SyntaxKind.SemicolonToken: return _syntaxFactory.EmptyStatement(this.EatToken()); case SyntaxKind.IdentifierToken: - if (isPossibleAwaitForEach()) - { - return this.ParseForEachStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams)); - } - else if (isPossibleAwaitUsing()) - { - if (PeekToken(2).Kind == SyntaxKind.OpenParenToken) - { - return this.ParseUsingStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncUsing)); - } - else - { - return this.ParseLocalDeclarationStatement(parseAwaitKeyword(MessageID.None)); - } - } - else if (this.IsPossibleLabeledStatement()) - { - return this.ParseLabeledStatement(); - } - else if (this.IsPossibleYieldStatement()) - { - return this.ParseYieldStatement(); - } - else if (this.IsPossibleAwaitExpressionStatement()) - { - return this.ParseExpressionStatement(); - } - else if (this.IsQueryExpression(mayBeVariableDeclaration: true, mayBeMemberDeclaration: allowAnyExpression)) - { - return this.ParseExpressionStatement(this.ParseQueryExpression(0)); - } + result = TryParseStatementStartingWithIdentifier(allowAnyExpression); + if (result != null) + return result; break; } @@ -6473,6 +6445,43 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) { return this.ParseExpressionStatement(); } + } + + private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExpression) + { + if (isPossibleAwaitForEach()) + { + return this.ParseForEachStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams)); + } + else if (isPossibleAwaitUsing()) + { + if (PeekToken(2).Kind == SyntaxKind.OpenParenToken) + { + return this.ParseUsingStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncUsing)); + } + else + { + return this.ParseLocalDeclarationStatement(parseAwaitKeyword(MessageID.None)); + } + } + else if (this.IsPossibleLabeledStatement()) + { + return this.ParseLabeledStatement(); + } + else if (this.IsPossibleYieldStatement()) + { + return this.ParseYieldStatement(); + } + else if (this.IsPossibleAwaitExpressionStatement()) + { + return this.ParseExpressionStatement(); + } + else if (this.IsQueryExpression(mayBeVariableDeclaration: true, mayBeMemberDeclaration: allowAnyExpression)) + { + return this.ParseExpressionStatement(this.ParseQueryExpression(0)); + } + + return null; bool isPossibleAwaitForEach() { @@ -6494,6 +6503,10 @@ SyntaxToken parseAwaitKeyword(MessageID feature) } } + private UnsafeStatementSyntax TryParseStatementStartingWithUnsafe() + // Checking for brace to disambiguate between unsafe statement and unsafe local function + => this.IsPossibleUnsafeStatement() ? this.ParseUnsafeStatement() : null; + private bool IsPossibleLabeledStatement() { return this.PeekToken(1).Kind == SyntaxKind.ColonToken && this.IsTrueIdentifier(); From 9c7c4cbd6d6cf26ac34032ace7baef8696d06755 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:15:55 -0800 Subject: [PATCH 02/14] Simplify using-statements/local-decls --- .../CSharp/Portable/Parser/LanguageParser.cs | 77 +++++++++++++------ 1 file changed, 52 insertions(+), 25 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 0943425cbda4b..3fadd398e3be2 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6423,7 +6423,7 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) return result; break; case SyntaxKind.UsingKeyword: - return PeekToken(1).Kind == SyntaxKind.OpenParenToken ? this.ParseUsingStatement() : this.ParseLocalDeclarationStatement(); + return ParseStatementStartingWithUsing(); case SyntaxKind.WhileKeyword: return this.ParseWhileStatement(); case SyntaxKind.OpenBraceToken: @@ -6451,17 +6451,14 @@ private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExp { if (isPossibleAwaitForEach()) { - return this.ParseForEachStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams)); + return this.ParseForEachStatement(ParseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams)); } - else if (isPossibleAwaitUsing()) + else if (IsPossibleAwaitUsing()) { if (PeekToken(2).Kind == SyntaxKind.OpenParenToken) { - return this.ParseUsingStatement(parseAwaitKeyword(MessageID.IDS_FeatureAsyncUsing)); - } - else - { - return this.ParseLocalDeclarationStatement(parseAwaitKeyword(MessageID.None)); + // `await using Type ...` is handled later when ParseLocalDeclarationStatement is called. + return this.ParseUsingStatement(ParseAwaitKeyword(MessageID.IDS_FeatureAsyncUsing)); } } else if (this.IsPossibleLabeledStatement()) @@ -6488,25 +6485,25 @@ bool isPossibleAwaitForEach() return this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && this.PeekToken(1).Kind == SyntaxKind.ForEachKeyword; } - - bool isPossibleAwaitUsing() - { - return this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && - this.PeekToken(1).Kind == SyntaxKind.UsingKeyword; - } - - SyntaxToken parseAwaitKeyword(MessageID feature) - { - Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword); - SyntaxToken awaitToken = this.EatContextualToken(SyntaxKind.AwaitKeyword); - return feature != MessageID.None ? CheckFeatureAvailability(awaitToken, feature) : awaitToken; - } } + private StatementSyntax ParseStatementStartingWithUsing() + => PeekToken(1).Kind == SyntaxKind.OpenParenToken ? ParseUsingStatement() : ParseLocalDeclarationStatement(); + private UnsafeStatementSyntax TryParseStatementStartingWithUnsafe() // Checking for brace to disambiguate between unsafe statement and unsafe local function => this.IsPossibleUnsafeStatement() ? this.ParseUnsafeStatement() : null; + private SyntaxToken ParseAwaitKeyword(MessageID feature) + { + Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword); + SyntaxToken awaitToken = this.EatContextualToken(SyntaxKind.AwaitKeyword); + return feature != MessageID.None ? CheckFeatureAvailability(awaitToken, feature) : awaitToken; + } + + private bool IsPossibleAwaitUsing() + => this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && this.PeekToken(1).Kind == SyntaxKind.UsingKeyword; + private bool IsPossibleLabeledStatement() { return this.PeekToken(1).Kind == SyntaxKind.ColonToken && this.IsTrueIdentifier(); @@ -6538,6 +6535,19 @@ private bool IsPossibleLocalDeclarationStatement(bool allowAnyExpression) return true; } + // note: `using (` and `await using (` are already handled in ParseStatementCore. + if (tk == SyntaxKind.UsingKeyword) + { + Debug.Assert(PeekToken(1).Kind != SyntaxKind.OpenParenToken); + return true; + } + + if (IsPossibleAwaitUsing()) + { + Debug.Assert(PeekToken(2).Kind != SyntaxKind.OpenParenToken); + return true; + } + tk = this.CurrentToken.ContextualKind; if (IsAdditionalLocalFunctionModifier(tk) && (tk != SyntaxKind.AsyncKeyword || ShouldAsyncBeTreatedAsModifier(parsingStatementNotDeclaration: true))) @@ -8095,14 +8105,31 @@ private LabeledStatementSyntax ParseLabeledStatement() /// /// Parses any kind of local declaration statement: local variable or local function. /// - private StatementSyntax ParseLocalDeclarationStatement(SyntaxToken awaitKeywordOpt = default) + private StatementSyntax ParseLocalDeclarationStatement() { - var usingKeyword = TryEatToken(SyntaxKind.UsingKeyword); + SyntaxToken awaitKeyword, usingKeyword; + bool canParseAsLocalFunction = false; + if (IsPossibleAwaitUsing()) + { + awaitKeyword = ParseAwaitKeyword(MessageID.None); + usingKeyword = EatToken(); + } + else if (this.CurrentToken.Kind == SyntaxKind.UsingKeyword) + { + awaitKeyword = default; + usingKeyword = EatToken(); + } + else + { + awaitKeyword = default; + usingKeyword = null; + canParseAsLocalFunction = true; + } + if (usingKeyword != null) { usingKeyword = CheckFeatureAvailability(usingKeyword, MessageID.IDS_FeatureUsingDeclarations); } - bool canParseAsLocalFunction = usingKeyword == null; var mods = _pool.Allocate(); this.ParseDeclarationModifiers(mods); @@ -8144,7 +8171,7 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxToken awaitKeywordO } var semicolon = this.EatToken(SyntaxKind.SemicolonToken); return _syntaxFactory.LocalDeclarationStatement( - awaitKeywordOpt, + awaitKeyword, usingKeyword, mods.ToList(), _syntaxFactory.VariableDeclaration(type, variables), From 3c9027484fc8afcede053d6db2f038265b84e8e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:17:27 -0800 Subject: [PATCH 03/14] Inline method --- src/Compilers/CSharp/Portable/Parser/LanguageParser.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 3fadd398e3be2..10823e2bdc3a4 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6449,7 +6449,8 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExpression) { - if (isPossibleAwaitForEach()) + if (this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && + this.PeekToken(1).Kind == SyntaxKind.ForEachKeyword) { return this.ParseForEachStatement(ParseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams)); } @@ -6479,12 +6480,6 @@ private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExp } return null; - - bool isPossibleAwaitForEach() - { - return this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && - this.PeekToken(1).Kind == SyntaxKind.ForEachKeyword; - } } private StatementSyntax ParseStatementStartingWithUsing() From 03ea8480b402400b4eb9ae2869d8c0ef7a1e5982 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 21:54:23 -0800 Subject: [PATCH 04/14] Rename variable for clarity --- .../CSharp/Portable/Parser/LanguageParser.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 10823e2bdc3a4..fb89d70d594f6 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -2095,8 +2095,9 @@ private MemberDeclarationSyntax ParseMemberDeclarationOrStatementCore(SyntaxKind var saveTerm = _termState; _termState |= TerminatorState.IsPossibleStatementStartOrStop; // partial statements can abort if a new statement starts - // Any expression is allowed, not just expression statements: - var statement = this.ParseStatementNoDeclaration(allowAnyExpression: true); + // We don't allow local declaration statements or functions at the top level. We want + // to fall out below and parse them instead as fields or methods. + var statement = this.ParseStatementNoDeclaration(isGlobalScriptLevel: true); _termState = saveTerm; if (statement != null) @@ -6291,7 +6292,7 @@ private StatementSyntax ParseStatementCore() // expression then we only allow legal expression statements. (That is, "new C();", // "C();", "x = y;" and so on.) - StatementSyntax result = ParseStatementNoDeclaration(allowAnyExpression: false); + StatementSyntax result = ParseStatementNoDeclaration(isGlobalScriptLevel: false); if (result != null) { return result; @@ -6358,7 +6359,7 @@ private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() this.Reset(ref resetPointBeforeStatement); IsInAsync = true; - result = ParseStatementNoDeclaration(allowAnyExpression: false); + result = ParseStatementNoDeclaration(isGlobalScriptLevel: false); IsInAsync = false; return result; @@ -6375,7 +6376,11 @@ private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() /// /// Variable declarations in global code are parsed as field declarations so we need to fallback if we encounter a declaration statement. /// - private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) + /// If we're being called while parsing a C# script from + /// the top-level. At the top level, we allow most statements *except* for + /// local-decls/local-funcs. Those will instead be parsed out as + /// script-fields/methods. + private StatementSyntax ParseStatementNoDeclaration(bool isGlobalScriptLevel) { StatementSyntax result; @@ -6431,13 +6436,13 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) case SyntaxKind.SemicolonToken: return _syntaxFactory.EmptyStatement(this.EatToken()); case SyntaxKind.IdentifierToken: - result = TryParseStatementStartingWithIdentifier(allowAnyExpression); + result = TryParseStatementStartingWithIdentifier(isGlobalScriptLevel); if (result != null) return result; break; } - if (this.IsPossibleLocalDeclarationStatement(allowAnyExpression)) + if (this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) { return null; } @@ -6447,7 +6452,7 @@ private StatementSyntax ParseStatementNoDeclaration(bool allowAnyExpression) } } - private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExpression) + private StatementSyntax TryParseStatementStartingWithIdentifier(bool isGlobalScriptLevel) { if (this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && this.PeekToken(1).Kind == SyntaxKind.ForEachKeyword) @@ -6474,7 +6479,7 @@ private StatementSyntax TryParseStatementStartingWithIdentifier(bool allowAnyExp { return this.ParseExpressionStatement(); } - else if (this.IsQueryExpression(mayBeVariableDeclaration: true, mayBeMemberDeclaration: allowAnyExpression)) + else if (this.IsQueryExpression(mayBeVariableDeclaration: true, mayBeMemberDeclaration: isGlobalScriptLevel)) { return this.ParseExpressionStatement(this.ParseQueryExpression(0)); } @@ -6514,7 +6519,7 @@ private bool IsPossibleYieldStatement() return this.CurrentToken.ContextualKind == SyntaxKind.YieldKeyword && (this.PeekToken(1).Kind == SyntaxKind.ReturnKeyword || this.PeekToken(1).Kind == SyntaxKind.BreakKeyword); } - private bool IsPossibleLocalDeclarationStatement(bool allowAnyExpression) + private bool IsPossibleLocalDeclarationStatement(bool isGlobalScriptLevel) { // This method decides whether to parse a statement as a // declaration or as an expression statement. In the old @@ -6625,7 +6630,7 @@ private bool IsPossibleLocalDeclarationStatement(bool allowAnyExpression) } // T? and T* might start an expression, we need to parse further to disambiguate: - if (allowAnyExpression) + if (isGlobalScriptLevel) { if (st == ScanTypeFlags.PointerOrMultiplication) { From 46afc83358b6166f123982075d7e98076d86aab1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 21:57:50 -0800 Subject: [PATCH 05/14] Inline method --- .../CSharp/Portable/Parser/LanguageParser.cs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index fb89d70d594f6..4773a5015599f 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6278,6 +6278,7 @@ public StatementSyntax ParseStatement() private StatementSyntax ParseStatementCore() { + ResetPoint resetPointBeforeStatement = this.GetResetPoint(); try { _recursionDepth++; @@ -6302,19 +6303,6 @@ private StatementSyntax ParseStatementCore() // it as either a declaration or as an "await X();" statement that is in a non-async // method. - return TryParsePossibleDeclarationOrBadAwaitStatement(); - } - finally - { - _recursionDepth--; - } - } - - private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() - { - ResetPoint resetPointBeforeStatement = this.GetResetPoint(); - try - { // Precondition: We have already attempted to parse the statement as a non-declaration and failed. // // That means that we are in one of the following cases: @@ -6334,7 +6322,7 @@ private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() // semantic code will error out that this isn't legal. bool beginsWithAwait = this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword; - StatementSyntax result = ParseLocalDeclarationStatement(); + result = ParseLocalDeclarationStatement(); // Case (1) if (result == null) @@ -6366,6 +6354,7 @@ private StatementSyntax TryParsePossibleDeclarationOrBadAwaitStatement() } finally { + _recursionDepth--; this.Release(ref resetPointBeforeStatement); } } From a7a59b27c90371abe3f5003fb98e746b9fc3d10d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:03:05 -0800 Subject: [PATCH 06/14] Directly call into more precise parse function --- src/Compilers/CSharp/Portable/Parser/LanguageParser.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 4773a5015599f..0cf7e7c5a1ab9 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6344,10 +6344,15 @@ private StatementSyntax ParseStatementCore() Debug.Assert(!IsInAsync); // Let's see if we're in case (5). Pretend that we're in an async method and retry. + // + // Because we know we started with 'await' and because we're attempting to parse out + // an await-expression, we can call directly into ParseExpressionStatement here + // instead of calling into another top-level ParseStatement function that will + // eventually bottom out there. this.Reset(ref resetPointBeforeStatement); IsInAsync = true; - result = ParseStatementNoDeclaration(isGlobalScriptLevel: false); + result = ParseExpressionStatement(); IsInAsync = false; return result; From 52adb4e999a81c8ad43de095544e5dbeaa0a2e58 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:08:29 -0800 Subject: [PATCH 07/14] Have all statement parsing call through the same top level helper --- .../CSharp/Portable/Parser/LanguageParser.cs | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 0cf7e7c5a1ab9..4f7f925a63a12 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -2097,7 +2097,7 @@ private MemberDeclarationSyntax ParseMemberDeclarationOrStatementCore(SyntaxKind // We don't allow local declaration statements or functions at the top level. We want // to fall out below and parse them instead as fields or methods. - var statement = this.ParseStatementNoDeclaration(isGlobalScriptLevel: true); + var statement = this.ParseStatementCore(isGlobalScriptLevel: true); _termState = saveTerm; if (statement != null) @@ -6272,11 +6272,15 @@ private TypeSyntax ParsePointerTypeMods(TypeSyntax type) public StatementSyntax ParseStatement() { return ParseWithStackGuard( - () => ParseStatementCore() ?? ParseExpressionStatement(), + () => ParseStatementCore(isGlobalScriptLevel: false) ?? ParseExpressionStatement(), () => SyntaxFactory.EmptyStatement(SyntaxFactory.MissingToken(SyntaxKind.SemicolonToken))); } - private StatementSyntax ParseStatementCore() + /// If we're being called while parsing a C# script from + /// the top-level. At the top level, we allow most statements *except* for + /// local-decls/local-funcs. Those will instead be parsed out as + /// script-fields/methods. + private StatementSyntax ParseStatementCore(bool isGlobalScriptLevel) { ResetPoint resetPointBeforeStatement = this.GetResetPoint(); try @@ -6284,7 +6288,11 @@ private StatementSyntax ParseStatementCore() _recursionDepth++; StackGuard.EnsureSufficientExecutionStack(_recursionDepth); - if (this.IsIncrementalAndFactoryContextMatches && this.CurrentNode is CSharp.Syntax.StatementSyntax) + // Can only reuse a global-script or regular statement if we're in the same + // global-script context we were originally in. + if (this.IsIncrementalAndFactoryContextMatches && + this.CurrentNode is CSharp.Syntax.StatementSyntax && + isGlobalScriptLevel == (this.CurrentNode.Parent is Syntax.GlobalStatementSyntax)) { return (StatementSyntax)this.EatNode(); } @@ -6299,6 +6307,14 @@ private StatementSyntax ParseStatementCore() return result; } + if (isGlobalScriptLevel) + { + // if we're at the global script level, then we don't support local-decls or + // local-funcs. The caller instead will look for those and parse them as + // fields/methods in the global script scope. + return null; + } + // We could not successfully parse the statement as a non-declaration. Try to parse // it as either a declaration or as an "await X();" statement that is in a non-async // method. @@ -7041,7 +7057,7 @@ private void ParseStatements(ref CSharpSyntaxNode previousNode, SyntaxListBuilde { if (this.IsPossibleStatement(acceptAccessibilityMods: true)) { - var statement = this.ParseStatementCore(); + var statement = this.ParseStatementCore(isGlobalScriptLevel: false); if (statement != null) { statements.Add(statement); @@ -7170,7 +7186,7 @@ private StatementSyntax ParseEmbeddedStatement() // deep impact on the number of recursive calls we can make (more than a hundred during // empirical testing). - return parseEmbeddedStatementRest(this.ParseStatementCore()); + return parseEmbeddedStatementRest(this.ParseStatementCore(isGlobalScriptLevel: false)); StatementSyntax parseEmbeddedStatementRest(StatementSyntax statement) { @@ -8092,7 +8108,7 @@ private LabeledStatementSyntax ParseLabeledStatement() var label = this.ParseIdentifierToken(); var colon = this.EatToken(SyntaxKind.ColonToken); Debug.Assert(!colon.IsMissing); - var statement = this.ParseStatementCore() ?? SyntaxFactory.EmptyStatement(EatToken(SyntaxKind.SemicolonToken)); + var statement = this.ParseStatementCore(isGlobalScriptLevel: false) ?? SyntaxFactory.EmptyStatement(EatToken(SyntaxKind.SemicolonToken)); return _syntaxFactory.LabeledStatement(label, colon, statement); } From b6ccf5e018bd3e277b9a20c507b7af4806abd72e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:20:04 -0800 Subject: [PATCH 08/14] Restructure code for clarity --- .../CSharp/Portable/Parser/LanguageParser.cs | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 4f7f925a63a12..d2fa3689b4e33 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6347,30 +6347,22 @@ this.CurrentNode is CSharp.Syntax.StatementSyntax && return null; } - // Cases (2), (3) and (4): - if (!beginsWithAwait || !result.ContainsDiagnostics) + if (result.ContainsDiagnostics && + beginsWithAwait && + !IsInAsync) { - return result; + // Local decl had issues. We were also starting with 'await' in a non-async + // context. Retry parsing this as if we were in an 'async' context as it's much + // more likely that this was a misplace await-expr' than a local decl. + // + // The user will still get a later binding error about an await-expr in a non-async + // context. + this.Reset(ref resetPointBeforeStatement); + IsInAsync = true; + result = ParseExpressionStatement(); + IsInAsync = false; } - // The statement begins with "await" and could not be parsed as a legal declaration statement. - // We know from our precondition that it is not a legal "await X();" statement, though it is - // possible that it was only not legal because we were not in an async context. - - Debug.Assert(!IsInAsync); - - // Let's see if we're in case (5). Pretend that we're in an async method and retry. - // - // Because we know we started with 'await' and because we're attempting to parse out - // an await-expression, we can call directly into ParseExpressionStatement here - // instead of calling into another top-level ParseStatement function that will - // eventually bottom out there. - - this.Reset(ref resetPointBeforeStatement); - IsInAsync = true; - result = ParseExpressionStatement(); - IsInAsync = false; - return result; } finally From f7e930e1cfaecdb28bdc243c1c756a1dfa7916ce Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:21:06 -0800 Subject: [PATCH 09/14] Invert conditional --- src/Compilers/CSharp/Portable/Parser/LanguageParser.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index d2fa3689b4e33..e247c5e84c116 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6444,14 +6444,12 @@ private StatementSyntax ParseStatementNoDeclaration(bool isGlobalScriptLevel) break; } - if (this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) - { - return null; - } - else + if (!this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) { return this.ParseExpressionStatement(); } + + return null; } private StatementSyntax TryParseStatementStartingWithIdentifier(bool isGlobalScriptLevel) From dc4ff3cbbd1f62f14137befe0aa555969b1414c3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:23:33 -0800 Subject: [PATCH 10/14] Inline method --- .../CSharp/Portable/Parser/LanguageParser.cs | 146 +++++++----------- 1 file changed, 60 insertions(+), 86 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index e247c5e84c116..026942ed56fb9 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6297,14 +6297,68 @@ this.CurrentNode is CSharp.Syntax.StatementSyntax && return (StatementSyntax)this.EatNode(); } - // First, try to parse as a non-declaration statement. If the statement is a single - // expression then we only allow legal expression statements. (That is, "new C();", - // "C();", "x = y;" and so on.) + StatementSyntax result; - StatementSyntax result = ParseStatementNoDeclaration(isGlobalScriptLevel: false); - if (result != null) + // Main switch to handle processing almost any statement. + switch (this.CurrentToken.Kind) { - return result; + case SyntaxKind.FixedKeyword: + return this.ParseFixedStatement(); + case SyntaxKind.BreakKeyword: + return this.ParseBreakStatement(); + case SyntaxKind.ContinueKeyword: + return this.ParseContinueStatement(); + case SyntaxKind.TryKeyword: + case SyntaxKind.CatchKeyword: + case SyntaxKind.FinallyKeyword: + return this.ParseTryStatement(); + case SyntaxKind.CheckedKeyword: + case SyntaxKind.UncheckedKeyword: + return this.ParseCheckedStatement(); + case SyntaxKind.DoKeyword: + return this.ParseDoStatement(); + case SyntaxKind.ForKeyword: + return this.ParseForOrForEachStatement(); + case SyntaxKind.ForEachKeyword: + return this.ParseForEachStatement(awaitTokenOpt: default); + case SyntaxKind.GotoKeyword: + return this.ParseGotoStatement(); + case SyntaxKind.IfKeyword: + return this.ParseIfStatement(); + case SyntaxKind.ElseKeyword: + // Including 'else' keyword to handle 'else without if' error cases + return this.ParseMisplacedElse(); + case SyntaxKind.LockKeyword: + return this.ParseLockStatement(); + case SyntaxKind.ReturnKeyword: + return this.ParseReturnStatement(); + case SyntaxKind.SwitchKeyword: + return this.ParseSwitchStatement(); + case SyntaxKind.ThrowKeyword: + return this.ParseThrowStatement(); + case SyntaxKind.UnsafeKeyword: + result = TryParseStatementStartingWithUnsafe(); + if (result != null) + return result; + break; + case SyntaxKind.UsingKeyword: + return ParseStatementStartingWithUsing(); + case SyntaxKind.WhileKeyword: + return this.ParseWhileStatement(); + case SyntaxKind.OpenBraceToken: + return this.ParseBlock(); + case SyntaxKind.SemicolonToken: + return _syntaxFactory.EmptyStatement(this.EatToken()); + case SyntaxKind.IdentifierToken: + result = TryParseStatementStartingWithIdentifier(isGlobalScriptLevel); + if (result != null) + return result; + break; + } + + if (!this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) + { + return this.ParseExpressionStatement(); } if (isGlobalScriptLevel) @@ -6372,86 +6426,6 @@ this.CurrentNode is CSharp.Syntax.StatementSyntax && } } - /// - /// Parses any statement but a declaration statement. Returns null if the lookahead looks like a declaration. - /// - /// - /// Variable declarations in global code are parsed as field declarations so we need to fallback if we encounter a declaration statement. - /// - /// If we're being called while parsing a C# script from - /// the top-level. At the top level, we allow most statements *except* for - /// local-decls/local-funcs. Those will instead be parsed out as - /// script-fields/methods. - private StatementSyntax ParseStatementNoDeclaration(bool isGlobalScriptLevel) - { - StatementSyntax result; - - switch (this.CurrentToken.Kind) - { - case SyntaxKind.FixedKeyword: - return this.ParseFixedStatement(); - case SyntaxKind.BreakKeyword: - return this.ParseBreakStatement(); - case SyntaxKind.ContinueKeyword: - return this.ParseContinueStatement(); - case SyntaxKind.TryKeyword: - case SyntaxKind.CatchKeyword: - case SyntaxKind.FinallyKeyword: - return this.ParseTryStatement(); - case SyntaxKind.CheckedKeyword: - case SyntaxKind.UncheckedKeyword: - return this.ParseCheckedStatement(); - case SyntaxKind.ConstKeyword: - return null; - case SyntaxKind.DoKeyword: - return this.ParseDoStatement(); - case SyntaxKind.ForKeyword: - return this.ParseForOrForEachStatement(); - case SyntaxKind.ForEachKeyword: - return this.ParseForEachStatement(awaitTokenOpt: default); - case SyntaxKind.GotoKeyword: - return this.ParseGotoStatement(); - case SyntaxKind.IfKeyword: - return this.ParseIfStatement(); - case SyntaxKind.ElseKeyword: - // Including 'else' keyword to handle 'else without if' error cases - return this.ParseMisplacedElse(); - case SyntaxKind.LockKeyword: - return this.ParseLockStatement(); - case SyntaxKind.ReturnKeyword: - return this.ParseReturnStatement(); - case SyntaxKind.SwitchKeyword: - return this.ParseSwitchStatement(); - case SyntaxKind.ThrowKeyword: - return this.ParseThrowStatement(); - case SyntaxKind.UnsafeKeyword: - result = TryParseStatementStartingWithUnsafe(); - if (result != null) - return result; - break; - case SyntaxKind.UsingKeyword: - return ParseStatementStartingWithUsing(); - case SyntaxKind.WhileKeyword: - return this.ParseWhileStatement(); - case SyntaxKind.OpenBraceToken: - return this.ParseBlock(); - case SyntaxKind.SemicolonToken: - return _syntaxFactory.EmptyStatement(this.EatToken()); - case SyntaxKind.IdentifierToken: - result = TryParseStatementStartingWithIdentifier(isGlobalScriptLevel); - if (result != null) - return result; - break; - } - - if (!this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) - { - return this.ParseExpressionStatement(); - } - - return null; - } - private StatementSyntax TryParseStatementStartingWithIdentifier(bool isGlobalScriptLevel) { if (this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && From f5ee1a61ca65d9dd3c33288f0659e249dfe05925 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:25:07 -0800 Subject: [PATCH 11/14] Re-extract remainder of method --- .../CSharp/Portable/Parser/LanguageParser.cs | 127 +++++++++--------- 1 file changed, 66 insertions(+), 61 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 026942ed56fb9..95149726763c3 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6356,74 +6356,79 @@ this.CurrentNode is CSharp.Syntax.StatementSyntax && break; } - if (!this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) - { - return this.ParseExpressionStatement(); - } - - if (isGlobalScriptLevel) - { - // if we're at the global script level, then we don't support local-decls or - // local-funcs. The caller instead will look for those and parse them as - // fields/methods in the global script scope. - return null; - } + return ParseStatementCoreRest(isGlobalScriptLevel, ref resetPointBeforeStatement); + } + finally + { + _recursionDepth--; + this.Release(ref resetPointBeforeStatement); + } + } - // We could not successfully parse the statement as a non-declaration. Try to parse - // it as either a declaration or as an "await X();" statement that is in a non-async - // method. + private StatementSyntax ParseStatementCoreRest(bool isGlobalScriptLevel, ref ResetPoint resetPointBeforeStatement) + { + if (!this.IsPossibleLocalDeclarationStatement(isGlobalScriptLevel)) + { + return this.ParseExpressionStatement(); + } - // Precondition: We have already attempted to parse the statement as a non-declaration and failed. - // - // That means that we are in one of the following cases: - // - // 1) This is not a statement. This can happen if the start of the statement was an - // accessibility modifier, but the rest of the statement did not parse as a local - // function. If there was an accessibility modifier and the statement parsed as - // local function, that should be marked as a mistake with local function visibility. - // Otherwise, it's likely the user just forgot a closing brace on their method. - // 2) This is a perfectly mundane and correct local declaration statement like "int x;" - // 3) This is a perfectly mundane but erroneous local declaration statement, like "int X();" - // 4) We are in the rare case of the code containing "await x;" and the intention is that - // "await" is the type of "x". This only works in a non-async method. - // 5) We have a misplaced await statement in a non-async method, like "await X();", - // so the parse failed. Had we been in an async method then the parse attempt - // done by our caller would have succeeded. Retry as if we were async. Later - // semantic code will error out that this isn't legal. - - bool beginsWithAwait = this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword; - result = ParseLocalDeclarationStatement(); - - // Case (1) - if (result == null) - { - this.Reset(ref resetPointBeforeStatement); - return null; - } + if (isGlobalScriptLevel) + { + // if we're at the global script level, then we don't support local-decls or + // local-funcs. The caller instead will look for those and parse them as + // fields/methods in the global script scope. + return null; + } - if (result.ContainsDiagnostics && - beginsWithAwait && - !IsInAsync) - { - // Local decl had issues. We were also starting with 'await' in a non-async - // context. Retry parsing this as if we were in an 'async' context as it's much - // more likely that this was a misplace await-expr' than a local decl. - // - // The user will still get a later binding error about an await-expr in a non-async - // context. - this.Reset(ref resetPointBeforeStatement); - IsInAsync = true; - result = ParseExpressionStatement(); - IsInAsync = false; - } + // We could not successfully parse the statement as a non-declaration. Try to parse + // it as either a declaration or as an "await X();" statement that is in a non-async + // method. - return result; + // Precondition: We have already attempted to parse the statement as a non-declaration and failed. + // + // That means that we are in one of the following cases: + // + // 1) This is not a statement. This can happen if the start of the statement was an + // accessibility modifier, but the rest of the statement did not parse as a local + // function. If there was an accessibility modifier and the statement parsed as + // local function, that should be marked as a mistake with local function visibility. + // Otherwise, it's likely the user just forgot a closing brace on their method. + // 2) This is a perfectly mundane and correct local declaration statement like "int x;" + // 3) This is a perfectly mundane but erroneous local declaration statement, like "int X();" + // 4) We are in the rare case of the code containing "await x;" and the intention is that + // "await" is the type of "x". This only works in a non-async method. + // 5) We have a misplaced await statement in a non-async method, like "await X();", + // so the parse failed. Had we been in an async method then the parse attempt + // done by our caller would have succeeded. Retry as if we were async. Later + // semantic code will error out that this isn't legal. + + bool beginsWithAwait = this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword; + var result = ParseLocalDeclarationStatement(); + + // Case (1) + if (result == null) + { + this.Reset(ref resetPointBeforeStatement); + return null; } - finally + + if (result.ContainsDiagnostics && + beginsWithAwait && + !IsInAsync) { - _recursionDepth--; - this.Release(ref resetPointBeforeStatement); + // Local decl had issues. We were also starting with 'await' in a non-async + // context. Retry parsing this as if we were in an 'async' context as it's much + // more likely that this was a misplace await-expr' than a local decl. + // + // The user will still get a later binding error about an await-expr in a non-async + // context. + this.Reset(ref resetPointBeforeStatement); + IsInAsync = true; + result = ParseExpressionStatement(); + IsInAsync = false; } + + return result; } private StatementSyntax TryParseStatementStartingWithIdentifier(bool isGlobalScriptLevel) From cfb694499107dfe89921f53fbfbbce90cba0dc3a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:27:49 -0800 Subject: [PATCH 12/14] Simplify comments --- .../CSharp/Portable/Parser/LanguageParser.cs | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 95149726763c3..84c452ae6a459 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6380,34 +6380,14 @@ private StatementSyntax ParseStatementCoreRest(bool isGlobalScriptLevel, ref Res return null; } - // We could not successfully parse the statement as a non-declaration. Try to parse - // it as either a declaration or as an "await X();" statement that is in a non-async - // method. - - // Precondition: We have already attempted to parse the statement as a non-declaration and failed. - // - // That means that we are in one of the following cases: - // - // 1) This is not a statement. This can happen if the start of the statement was an - // accessibility modifier, but the rest of the statement did not parse as a local - // function. If there was an accessibility modifier and the statement parsed as - // local function, that should be marked as a mistake with local function visibility. - // Otherwise, it's likely the user just forgot a closing brace on their method. - // 2) This is a perfectly mundane and correct local declaration statement like "int x;" - // 3) This is a perfectly mundane but erroneous local declaration statement, like "int X();" - // 4) We are in the rare case of the code containing "await x;" and the intention is that - // "await" is the type of "x". This only works in a non-async method. - // 5) We have a misplaced await statement in a non-async method, like "await X();", - // so the parse failed. Had we been in an async method then the parse attempt - // done by our caller would have succeeded. Retry as if we were async. Later - // semantic code will error out that this isn't legal. - bool beginsWithAwait = this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword; var result = ParseLocalDeclarationStatement(); - // Case (1) if (result == null) { + // didn't get any sort of statement. This was something else entirely (like just a + // `}`). No need to retry anything here. Just reset back to where we started from + // and bail entirely from parsing a statement. this.Reset(ref resetPointBeforeStatement); return null; } From f3afdd038e2d67d3f6b4b4f68cd8155072e6aa6d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:29:38 -0800 Subject: [PATCH 13/14] Extract local function --- .../CSharp/Portable/Parser/LanguageParser.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 84c452ae6a459..8691db93aa2d5 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -6288,11 +6288,7 @@ private StatementSyntax ParseStatementCore(bool isGlobalScriptLevel) _recursionDepth++; StackGuard.EnsureSufficientExecutionStack(_recursionDepth); - // Can only reuse a global-script or regular statement if we're in the same - // global-script context we were originally in. - if (this.IsIncrementalAndFactoryContextMatches && - this.CurrentNode is CSharp.Syntax.StatementSyntax && - isGlobalScriptLevel == (this.CurrentNode.Parent is Syntax.GlobalStatementSyntax)) + if (canReuseStatement(isGlobalScriptLevel)) { return (StatementSyntax)this.EatNode(); } @@ -6363,6 +6359,15 @@ this.CurrentNode is CSharp.Syntax.StatementSyntax && _recursionDepth--; this.Release(ref resetPointBeforeStatement); } + + bool canReuseStatement(bool isGlobalScriptLevel) + { + // Can only reuse a global-script or regular statement if we're in the same + // global-script context we were originally in. + return this.IsIncrementalAndFactoryContextMatches && + this.CurrentNode is CSharp.Syntax.StatementSyntax && + isGlobalScriptLevel == (this.CurrentNode.Parent is Syntax.GlobalStatementSyntax); + } } private StatementSyntax ParseStatementCoreRest(bool isGlobalScriptLevel, ref ResetPoint resetPointBeforeStatement) From 3955ffff4ce373a287ae4710ea773116e435f2ae Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 16 Feb 2020 22:34:11 -0800 Subject: [PATCH 14/14] Increase test parsing depth --- .../CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs index fc4dcd32473c9..deaa02cfbbd86 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs @@ -6495,7 +6495,7 @@ static void Main(string[] args) { "); - const int depth = 10000; + const int depth = 100000; for (int i = 0; i < depth; i++) { var line = string.Format("Action a{0} = delegate d{0} {{", i);