Skip to content

[SwiftIDEUtils] Fix infinite loop in NameMatcher for whitespace positions in string segments#3348

Open
inju2403 wants to merge 1 commit into
swiftlang:mainfrom
inju2403:inju2403/fix-name-matcher-string-segment-infinite-loop
Open

[SwiftIDEUtils] Fix infinite loop in NameMatcher for whitespace positions in string segments#3348
inju2403 wants to merge 1 commit into
swiftlang:mainfrom
inju2403:inju2403/fix-name-matcher-string-segment-infinite-loop

Conversation

@inju2403

Copy link
Copy Markdown

Summary

Fixes an infinite loop in NameMatcher.visit(_:TokenSyntax) when a position in positionsToResolve lands on whitespace inside a .stringSegment token.

Background

#2708 (528257f) fixed an infinite recursion in the same function for positions landing in leading/trailing trivia. The .stringSegment branch of the same function uses the same while let first(where:) pattern but was not covered by that fix.

Root cause

In the .stringSegment branch:

while let baseNamePosition = positionsToResolve.first(where: { token.rangeWithoutTrivia.contains($0) }) {
  let positionOffsetInStringSegment = baseNamePosition.utf8Offset - token.position.utf8Offset
  guard let tokenLength = getFirstTokenLength(in: token.syntaxTextBytes[positionOffsetInStringSegment...]) else {
    continue
  }
  addResolvedLocIfRequested(...)
}

addResolvedLocIfRequested is the only path that removes a position from positionsToResolve. When getFirstTokenLength returns nil — which happens when the slice begins with whitespace, because firstToken.leadingTriviaLength is non-zero — the loop hits continue without removing the position. first(where:) then re-picks the same position on every subsequent iteration, causing an infinite loop.

Fix

Iterate a snapshot of the matching positions, so that a position for which getFirstTokenLength returns nil is simply skipped to the next one:

let positionsInToken = positionsToResolve.filter { token.rangeWithoutTrivia.contains($0) }
for baseNamePosition in positionsInToken {
  ...
}

addResolvedLocIfRequested still mutates positionsToResolve as before, so subsequent tokens see the same set as in the previous behavior — only the within-token iteration changes.

Test

Adds testStringLiteralWithPositionOnWhitespace, which hangs indefinitely against main and passes in <5ms with this change:

"hello1️⃣ world"

(The 1️⃣ marker resolves to a position that lands on the whitespace inside the string segment.)

Notes

I noticed this while reviewing the NameMatcher after #2708 and checking whether the same antipattern existed elsewhere in the same function. I don't have a real-world reproducer beyond the test, but the failure mode, trigger condition, and fix shape are all mechanically identical to #2708, in the same file and same function.

…ions in string segments

The `while let baseNamePosition = positionsToResolve.first(where:)` loop in the `.stringSegment` branch of `visit(_:TokenSyntax)` only mutated `positionsToResolve` via `addResolvedLocIfRequested`. When `getFirstTokenLength` returned `nil` (the position landed on whitespace inside the string segment, so the parser saw leading trivia) the loop hit `continue` without removing the position, and `first(where:)` found the same position again on every subsequent iteration -- an infinite loop.

Iterate a snapshot of the matching positions instead, so a position for which `getFirstTokenLength` returns `nil` is simply skipped to the next one.

This is the same class of bug as the one fixed in 528257f for the trivia loop earlier in the same function.

Adds a regression test (`testStringLiteralWithPositionOnWhitespace`) that hangs against `main` and passes in <5ms with this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant