diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -383,7 +383,7 @@ Includes.MainFileIncludes = Patch->preambleIncludes(); // Replay the preamble includes so that clang-tidy checks can see them. ReplayPreamble::attach(Patch->preambleIncludes(), *Clang, - Preamble->Preamble.getBounds()); + Patch->modifiedBounds()); } // Important: collectIncludeStructure is registered *after* ReplayPreamble! // Otherwise we would collect the replayed includes again... diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -31,6 +31,7 @@ #include "support/Path.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PrecompiledPreamble.h" +#include "clang/Lex/Lexer.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/StringRef.h" @@ -119,6 +120,9 @@ /// using the presumed-location mechanism. std::vector preambleIncludes() const; + /// Returns preamble bounds for the Modified. + PreambleBounds modifiedBounds() const { return ModifiedBounds; } + /// Returns textual patch contents. llvm::StringRef text() const { return PatchContents; } @@ -129,6 +133,7 @@ /// Includes that are present in both \p Baseline and \p Modified. Used for /// patching includes of baseline preamble. std::vector PreambleIncludes; + PreambleBounds ModifiedBounds = {0, false}; }; /// Translates locations inside preamble patch to their main-file equivalent diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -217,6 +217,7 @@ struct ScannedPreamble { std::vector Includes; std::vector TextualDirectives; + PreambleBounds Bounds = {0, false}; }; /// Scans the preprocessor directives in the preamble section of the file by @@ -284,6 +285,7 @@ IncludeStructure Includes; PP.addPPCallbacks(collectIncludeStructureCallback(SM, &Includes)); ScannedPreamble SP; + SP.Bounds = Bounds; PP.addPPCallbacks( std::make_unique(PP, SP.TextualDirectives)); if (llvm::Error Err = Action.Execute()) @@ -463,6 +465,7 @@ llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName), PreamblePatchHeaderName); PP.PatchFileName = PatchName.str().str(); + PP.ModifiedBounds = ModifiedScan->Bounds; llvm::raw_string_ostream Patch(PP.PatchContents); // Set default filename for subsequent #line directives @@ -548,6 +551,7 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData &Preamble) { PreamblePatch PP; PP.PreambleIncludes = Preamble.Includes.MainFileIncludes; + PP.ModifiedBounds = Preamble.Preamble.getBounds(); return PP; } diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -444,24 +444,76 @@ EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name); } + TU.AdditionalFiles["a.h"] = ""; + TU.AdditionalFiles["b.h"] = ""; + TU.AdditionalFiles["c.h"] = ""; // Make sure replay logic works with patched preambles. - TU.Code = ""; - StoreDiags Diags; + llvm::StringLiteral Baseline = R"cpp( + #include "a.h" + #include "c.h")cpp"; MockFSProvider FS; + TU.Code = Baseline.str(); auto Inputs = TU.inputs(FS); - auto CI = buildCompilerInvocation(Inputs, Diags); - auto EmptyPreamble = - buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr); - ASSERT_TRUE(EmptyPreamble); - TU.Code = "#include "; + auto BaselinePreamble = TU.preamble(); + ASSERT_TRUE(BaselinePreamble); + + // First make sure we don't crash on various modifications to the preamble. + llvm::StringLiteral Cases[] = { + // clang-format off + // New include in middle. + R"cpp( + #include "a.h" + #include "b.h" + #include "c.h")cpp", + // New include at top. + R"cpp( + #include "b.h" + #include "a.h" + #include "c.h")cpp", + // New include at bottom. + R"cpp( + #include "a.h" + #include "c.h" + #include "b.h")cpp", + // Same size with a missing include. + R"cpp( + #include "a.h" + #include "b.h")cpp", + // Smaller with no new includes. + R"cpp( + #include "a.h")cpp", + // Smaller with a new includes. + R"cpp( + #include "b.h")cpp", + // clang-format on + }; + for (llvm::StringLiteral Case : Cases) { + TU.Code = Case.str(); + + IgnoreDiagnostics Diags; + auto CI = buildCompilerInvocation(TU.inputs(FS), Diags); + auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), + std::move(CI), {}, BaselinePreamble); + ASSERT_TRUE(PatchedAST); + EXPECT_TRUE(PatchedAST->getDiagnostics().empty()); + } + + // Then ensure correctness by making sure includes were seen only once. + // Note that we first see the includes from the patch, as preamble includes + // are replayed after exiting the built-in file. Includes.clear(); + TU.Code = R"cpp( + #include "a.h" + #include "b.h")cpp"; + IgnoreDiagnostics Diags; + auto CI = buildCompilerInvocation(TU.inputs(FS), Diags); auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS), - std::move(CI), {}, EmptyPreamble); + std::move(CI), {}, BaselinePreamble); ASSERT_TRUE(PatchedAST); - // Make sure includes were seen only once. + EXPECT_TRUE(PatchedAST->getDiagnostics().empty()); EXPECT_THAT(Includes, ElementsAre(WithFileName(testPath("__preamble_patch__.h")), - WithFileName("a.h"))); + WithFileName("b.h"), WithFileName("a.h"))); } TEST(ParsedASTTest, PatchesAdditionalIncludes) { diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -488,6 +488,51 @@ EXPECT_EQ(DecompLoc.first, SM.getMainFileID()); EXPECT_EQ(SM.getLineNumber(DecompLoc.first, DecompLoc.second), 3U); } + +TEST(PreamblePatch, ModifiedBounds) { + struct { + llvm::StringLiteral Baseline; + llvm::StringLiteral Modified; + } Cases[] = { + // Size increased + { + "", + R"cpp( + #define FOO + FOO)cpp", + }, + // Stayed same + {"#define FOO", "#define BAR"}, + // Got smaller + { + R"cpp( + #define FOO + #undef FOO)cpp", + "#define FOO"}, + }; + + for (const auto &Case : Cases) { + auto TU = TestTU::withCode(Case.Baseline); + auto BaselinePreamble = TU.preamble(); + ASSERT_TRUE(BaselinePreamble); + + Annotations Modified(Case.Modified); + TU.Code = Modified.code().str(); + MockFSProvider FSProvider; + auto PP = PreamblePatch::create(testPath(TU.Filename), + TU.inputs(FSProvider), *BaselinePreamble); + + IgnoreDiagnostics Diags; + auto CI = buildCompilerInvocation(TU.inputs(FSProvider), Diags); + ASSERT_TRUE(CI); + + const auto ExpectedBounds = + Lexer::ComputePreamble(Case.Modified, *CI->getLangOpts()); + EXPECT_EQ(PP.modifiedBounds().Size, ExpectedBounds.Size); + EXPECT_EQ(PP.modifiedBounds().PreambleEndsAtStartOfLine, + ExpectedBounds.PreambleEndsAtStartOfLine); + } +} } // namespace } // namespace clangd } // namespace clang