Repository navigation
fix(semantic tokens): fix delta underflow after multiline tokens - #513
Conversation
📝 WalkthroughWalkthroughSemantic token emission now uses a shared absolute-position encoder for single-line and multiline tokens. Tests widen position tracking to 64-bit, tolerate unmappable lines, and cover tokens following raw strings and multiline comments. ChangesSemantic token positions
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/feature/semantic_tokens.cpp (1)
540-562: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDefensively guard against out-of-order tokens.
Although the token stream is sorted and merged before reaching the encoder, adding a guard here prevents any theoretical upstream sorting bug from underflowing the LSP deltas. An underflow here would corrupt the entire file's subsequent syntax highlighting in the editor.
🛡️ Proposed defensive guard
SymbolKind kind, std::uint32_t modifiers) { if(token_length == 0) { return; } + + if(line < last_line || (line == last_line && character < last_start_character)) { + return; + } auto delta_line = line - last_line; auto delta_start = delta_line == 0 ? character - last_start_character : character;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/feature/semantic_tokens.cpp` around lines 540 - 562, Update the semantic-token encoder’s emit method to detect tokens that precede the previously emitted position before subtracting line or character values, and skip or otherwise safely handle those out-of-order tokens without appending invalid deltas or updating last_line/last_start_character. Preserve the existing zero-length filtering and normal ordered-token encoding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/feature/semantic_tokens.cpp`:
- Around line 540-562: Update the semantic-token encoder’s emit method to detect
tokens that precede the previously emitted position before subtracting line or
character values, and skip or otherwise safely handle those out-of-order tokens
without appending invalid deltas or updating last_line/last_start_character.
Preserve the existing zero-length filtering and normal ordered-token encoding
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c278d9a-d631-4bea-a6b1-7196df65a817
📒 Files selected for processing (2)
src/feature/semantic_tokens.cpptests/unit/feature/semantic_tokens_tests.cpp
Problem
When a line contains tokens after a multiline token ends on it (code following the closing quote of a raw string literal, or after a
*/of a multiline block comment), the semantic-tokens delta encoding is corrupted.SemanticTokenEncoder::appendsplits a multiline token into per-line entries whose last piece starts at column 0 of the end line, butlast_start_characterwas unconditionally set to the token's start column on its FIRST line. The next token'sdeltaStart = char - last_start_characterthen either underflows u32 (runtime-measuredchar=4294967292, destroying highlighting for the whole line) or silently shifts columns when it stays positive. A secondary flaw:last_lineadvanced to the token's end line even when nothing was emitted there (token ending exactly at a newline).Fix
Delta computation and prev-position bookkeeping fold into a single
emit(line, character, ...)taking absolute positions — the only reader/writer oflast_line/last_start_character, updated only when an entry is actually emitted. The split loop tracks each piece's absolute position; piece lengths and the splitting itself are unchanged.Testing
CodeAfterRawStringandCodeAfterMultilineCommentdecode the delta stream and assert exact absolute columns; the test decoders accumulate in 64-bit so an underflow can never wrap back into plausible values (which had masked this bug). Before the fix the raw-string case decoded a token start beyond the line end and the comment case landed at columns 6/10 instead of 10/14; both now decode exactly.last_lineupdate when nothing is emitted on the end line.