Skip to content

Commit

Permalink
[clangd] Handle an expanded token range that ends in the eof token …
Browse files Browse the repository at this point in the history
…in TokenBuffer::spelledForExpanded() (#78092)

Such ranges can legitimately arise in the case of invalid code, such as
a declaration missing an ending brace.

Fixes clangd/clangd#1559
  • Loading branch information
HighCommander4 committed Jan 18, 2024
1 parent 8ff0ab0 commit 9d1dada
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
11 changes: 11 additions & 0 deletions clang-tools-extra/clangd/unittests/DumpASTTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,17 @@ TEST(DumpASTTests, Arcana) {
EXPECT_THAT(Node.children.front().arcana, testing::StartsWith("QualType "));
}

TEST(DumpASTTests, UnbalancedBraces) {
// Test that we don't crash while trying to compute a source range for the
// node whose ending brace is missing, and also that the source range is
// not empty.
Annotations Case("/*error-ok*/ $func[[int main() {]]");
ParsedAST AST = TestTU::withCode(Case.code()).build();
auto Node = dumpAST(DynTypedNode::create(findDecl(AST, "main")),
AST.getTokens(), AST.getASTContext());
ASSERT_EQ(Node.range, Case.range("func"));
}

} // namespace
} // namespace clangd
} // namespace clang
6 changes: 6 additions & 0 deletions clang/lib/Tooling/Syntax/Tokens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,12 @@ std::string TokenBuffer::Mapping::str() const {

std::optional<llvm::ArrayRef<syntax::Token>>
TokenBuffer::spelledForExpanded(llvm::ArrayRef<syntax::Token> Expanded) const {
// In cases of invalid code, AST nodes can have source ranges that include
// the `eof` token. As there's no spelling for this token, exclude it from
// the range.
if (!Expanded.empty() && Expanded.back().kind() == tok::eof) {
Expanded = Expanded.drop_back();
}
// Mapping an empty range is ambiguous in case of empty mappings at either end
// of the range, bail out in that case.
if (Expanded.empty())
Expand Down
12 changes: 12 additions & 0 deletions clang/unittests/Tooling/Syntax/TokensTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,18 @@ TEST_F(TokenBufferTest, SpelledByExpanded) {
EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("prev good")), std::nullopt);
}

TEST_F(TokenBufferTest, NoCrashForEofToken) {
recordTokens(R"cpp(
int main() {
)cpp");
ASSERT_TRUE(!Buffer.expandedTokens().empty());
ASSERT_EQ(Buffer.expandedTokens().back().kind(), tok::eof);
// Expanded range including `eof` is handled gracefully (`eof` is ignored).
EXPECT_THAT(
Buffer.spelledForExpanded(Buffer.expandedTokens()),
ValueIs(SameRange(Buffer.spelledTokens(SourceMgr->getMainFileID()))));
}

TEST_F(TokenBufferTest, ExpandedTokensForRange) {
recordTokens(R"cpp(
#define SIGN(X) X##_washere
Expand Down

0 comments on commit 9d1dada

Please sign in to comment.