From 88160496e17954a21eaff4d99088a610ef7d80ec Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 6 Jul 2026 07:39:50 -0700 Subject: [PATCH] Fix which tokens the lexer flags as bracket-recovery tokens. (#7457) `ErrorRecoveryBuffer::Apply` flagged each inserted token via the pre-insertion index of the token it was inserted before. Once any earlier insertion had been applied, that index no longer matches the token's position in the merged list: with two closers inserted at the same point (`{((}`), the second recovery token was left unflagged, and with insertions at two separate points (`{(} {(}`), a real token was flagged in the second one's place. `IsRecoveryToken` now reports exactly the inserted tokens. This will matter for `carbon format`: a recovery token's text exists in no source byte range, so a minimal-edit model must know not to anchor edits on it. Assisted-by: Claude Code --- toolchain/lex/lex.cpp | 7 +++-- toolchain/lex/tokenized_buffer_test.cpp | 38 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 025d7a5b6dff..59a9dca084dc 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -1631,8 +1631,11 @@ class Lexer::ErrorRecoveryBuffer { for (; old_tokens_it->first < next_offset; ++old_tokens_it) { buffer_->token_infos_.Add(old_tokens_it->second); } - buffer_->AddToken(info); - buffer_->recovery_tokens_.set(next_offset.index); + // Flag the token just added at its index in the merged list, which + // shifts past `next_offset` (the pre-insertion index) once any earlier + // insertion has been applied. + TokenIndex added = buffer_->AddToken(info); + buffer_->recovery_tokens_.set(added.index); } for (; old_tokens_it != old_tokens_range.end(); ++old_tokens_it) { buffer_->token_infos_.Add(old_tokens_it->second); diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index 051031959865..c2d389edad33 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -618,6 +618,44 @@ TEST_F(LexerTest, MismatchedGroups) { {.kind = TokenKind::FileEnd}, })); + // Two recovery tokens inserted at the same point: each must be flagged at + // its own merged index, not the pre-insertion index of the token it was + // inserted before. + auto& buffer3b = compile_helper_.GetTokenizedBuffer("{((}"); + EXPECT_TRUE(buffer3b.has_errors()); + EXPECT_THAT( + buffer3b, + HasTokens(llvm::ArrayRef{ + {.kind = TokenKind::FileStart}, + {.kind = TokenKind::OpenCurlyBrace, .column = 1}, + {.kind = TokenKind::OpenParen, .column = 2}, + {.kind = TokenKind::OpenParen, .column = 3}, + {.kind = TokenKind::CloseParen, .column = 4, .recovery = true}, + {.kind = TokenKind::CloseParen, .column = 4, .recovery = true}, + {.kind = TokenKind::CloseCurlyBrace, .column = 4}, + {.kind = TokenKind::FileEnd}, + })); + + // Recovery insertions at two separate points: the second one's merged index + // is shifted by the first, and a real token must not be flagged in its + // place. + auto& buffer3c = compile_helper_.GetTokenizedBuffer("{(} {(}"); + EXPECT_TRUE(buffer3c.has_errors()); + EXPECT_THAT( + buffer3c, + HasTokens(llvm::ArrayRef{ + {.kind = TokenKind::FileStart}, + {.kind = TokenKind::OpenCurlyBrace, .column = 1}, + {.kind = TokenKind::OpenParen, .column = 2}, + {.kind = TokenKind::CloseParen, .column = 3, .recovery = true}, + {.kind = TokenKind::CloseCurlyBrace, .column = 3}, + {.kind = TokenKind::OpenCurlyBrace, .column = 5}, + {.kind = TokenKind::OpenParen, .column = 6}, + {.kind = TokenKind::CloseParen, .column = 7, .recovery = true}, + {.kind = TokenKind::CloseCurlyBrace, .column = 7}, + {.kind = TokenKind::FileEnd}, + })); + auto& buffer4 = compile_helper_.GetTokenizedBuffer(")({)"); EXPECT_TRUE(buffer4.has_errors()); EXPECT_THAT(