Author: Alexander Shaposhnikov Date: 2026-08-13T09:00:37-07:00 New Revision: 4a212f72234b7485666fe034ee49fea3a7c6f579
URL: https://github.com/llvm/llvm-project/commit/4a212f72234b7485666fe034ee49fea3a7c6f579 DIFF: https://github.com/llvm/llvm-project/commit/4a212f72234b7485666fe034ee49fea3a7c6f579.diff LOG: [clangd] Fix hang in include-cleaner (#215980) Address FIXME in computeIncludeCleanerFindings: locations inside the preamble patch are mapped back into the main file via their presumed locations. This fixes hang in include-cleaner when mapping refs through a stale preamble. Test plan: ninja check-all Added: Modified: clang-tools-extra/clangd/IncludeCleaner.cpp clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 382ea3ffe342b..92637178d4158 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -445,16 +445,31 @@ 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()) { + // An include replayed through the preamble patch is not transitively + // included from the main file. Translate its patch location to the + // corresponding presumed location in the main file. + SourceLocation Translated = translatePreamblePatchLocation(Loc, SM); + if (Translated != Loc) { + Loc = Translated; + break; + } Loc = SM.getIncludeLoc(SM.getFileID(Loc)); + } + // Some files, such as command-line implicit includes, are not + // transitively included from the main file. + 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
