diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp index 5fb6018e87fa..df2a4e8c9126 100644 --- a/clang-tools-extra/clangd/ClangdUnit.cpp +++ b/clang-tools-extra/clangd/ClangdUnit.cpp @@ -34,6 +34,13 @@ using namespace clang; namespace { +bool compileCommandsAreEqual(const tooling::CompileCommand &LHS, + const tooling::CompileCommand &RHS) { + // We don't check for Output, it should not matter to clangd. + return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename && + llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine); +} + template std::size_t getUsedBytes(const std::vector &Vec) { return Vec.capacity() * sizeof(T); } @@ -417,7 +424,7 @@ CppFile::rebuild(ParseInputs &&Inputs) { // Compute updated Preamble. std::shared_ptr NewPreamble = - rebuildPreamble(*CI, Inputs.FS, *ContentsBuffer); + rebuildPreamble(*CI, Inputs.CompileCommand, Inputs.FS, *ContentsBuffer); // Remove current AST to avoid wasting memory. AST = llvm::None; @@ -445,6 +452,7 @@ CppFile::rebuild(ParseInputs &&Inputs) { } // Write the results of rebuild into class fields. + Command = std::move(Inputs.CompileCommand); Preamble = std::move(NewPreamble); AST = std::move(NewAST); return Diagnostics; @@ -471,11 +479,12 @@ std::size_t CppFile::getUsedBytes() const { std::shared_ptr CppFile::rebuildPreamble(CompilerInvocation &CI, + const tooling::CompileCommand &Command, IntrusiveRefCntPtr FS, llvm::MemoryBuffer &ContentsBuffer) const { const auto &OldPreamble = this->Preamble; auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), &ContentsBuffer, 0); - if (OldPreamble && + if (OldPreamble && compileCommandsAreEqual(this->Command, Command) && OldPreamble->Preamble.CanReuse(CI, &ContentsBuffer, Bounds, FS.get())) { log("Reusing preamble for file " + Twine(FileName)); return OldPreamble; diff --git a/clang-tools-extra/clangd/ClangdUnit.h b/clang-tools-extra/clangd/ClangdUnit.h index d3849af3bd52..7d98e720b804 100644 --- a/clang-tools-extra/clangd/ClangdUnit.h +++ b/clang-tools-extra/clangd/ClangdUnit.h @@ -152,12 +152,15 @@ private: /// This method is const to ensure we don't incidentally modify any fields. std::shared_ptr rebuildPreamble(CompilerInvocation &CI, + const tooling::CompileCommand &Command, IntrusiveRefCntPtr FS, llvm::MemoryBuffer &ContentsBuffer) const; const Path FileName; const bool StorePreamblesInMemory; + /// The last CompileCommand used to build AST and Preamble. + tooling::CompileCommand Command; /// The last parsed AST. llvm::Optional AST; /// The last built Preamble. diff --git a/clang-tools-extra/unittests/clangd/ClangdTests.cpp b/clang-tools-extra/unittests/clangd/ClangdTests.cpp index 88b240ea5dc0..3f402d7dd060 100644 --- a/clang-tools-extra/unittests/clangd/ClangdTests.cpp +++ b/clang-tools-extra/unittests/clangd/ClangdTests.cpp @@ -373,6 +373,46 @@ struct bar { T x; }; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); } +TEST_F(ClangdVFSTest, ForceReparseCompileCommandDefines) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, DiagConsumer, FS, + /*AsyncThreadsCount=*/0, + /*StorePreamblesInMemory=*/true); + + // No need to sync reparses, because reparses are performed on the calling + // thread. + auto FooCpp = testPath("foo.cpp"); + const auto SourceContents = R"cpp( +#ifdef WITH_ERROR +this +#endif + +int main() { return 0; } +)cpp"; + FS.Files[FooCpp] = ""; + FS.ExpectedFile = FooCpp; + + // Parse with define, we expect to see the errors. + CDB.ExtraClangFlags = {"-DWITH_ERROR"}; + Server.addDocument(FooCpp, SourceContents); + EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); + + // Parse without the define, no errors should be produced. + CDB.ExtraClangFlags = {}; + // Currently, addDocument never checks if CompileCommand has changed, so we + // expect to see the errors. + Server.addDocument(FooCpp, SourceContents); + EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); + // But forceReparse should reparse the file with proper flags. + Server.forceReparse(FooCpp); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); + // Subsequent addDocument call should finish without errors too. + Server.addDocument(FooCpp, SourceContents); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); +} + TEST_F(ClangdVFSTest, MemoryUsage) { MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer;