[Parser] Consolidate recovery codepaths

We had two different codepaths for recovery, which could result in
cases where we attempt to recover, but `expect` is then not able to
do so. Consolidate them into a single function.

Author: hamishknight

Sources/SwiftParser/Recovery.swift

@@ -89,83 +89,34 @@ extension Parser.Lookahead {
     _ spec3: TokenSpec,
     recursionDepth: Int = 1
   ) -> RecoveryConsumptionHandle? {
-    if recursionDepth > 10 {
-      // `canRecoverTo` calls itself recursively if it finds a nested opening token, eg. when calling `canRecoverTo` on
-      // `{{{`. To avoid stack overflowing, limit the number of nested `canRecoverTo` calls we make. Since returning a
-      // recovery handle from this function only improves error recovery but is not necessary for correctness, bailing
-      // from recovery is safe.
-      // The value 10 was chosen fairly arbitrarily. It seems unlikely that we get useful recovery if we find more than
-      // 10 nested open and closing delimiters.
-      return nil
-    }
     #if SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION
     if shouldRecordAlternativeTokenChoices {
       recordAlternativeTokenChoice(for: self.currentToken, choices: [spec1, spec2, spec3])
     }
     #endif
-    let initialTokensConsumed = self.tokensConsumed
 
-    let recoveryPrecedence = min(spec1.recoveryPrecedence, spec2.recoveryPrecedence, spec3.recoveryPrecedence)
-    let shouldSkipOverNewlines =
-      recoveryPrecedence.shouldSkipOverNewlines && spec1.allowAtStartOfLine && spec2.allowAtStartOfLine
-      && spec3.allowAtStartOfLine
-
-    while !self.at(.endOfFile) {
-      if !shouldSkipOverNewlines, self.atStartOfLine {
-        break
-      }
-      let matchedSpec: TokenSpec?
-      switch self.currentToken {
-      case spec1:
-        matchedSpec = spec1
-      case spec2:
-        matchedSpec = spec2
-      case spec3:
-        matchedSpec = spec3
-      default:
-        matchedSpec = nil
-      }
-      if let matchedSpec {
-        return RecoveryConsumptionHandle(
-          unexpectedTokens: self.tokensConsumed - initialTokensConsumed,
-          tokenConsumptionHandle: TokenConsumptionHandle(spec: matchedSpec)
-        )
-      }
-      let currentTokenPrecedence = TokenPrecedence(self.currentToken)
-      if currentTokenPrecedence >= recoveryPrecedence {
-        break
-      }
-      if let closingDelimiter = currentTokenPrecedence.closingTokenKind {
-        let closingDelimiterSpec = TokenSpec(closingDelimiter)
-        let canCloseAtSameLine: Int? = self.withLookahead { lookahead in
-          var tokensToSkip = 0
-          while !lookahead.at(.endOfFile), !lookahead.currentToken.isAtStartOfLine {
-            tokensToSkip += 1
-            if lookahead.at(closingDelimiterSpec) {
-              return tokensToSkip
-            } else {
-              lookahead.consumeAnyToken()
-            }
+    let result = canRecoverToImpl(
+      recoveryPrecedence: min(spec1.recoveryPrecedence, spec2.recoveryPrecedence, spec3.recoveryPrecedence),
+      recursionDepth: recursionDepth,
+      matchesSpec: { lookahead -> (TokenSpec, _)? in
+        let match: TokenSpec? =
+          switch lookahead.currentToken {
+          case spec1:
+            spec1
+          case spec2:
+            spec2
+          case spec3:
+            spec3
+          default:
+            nil
           }
+        guard let match, match.allowAtStartOfLine || !lookahead.atStartOfLine else {
           return nil
         }
-        if let tokensToSkip = canCloseAtSameLine {
-          for _ in 0..<tokensToSkip {
-            self.consumeAnyToken()
-          }
-          continue
-        }
-        self.consumeAnyToken()
-        guard self.canRecoverTo(closingDelimiterSpec, recursionDepth: recursionDepth + 1) != nil else {
-          continue
-        }
-        self.eat(closingDelimiterSpec)
-      } else {
-        self.consumeAnyToken()
+        return (match, TokenConsumptionHandle(spec: match))
       }
-    }
-
-    return nil
+    )
+    return result?.handle
   }
 
   /// Checks if we can reach a token in `subset` by skipping tokens that have
@@ -182,7 +133,6 @@ extension Parser.Lookahead {
       recordAlternativeTokenChoice(for: self.currentToken, choices: specSet.allCases.map(\.spec))
     }
     #endif
-    let initialTokensConsumed = self.tokensConsumed
 
     if specSet.allCases.isEmpty {
       return nil
@@ -192,31 +142,80 @@ extension Parser.Lookahead {
       overrideRecoveryPrecedence ?? specSet.allCases.map({
         return $0.spec.recoveryPrecedence
       }).min()!
-    var loopProgress = LoopProgressCondition()
-    while !self.at(.endOfFile) && self.hasProgressed(&loopProgress) {
+
+    return self.canRecoverToImpl(
+      recoveryPrecedence: recoveryPrecedence,
+      recursionDepth: 1,
+      matchesSpec: { lookahead in
+        guard let (specSet, handle) = lookahead.at(anyIn: specSet),
+          specSet.spec.allowAtStartOfLine || !lookahead.atStartOfLine
+        else {
+          return nil
+        }
+        return (specSet, handle)
+      }
+    )
+  }
+
+  @inline(__always)
+  private mutating func canRecoverToImpl<Match>(
+    recoveryPrecedence: TokenPrecedence,
+    recursionDepth: Int,
+    matchesSpec: (Parser.Lookahead) -> (Match, TokenConsumptionHandle)?
+  ) -> (match: Match, handle: RecoveryConsumptionHandle)? {
+    if recursionDepth > 10 {
+      // `canRecoverToImpl` calls itself recursively if it finds a nested opening token, eg. when calling `canRecoverTo` on
+      // `{{{`. To avoid stack overflowing, limit the number of nested `canRecoverTo` calls we make. Since returning a
+      // recovery handle from this function only improves error recovery but is not necessary for correctness, bailing
+      // from recovery is safe.
+      // The value 10 was chosen fairly arbitrarily. It seems unlikely that we get useful recovery if we find more than
+      // 10 nested open and closing delimiters.
+      return nil
+    }
+    let initialTokensConsumed = self.tokensConsumed
+
+    while !self.at(.endOfFile) {
       if !recoveryPrecedence.shouldSkipOverNewlines, self.atStartOfLine {
         break
       }
-      if let (kind, handle) = self.at(anyIn: specSet) {
-        return (
-          kind,
-          RecoveryConsumptionHandle(
-            unexpectedTokens: self.tokensConsumed - initialTokensConsumed,
-            tokenConsumptionHandle: handle
-          )
+      if let (matchedSpec, tokenHandle) = matchesSpec(self) {
+        let handle = RecoveryConsumptionHandle(
+          unexpectedTokens: self.tokensConsumed - initialTokensConsumed,
+          tokenConsumptionHandle: tokenHandle
         )
+        return (matchedSpec, handle)
       }
       let currentTokenPrecedence = TokenPrecedence(self.currentToken)
       if currentTokenPrecedence >= recoveryPrecedence {
         break
       }
-      self.consumeAnyToken()
       if let closingDelimiter = currentTokenPrecedence.closingTokenKind {
         let closingDelimiterSpec = TokenSpec(closingDelimiter)
-        guard self.canRecoverTo(closingDelimiterSpec) != nil else {
-          break
+        let canCloseAtSameLine: Int? = self.withLookahead { lookahead in
+          var tokensToSkip = 0
+          while !lookahead.at(.endOfFile), !lookahead.currentToken.isAtStartOfLine {
+            tokensToSkip += 1
+            if lookahead.at(closingDelimiterSpec) {
+              return tokensToSkip
+            } else {
+              lookahead.consumeAnyToken()
+            }
+          }
+          return nil
+        }
+        if let tokensToSkip = canCloseAtSameLine {
+          for _ in 0..<tokensToSkip {
+            self.consumeAnyToken()
+          }
+          continue
+        }
+        self.consumeAnyToken()
+        guard self.canRecoverTo(closingDelimiterSpec, recursionDepth: recursionDepth + 1) != nil else {
+          continue
         }
         self.eat(closingDelimiterSpec)
+      } else {
+        self.consumeAnyToken()
       }
     }
 

Tests/SwiftParserTest/DeclarationTests.swift

@@ -617,6 +617,23 @@ final class DeclarationTests: ParserTestCase {
         private(set) var get, didSet var a = 0
         """
     )
+
+    assertParse(
+      """
+      public 1️⃣{ {} }
+      open
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected declaration and ';' after 'public' modifier",
+          fixIts: ["insert declaration and ';'"]
+        )
+      ],
+      fixedSource: """
+        public <#declaration#>; { {} }
+        open
+        """
+    )
   }
 
   func testTypealias() {