Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H #include "ClangdServer.h" +#include "GlobalCompilationDatabase.h" #include "Path.h" #include "Protocol.h" #include "clang/Tooling/Core/Replacement.h" @@ -34,7 +35,17 @@ private: class LSPProtocolCallbacks; - class LSPDiagnosticsConsumer; + class LSPDiagnosticsConsumer : public DiagnosticsConsumer { + public: + LSPDiagnosticsConsumer(ClangdLSPServer &Server); + + virtual void + onDiagnosticsReady(PathRef File, + Tagged> Diagnostics); + + private: + ClangdLSPServer &Server; + }; std::vector getFixIts(StringRef File, const clangd::Diagnostic &D); @@ -56,6 +67,13 @@ DiagnosticToReplacementMap; /// Caches FixIts per file and diagnostics llvm::StringMap FixItsMap; + + // Various ClangdServer parameters go here. It's important they're created + // before ClangdServer. + DirectoryBasedGlobalCompilationDatabase CDB; + LSPDiagnosticsConsumer DiagConsumer; + RealFileSystemProvider FSProvider; + // Server must be the last member of the class to allow its destructor to exit // the worker thread that may otherwise run an async callback on partially // destructed instance of ClangdLSPServer. Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -38,18 +38,14 @@ } // namespace -class ClangdLSPServer::LSPDiagnosticsConsumer : public DiagnosticsConsumer { -public: - LSPDiagnosticsConsumer(ClangdLSPServer &Server) : Server(Server) {} - - virtual void onDiagnosticsReady(PathRef File, - Tagged> Diagnostics) { - Server.consumeDiagnostics(File, Diagnostics.Value); - } - -private: - ClangdLSPServer &Server; -}; +ClangdLSPServer::LSPDiagnosticsConsumer::LSPDiagnosticsConsumer( + ClangdLSPServer &Server) + : Server(Server) {} + +void ClangdLSPServer::LSPDiagnosticsConsumer::onDiagnosticsReady( + PathRef File, Tagged> Diagnostics) { + Server.consumeDiagnostics(File, Diagnostics.Value); +} class ClangdLSPServer::LSPProtocolCallbacks : public ProtocolCallbacks { public: @@ -196,11 +192,8 @@ } ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, bool RunSynchronously) - : Out(Out), - Server(llvm::make_unique(), - llvm::make_unique(*this), - llvm::make_unique(), - RunSynchronously) {} + : Out(Out), DiagConsumer(*this), + Server(CDB, DiagConsumer, FSProvider, RunSynchronously) {} void ClangdLSPServer::run(std::istream &In) { assert(!IsDone && "Run was called before"); Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -136,10 +136,9 @@ /// diagnostics for tracked files). class ClangdServer { public: - ClangdServer(std::unique_ptr CDB, - std::unique_ptr DiagConsumer, - std::unique_ptr FSProvider, - bool RunSynchronously); + ClangdServer(GlobalCompilationDatabase &CDB, + DiagnosticsConsumer &DiagConsumer, + FileSystemProvider &FSProvider, bool RunSynchronously); /// Add a \p File to the list of tracked C++ files or update the contents if /// \p File is already tracked. Also schedules parsing of the AST for it on a @@ -181,9 +180,9 @@ std::string dumpAST(PathRef File); private: - std::unique_ptr CDB; - std::unique_ptr DiagConsumer; - std::unique_ptr FSProvider; + GlobalCompilationDatabase &CDB; + DiagnosticsConsumer &DiagConsumer; + FileSystemProvider &FSProvider; DraftStore DraftMgr; ClangdUnitStore Units; std::shared_ptr PCHs; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -138,12 +138,11 @@ RequestCV.notify_one(); } -ClangdServer::ClangdServer(std::unique_ptr CDB, - std::unique_ptr DiagConsumer, - std::unique_ptr FSProvider, +ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB, + DiagnosticsConsumer &DiagConsumer, + FileSystemProvider &FSProvider, bool RunSynchronously) - : CDB(std::move(CDB)), DiagConsumer(std::move(DiagConsumer)), - FSProvider(std::move(FSProvider)), + : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), PCHs(std::make_shared()), WorkScheduler(RunSynchronously) {} @@ -157,11 +156,11 @@ assert(FileContents.Draft && "No contents inside a file that was scheduled for reparse"); - auto TaggedFS = FSProvider->getTaggedFileSystem(); + auto TaggedFS = FSProvider.getTaggedFileSystem(); Units.runOnUnit( - FileStr, *FileContents.Draft, *CDB, PCHs, TaggedFS.Value, + FileStr, *FileContents.Draft, CDB, PCHs, TaggedFS.Value, [&](ClangdUnit const &Unit) { - DiagConsumer->onDiagnosticsReady( + DiagConsumer.onDiagnosticsReady( FileStr, make_tagged(Unit.getLocalDiagnostics(), TaggedFS.Tag)); }); }); @@ -198,11 +197,11 @@ } std::vector Result; - auto TaggedFS = FSProvider->getTaggedFileSystem(); + auto TaggedFS = FSProvider.getTaggedFileSystem(); // It would be nice to use runOnUnitWithoutReparse here, but we can't // guarantee the correctness of code completion cache here if we don't do the // reparse. - Units.runOnUnit(File, *OverridenContents, *CDB, PCHs, TaggedFS.Value, + Units.runOnUnit(File, *OverridenContents, CDB, PCHs, TaggedFS.Value, [&](ClangdUnit &Unit) { Result = Unit.codeComplete(*OverridenContents, Pos, TaggedFS.Value); Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -214,11 +214,6 @@ return replacePtrsInDump(DumpWithMemLocs); } -template -std::unique_ptr getAndMove(std::unique_ptr Ptr, T *&Output) { - Output = Ptr.get(); - return Ptr; -} } // namespace class ClangdVFSTest : public ::testing::Test { @@ -243,22 +238,19 @@ PathRef SourceFileRelPath, StringRef SourceContents, std::vector> ExtraFiles = {}, bool ExpectErrors = false) { - MockFSProvider *FS; - ErrorCheckingDiagConsumer *DiagConsumer; - ClangdServer Server( - llvm::make_unique(), - getAndMove(llvm::make_unique(), - DiagConsumer), - getAndMove(llvm::make_unique(), FS), - /*RunSynchronously=*/false); + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, DiagConsumer, FS, + /*RunSynchronously=*/false); for (const auto &FileWithContents : ExtraFiles) - FS->Files[getVirtualTestFilePath(FileWithContents.first)] = + FS.Files[getVirtualTestFilePath(FileWithContents.first)] = FileWithContents.second; auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath); Server.addDocument(SourceFilename, SourceContents); auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); - EXPECT_EQ(ExpectErrors, DiagConsumer->hadErrorInLastDiags()); + EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); return Result; } }; @@ -299,13 +291,11 @@ } TEST_F(ClangdVFSTest, Reparse) { - MockFSProvider *FS; - ErrorCheckingDiagConsumer *DiagConsumer; - ClangdServer Server( - llvm::make_unique(), - getAndMove(llvm::make_unique(), DiagConsumer), - getAndMove(llvm::make_unique(), FS), - /*RunSynchronously=*/false); + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, DiagConsumer, FS, + /*RunSynchronously=*/false); const auto SourceContents = R"cpp( #include "foo.h" @@ -315,34 +305,32 @@ auto FooCpp = getVirtualTestFilePath("foo.cpp"); auto FooH = getVirtualTestFilePath("foo.h"); - FS->Files[FooH] = "int a;"; - FS->Files[FooCpp] = SourceContents; + FS.Files[FooH] = "int a;"; + FS.Files[FooCpp] = SourceContents; Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); Server.addDocument(FooCpp, ""); auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); Server.addDocument(FooCpp, SourceContents); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); EXPECT_NE(DumpParse1, DumpParseEmpty); } TEST_F(ClangdVFSTest, ReparseOnHeaderChange) { - MockFSProvider *FS; - ErrorCheckingDiagConsumer *DiagConsumer; + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; - ClangdServer Server( - llvm::make_unique(), - getAndMove(llvm::make_unique(), DiagConsumer), - getAndMove(llvm::make_unique(), FS), - /*RunSynchronously=*/false); + ClangdServer Server(CDB, DiagConsumer, FS, + /*RunSynchronously=*/false); const auto SourceContents = R"cpp( #include "foo.h" @@ -352,50 +340,47 @@ auto FooCpp = getVirtualTestFilePath("foo.cpp"); auto FooH = getVirtualTestFilePath("foo.h"); - FS->Files[FooH] = "int a;"; - FS->Files[FooCpp] = SourceContents; + FS.Files[FooH] = "int a;"; + FS.Files[FooCpp] = SourceContents; Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); - FS->Files[FooH] = ""; + FS.Files[FooH] = ""; Server.forceReparse(FooCpp); auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_TRUE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); - FS->Files[FooH] = "int a;"; + FS.Files[FooH] = "int a;"; Server.forceReparse(FooCpp); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags()); + EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); EXPECT_NE(DumpParse1, DumpParseDifferent); } TEST_F(ClangdVFSTest, CheckVersions) { - MockFSProvider *FS; - ErrorCheckingDiagConsumer *DiagConsumer; - - ClangdServer Server( - llvm::make_unique(), - getAndMove(llvm::make_unique(), DiagConsumer), - getAndMove(llvm::make_unique(), FS), - /*RunSynchronously=*/true); + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, DiagConsumer, FS, + /*RunSynchronously=*/true); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = "int a;"; - FS->Files[FooCpp] = SourceContents; - FS->Tag = "123"; + FS.Files[FooCpp] = SourceContents; + FS.Tag = "123"; Server.addDocument(FooCpp, SourceContents); - EXPECT_EQ(DiagConsumer->lastVFSTag(), FS->Tag); - EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS->Tag); + EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); + EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); - FS->Tag = "321"; + FS.Tag = "321"; Server.addDocument(FooCpp, SourceContents); - EXPECT_EQ(DiagConsumer->lastVFSTag(), FS->Tag); - EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS->Tag); + EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); + EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); } class ClangdCompletionTest : public ClangdVFSTest { @@ -410,11 +395,11 @@ }; TEST_F(ClangdCompletionTest, CheckContentsOverride) { - MockFSProvider *FS; + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; - ClangdServer Server(llvm::make_unique(), - llvm::make_unique(), - getAndMove(llvm::make_unique(), FS), + ClangdServer Server(CDB, DiagConsumer, FS, /*RunSynchronously=*/false); auto FooCpp = getVirtualTestFilePath("foo.cpp"); @@ -433,7 +418,7 @@ // string literal of the SourceContents starts with a newline(it's easy to // miss). Position CompletePos = {2, 8}; - FS->Files[FooCpp] = SourceContents; + FS.Files[FooCpp] = SourceContents; Server.addDocument(FooCpp, SourceContents);