https://github.com/alexander-shaposhnikov updated https://github.com/llvm/llvm-project/pull/215980
>From 7c0fd3fb5d62fa4ab8e6160fe3cbd9625faca403 Mon Sep 17 00:00:00 2001 From: Alexander Shaposhnikov <[email protected]> Date: Thu, 13 Aug 2026 00:54:44 -0700 Subject: [PATCH] [clangd] Fix hang in include-cleaner when mapping refs through a stale preamble --- clang-tools-extra/clangd/IncludeCleaner.cpp | 27 +++++++++++-- .../clangd/unittests/IncludeCleanerTests.cpp | 38 +++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 382ea3ffe342b..c9b8f8cd2db00 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -445,16 +445,35 @@ computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeAngledIncludes) { // offsets could lead into crashes in presence of stale preambles. Hence // we use "getFileLoc" instead to make sure it always points into main // file. - // FIXME: Use presumed locations to map such usages back to patched - // locations safely. auto Loc = SM.getFileLoc(Ref.RefLocation); // File locations can be outside of the main file if macro is expanded // through an #include. - while (SM.getFileID(Loc) != SM.getMainFileID()) + while (Loc.isValid() && SM.getFileID(Loc) != SM.getMainFileID()) { + // Use presumed locations to map locations from the preamble section + // and the preamble patch (which is not included from the main file) + // back into the main file. + PresumedLoc Presumed = SM.getPresumedLoc(Loc); + if (Presumed.isValid() && Presumed.getLine() != 0 && + Presumed.getColumn() != 0 && + Presumed.getFilename() == + SM.getFileEntryRefForID(SM.getMainFileID())->getName()) { + Loc = SM.translateLineCol(SM.getMainFileID(), Presumed.getLine(), + Presumed.getColumn()); + break; + } Loc = SM.getIncludeLoc(SM.getFileID(Loc)); + } + // Bail out if the chain didn't reach the main file, e.g. a file + // entered from the command line is rooted at the predefines buffer. + if (Loc.isInvalid()) + return; auto TouchingTokens = syntax::spelledTokensTouching(Loc, AST.getTokens()); - assert(!TouchingTokens.empty()); + // Locations translated through a stale preamble refer to the baseline + // contents and are not guaranteed to point at a token in the current + // contents. + if (TouchingTokens.empty()) + return; // Loc points to the start offset of the ref token, here we use the last // element of the TouchingTokens, e.g. avoid getting the "::" for // "ns::^abc". diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index ec733cbe9c42d..5b9db8af33f12 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" +#include "Compiler.h" #include "Diagnostics.h" #include "IncludeCleaner.h" #include "ParsedAST.h" @@ -525,6 +526,43 @@ TEST(IncludeCleaner, MissingIncludesAreUnique) { EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range()); } +TEST(IncludeCleaner, NoHangOnRefExpandedInsidePreamblePatchInclude) { + llvm::StringLiteral Baseline = R"cpp(// comment +#include "all.h" +#define RET Foo +)cpp"; + Annotations Modified(R"cpp(// comment +#include "all.h" +#define RET Foo +#include [["rettype.inc"]] +plugin_callback(); +)cpp"); + + TestTU TU; + TU.AdditionalFiles["foo.h"] = guard("struct Foo {};"); + TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\""); + TU.AdditionalFiles["rettype.inc"] = "RET\n"; + + TU.Code = Baseline.str(); + auto BaselinePreamble = TU.preamble(); + ASSERT_TRUE(BaselinePreamble); + + IgnoreDiagnostics Diags; + MockFS FS; + TU.Code = Modified.code().str(); + auto CI = buildCompilerInvocation(TU.inputs(FS), Diags); + ASSERT_TRUE(CI); + auto AST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), + std::move(CI), {}, std::move(BaselinePreamble)); + ASSERT_TRUE(AST); + auto Findings = computeIncludeCleanerFindings(*AST).MissingIncludes; + ASSERT_THAT(Findings, testing::SizeIs(1)); + auto RefRange = Findings.front().SymRefRange; + const auto &SM = AST->getSourceManager(); + EXPECT_EQ(RefRange.file(), SM.getMainFileID()); + EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), Modified.range()); +} + TEST(IncludeCleaner, NoCrash) { TestTU TU; Annotations MainCode(R"cpp( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
