From b5626d4c102623083fb793fb26240e84f6ddae5b Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 21 Nov 2022 12:23:01 -0800 Subject: [PATCH] Improve parser recovery around invalid function definitions. (#2525) * Improve parser recovery around invalid function definitions. * Fix. * Clean up. * Clean up. Co-authored-by: Yong He --- source/slang/slang-parser.cpp | 82 +++++++++++++------ .../recovery-unknown-func-modifier.slang | 12 +++ ...y-unknown-func-modifier.slang.expected.txt | 12 +++ 3 files changed, 81 insertions(+), 25 deletions(-) create mode 100644 tests/language-server/recovery-unknown-func-modifier.slang create mode 100644 tests/language-server/recovery-unknown-func-modifier.slang.expected.txt diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp index b513217c80..ab849a98b1 100644 --- a/source/slang/slang-parser.cpp +++ b/source/slang/slang-parser.cpp @@ -161,7 +161,10 @@ namespace Slang //Session* getSession() { return m_session; } Token ReadToken(); + Token readTokenImpl(TokenType type, bool forceSkippingToClosingToken); Token ReadToken(TokenType type); + // Same as `ReadToken`, but force skip to the matching closing token on error. + Token ReadMatchingToken(TokenType type); Token ReadToken(const char* string); bool LookAheadToken(TokenType type); @@ -551,7 +554,7 @@ namespace Slang return TryRecover(parser, recoverBefore, 1, recoverAfter, 1); } - Token Parser::ReadToken(TokenType expected) + Token Parser::readTokenImpl(TokenType expected, bool forceSkippingToClosingToken) { if (tokenReader.peekTokenType() == expected) { @@ -563,33 +566,51 @@ namespace Slang if (!isRecovering) { Unexpected(this, expected); - return tokenReader.peekToken(); - } - else - { - // Try to find a place to recover - if (TryRecoverBefore(this, expected)) - { - isRecovering = false; - return tokenReader.advanceToken(); - } - // This could be dangerous: if `ReadToken()` is being called - // in a loop we may never make forward progress, so we use - // a counter to limit the maximum amount of times we are allowed - // to peek the same token. If the outter parsing logic is - // correct, we will pop back to the right level. If there are - // erroneous parsing logic, this counter is to prevent us - // looping infinitely. - static const int kMaxTokenPeekCount = 64; - sameTokenPeekedTimes++; - if (sameTokenPeekedTimes < kMaxTokenPeekCount) + if (!forceSkippingToClosingToken) return tokenReader.peekToken(); - else + switch (expected) { - sameTokenPeekedTimes = 0; - return tokenReader.advanceToken(); + case TokenType::RBrace: + case TokenType::RParent: + case TokenType::RBracket: + break; + default: + return tokenReader.peekToken(); } } + + // Try to find a place to recover + if (TryRecoverBefore(this, expected)) + { + isRecovering = false; + return tokenReader.advanceToken(); + } + // This could be dangerous: if `ReadToken()` is being called + // in a loop we may never make forward progress, so we use + // a counter to limit the maximum amount of times we are allowed + // to peek the same token. If the outter parsing logic is + // correct, we will pop back to the right level. If there are + // erroneous parsing logic, this counter is to prevent us + // looping infinitely. + static const int kMaxTokenPeekCount = 64; + sameTokenPeekedTimes++; + if (sameTokenPeekedTimes < kMaxTokenPeekCount) + return tokenReader.peekToken(); + else + { + sameTokenPeekedTimes = 0; + return tokenReader.advanceToken(); + } + } + + Token Parser::ReadToken(TokenType expected) + { + return readTokenImpl(expected, false); + } + + Token Parser::ReadMatchingToken(TokenType expected) + { + return readTokenImpl(expected, true); } bool Parser::LookAheadToken(const char* string, int offset) @@ -1677,7 +1698,7 @@ namespace Slang // parser->ReadToken(TokenType::LParent); declarator = parseDeclarator(parser, options); - parser->ReadToken(TokenType::RParent); + parser->ReadMatchingToken(TokenType::RParent); } break; @@ -3834,6 +3855,17 @@ namespace Slang } break; + case TokenType::LBrace: + case TokenType::LParent: + { + // We shouldn't be seeing an LBrace or an LParent when expecting a decl. + // However recovery logic may lead us here. In this case we just + // skip the whole `{}` block and return an empty decl. + SkipBalancedToken(&parser->tokenReader); + decl = parser->astBuilder->create(); + decl->loc = loc; + } + break; // If nothing else matched, we try to parse an "ordinary" declarator-based declaration default: decl = ParseDeclaratorDecl(parser, containerDecl, modifiers); diff --git a/tests/language-server/recovery-unknown-func-modifier.slang b/tests/language-server/recovery-unknown-func-modifier.slang new file mode 100644 index 0000000000..b558f85909 --- /dev/null +++ b/tests/language-server/recovery-unknown-func-modifier.slang @@ -0,0 +1,12 @@ +// Test that the parser can recover after unknown function modifier. +//TEST:LANG_SERVER: +UNKNOWN_MODIFIER UNKNOWN_TYPE apparentFunc(int x, int y) +{ + return 0; +} + +//HOVER:9,10 +void nextFunc() +{ + syntax_error(); +} \ No newline at end of file diff --git a/tests/language-server/recovery-unknown-func-modifier.slang.expected.txt b/tests/language-server/recovery-unknown-func-modifier.slang.expected.txt new file mode 100644 index 0000000000..5739b0fee8 --- /dev/null +++ b/tests/language-server/recovery-unknown-func-modifier.slang.expected.txt @@ -0,0 +1,12 @@ +-------- +range: 8,5 - 8,13 +content: +``` +func nextFunc() -> void +``` + +HOVER:9,10 + +{REDACTED}.slang(9) + +