Index: clangd/ClangdLSPServer.h =================================================================== --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -36,9 +36,11 @@ /// If \p CompileCommandsDir has a value, compile_commands.json will be /// loaded only from \p CompileCommandsDir. Otherwise, clangd will look /// for compile_commands.json in all parent directories of each file. + /// If UseDirBasedCDB is false, compile commands are not read from disk. + // FIXME: Clean up signature around CDBs. ClangdLSPServer(Transport &Transp, const clangd::CodeCompleteOptions &CCOpts, - llvm::Optional CompileCommandsDir, - bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts); + llvm::Optional CompileCommandsDir, bool UseDirBasedCDB, + const ClangdServer::Options &Opts); ~ClangdLSPServer(); /// Run LSP server loop, communicating with the Transport provided in the @@ -105,43 +107,6 @@ /// Caches FixIts per file and diagnostics llvm::StringMap FixItsMap; - /// Encapsulates the directory-based or the in-memory compilation database - /// that's used by the LSP server. - class CompilationDB { - public: - static CompilationDB makeInMemory(); - static CompilationDB - makeDirectoryBased(llvm::Optional CompileCommandsDir); - - /// Sets the compilation command for a particular file. - /// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB. - /// - /// \returns True if the File had no compilation command before. - bool - setCompilationCommandForFile(PathRef File, - tooling::CompileCommand CompilationCommand); - - /// Adds extra compilation flags to the compilation command for a particular - /// file. Only valid for directory-based CDB, no-op and error log on - /// InMemoryCDB; - void setExtraFlagsForFile(PathRef File, - std::vector ExtraFlags); - - /// Returns a CDB that should be used to get compile commands for the - /// current instance of ClangdLSPServer. - GlobalCompilationDatabase &getCDB() { return *CDB; } - - private: - CompilationDB(std::unique_ptr CDB, - bool IsDirectoryBased) - : CDB(std::move(CDB)), IsDirectoryBased(IsDirectoryBased) {} - - // if IsDirectoryBased is true, an instance of InMemoryCDB. - // If IsDirectoryBased is false, an instance of DirectoryBasedCDB. - std::unique_ptr CDB; - bool IsDirectoryBased; - }; - // Most code should not deal with Transport directly. // MessageHandler deals with incoming messages, use call() etc for outgoing. clangd::Transport &Transp; @@ -168,9 +133,11 @@ DraftStore DraftMgr; // The CDB is created by the "initialize" LSP method. - bool UseInMemoryCDB; // FIXME: make this a capability. + bool UseDirBasedCDB; // FIXME: make this a capability. llvm::Optional CompileCommandsDir; // FIXME: merge with capability? - llvm::Optional CDB; + std::unique_ptr BaseCDB; + // CDB is BaseCDB plus any comands overridden via LSP extensions. + llvm::Optional CDB; // The ClangdServer is created by the "initialize" LSP method. // It is destroyed before run() returns, to ensure worker threads exit. ClangdServer::Options ClangdServerOpts; Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -305,11 +305,12 @@ ErrorCode::InvalidRequest)); if (const auto &Dir = Params.initializationOptions.compilationDatabasePath) CompileCommandsDir = Dir; - CDB.emplace(UseInMemoryCDB - ? CompilationDB::makeInMemory() - : CompilationDB::makeDirectoryBased(CompileCommandsDir)); - Server.emplace(CDB->getCDB(), FSProvider, - static_cast(*this), ClangdServerOpts); + if (UseDirBasedCDB) + BaseCDB = llvm::make_unique( + CompileCommandsDir); + CDB.emplace(BaseCDB.get()); + Server.emplace(*CDB, FSProvider, static_cast(*this), + ClangdServerOpts); applyConfiguration(Params.initializationOptions.ConfigSettings); CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets; @@ -366,8 +367,6 @@ void ClangdLSPServer::onDocumentDidOpen( const DidOpenTextDocumentParams &Params) { PathRef File = Params.textDocument.uri.file(); - if (Params.metadata && !Params.metadata->extraFlags.empty()) - CDB->setExtraFlagsForFile(File, std::move(Params.metadata->extraFlags)); const std::string &Contents = Params.textDocument.text; @@ -660,12 +659,14 @@ /// The opened files need to be reparsed only when some existing /// entries are changed. PathRef File = Entry.first; - if (!CDB->setCompilationCommandForFile( - File, tooling::CompileCommand( - std::move(Entry.second.workingDirectory), File, - std::move(Entry.second.compilationCommand), - /*Output=*/""))) - ShouldReparseOpenFiles = true; + auto Old = CDB->getCompileCommand(File); + auto New = + tooling::CompileCommand(std::move(Entry.second.workingDirectory), File, + std::move(Entry.second.compilationCommand), + /*Output=*/""); + if (Old != New) + CDB->setCompileCommand(File, std::move(New)); + ShouldReparseOpenFiles = true; } if (ShouldReparseOpenFiles) reparseOpenedFiles(); @@ -686,12 +687,12 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp, const clangd::CodeCompleteOptions &CCOpts, Optional CompileCommandsDir, - bool ShouldUseInMemoryCDB, + bool UseDirBasedCDB, const ClangdServer::Options &Opts) : Transp(Transp), MsgHandler(new MessageHandler(*this)), CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()), SupportedCompletionItemKinds(defaultCompletionItemKinds()), - UseInMemoryCDB(ShouldUseInMemoryCDB), + UseDirBasedCDB(UseDirBasedCDB), CompileCommandsDir(std::move(CompileCommandsDir)), ClangdServerOpts(Opts) { // clang-format off @@ -785,43 +786,5 @@ WantDiagnostics::Auto); } -ClangdLSPServer::CompilationDB ClangdLSPServer::CompilationDB::makeInMemory() { - return CompilationDB(llvm::make_unique(), - /*IsDirectoryBased=*/false); -} - -ClangdLSPServer::CompilationDB -ClangdLSPServer::CompilationDB::makeDirectoryBased( - Optional CompileCommandsDir) { - auto CDB = llvm::make_unique( - std::move(CompileCommandsDir)); - return CompilationDB(std::move(CDB), - /*IsDirectoryBased=*/true); -} - -bool ClangdLSPServer::CompilationDB::setCompilationCommandForFile( - PathRef File, tooling::CompileCommand CompilationCommand) { - if (IsDirectoryBased) { - elog("Trying to set compile command for {0} while using directory-based " - "compilation database", - File); - return false; - } - return static_cast(CDB.get()) - ->setCompilationCommandForFile(File, std::move(CompilationCommand)); -} - -void ClangdLSPServer::CompilationDB::setExtraFlagsForFile( - PathRef File, std::vector ExtraFlags) { - if (!IsDirectoryBased) { - elog("Trying to set extra flags for {0} while using in-memory compilation " - "database", - File); - return; - } - static_cast(CDB.get()) - ->setExtraFlagsForFile(File, std::move(ExtraFlags)); -} - } // namespace clangd } // namespace clang Index: clangd/GlobalCompilationDatabase.h =================================================================== --- clangd/GlobalCompilationDatabase.h +++ clangd/GlobalCompilationDatabase.h @@ -59,16 +59,9 @@ llvm::Optional getCompileCommand(PathRef File) const override; - /// Uses the default fallback command, adding any extra flags. - tooling::CompileCommand getFallbackCommand(PathRef File) const override; - - /// Sets the extra flags that should be added to a file. - void setExtraFlagsForFile(PathRef File, std::vector ExtraFlags); - private: tooling::CompilationDatabase *getCDBForFile(PathRef File) const; tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const; - void addExtraFlags(PathRef File, tooling::CompileCommand &C) const; mutable std::mutex Mutex; /// Caches compilation databases loaded from directories(keys are @@ -76,30 +69,35 @@ mutable llvm::StringMap> CompilationDatabases; - /// Stores extra flags per file. - llvm::StringMap> ExtraFlagsForFile; /// Used for command argument pointing to folder where compile_commands.json /// is located. llvm::Optional CompileCommandsDir; }; -/// Gets compile args from an in-memory mapping based on a filepath. Typically -/// used by clients who provide the compile commands themselves. -class InMemoryCompilationDb : public GlobalCompilationDatabase { +/// Wraps another compilation database, and supports overriding the commands +/// using an in-memory mapping. +class OverlayCDB : public GlobalCompilationDatabase { public: - /// Gets compile command for \p File from the stored mapping. + // Base may be null, in which case no entries are inherited. + // FallbackFlags are added to the fallback compile command. + OverlayCDB(const GlobalCompilationDatabase *Base, + std::vector FallbackFlags = {}) + : Base(Base), FallbackFlags(std::move(FallbackFlags)) {} + llvm::Optional getCompileCommand(PathRef File) const override; + tooling::CompileCommand getFallbackCommand(PathRef File) const override; - /// Sets the compilation command for a particular file. - /// - /// \returns True if the File had no compilation command before. - bool setCompilationCommandForFile(PathRef File, - tooling::CompileCommand CompilationCommand); + /// Sets or clears the compilation command for a particular file. + void + setCompileCommand(PathRef File, + llvm::Optional CompilationCommand); private: mutable std::mutex Mutex; llvm::StringMap Commands; /* GUARDED_BY(Mut) */ + const GlobalCompilationDatabase *Base; + std::vector FallbackFlags; }; } // namespace clangd Index: clangd/GlobalCompilationDatabase.cpp =================================================================== --- clangd/GlobalCompilationDatabase.cpp +++ clangd/GlobalCompilationDatabase.cpp @@ -41,45 +41,14 @@ DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const { if (auto CDB = getCDBForFile(File)) { auto Candidates = CDB->getCompileCommands(File); - if (!Candidates.empty()) { - addExtraFlags(File, Candidates.front()); + if (!Candidates.empty()) return std::move(Candidates.front()); - } } else { log("Failed to find compilation database for {0}", File); } return None; } -tooling::CompileCommand -DirectoryBasedGlobalCompilationDatabase::getFallbackCommand( - PathRef File) const { - auto C = GlobalCompilationDatabase::getFallbackCommand(File); - addExtraFlags(File, C); - return C; -} - -void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile( - PathRef File, std::vector ExtraFlags) { - std::lock_guard Lock(Mutex); - ExtraFlagsForFile[File] = std::move(ExtraFlags); -} - -void DirectoryBasedGlobalCompilationDatabase::addExtraFlags( - PathRef File, tooling::CompileCommand &C) const { - std::lock_guard Lock(Mutex); - - auto It = ExtraFlagsForFile.find(File); - if (It == ExtraFlagsForFile.end()) - return; - - auto &Args = C.CommandLine; - assert(Args.size() >= 2 && "Expected at least [compiler, source file]"); - // The last argument of CommandLine is the name of the input file. - // Add ExtraFlags before it. - Args.insert(Args.end() - 1, It->second.begin(), It->second.end()); -} - tooling::CompilationDatabase * DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const { // FIXME(ibiryukov): Invalidate cached compilation databases on changes @@ -111,22 +80,32 @@ } Optional -InMemoryCompilationDb::getCompileCommand(PathRef File) const { +OverlayCDB::getCompileCommand(PathRef File) const { + { + std::lock_guard Lock(Mutex); + auto It = Commands.find(File); + if (It != Commands.end()) + return It->second; + } + return Base ? Base->getCompileCommand(File) : None; +} + +tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const { + auto Cmd = Base ? Base->getFallbackCommand(File) + : GlobalCompilationDatabase::getFallbackCommand(File); std::lock_guard Lock(Mutex); - auto It = Commands.find(File); - if (It == Commands.end()) - return None; - return It->second; + Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(), + FallbackFlags.end()); + return Cmd; } -bool InMemoryCompilationDb::setCompilationCommandForFile( - PathRef File, tooling::CompileCommand CompilationCommand) { +void OverlayCDB::setCompileCommand( + PathRef File, llvm::Optional Cmd) { std::unique_lock Lock(Mutex); - auto ItInserted = Commands.insert(std::make_pair(File, CompilationCommand)); - if (ItInserted.second) - return true; - ItInserted.first->setValue(std::move(CompilationCommand)); - return false; + if (Cmd) + Commands[File] = std::move(*Cmd); + else + Commands.erase(File); } } // namespace clangd Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -175,11 +175,6 @@ llvm::json::Value toJSON(const Location &); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Location &); -struct Metadata { - std::vector extraFlags; -}; -bool fromJSON(const llvm::json::Value &, Metadata &); - struct TextEdit { /// The range of the text document to be manipulated. To insert /// text into a document create a range where start === end. @@ -411,9 +406,6 @@ struct DidOpenTextDocumentParams { /// The document that was opened. TextDocumentItem textDocument; - - /// Extension storing per-file metadata, such as compilation flags. - llvm::Optional metadata; }; bool fromJSON(const llvm::json::Value &, DidOpenTextDocumentParams &); Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -119,14 +119,6 @@ O.map("version", R.version) && O.map("text", R.text); } -bool fromJSON(const json::Value &Params, Metadata &R) { - json::ObjectMapper O(Params); - if (!O) - return false; - O.map("extraFlags", R.extraFlags); - return true; -} - bool fromJSON(const json::Value &Params, TextEdit &R) { json::ObjectMapper O(Params); return O && O.map("range", R.range) && O.map("newText", R.newText); @@ -262,8 +254,7 @@ bool fromJSON(const json::Value &Params, DidOpenTextDocumentParams &R) { json::ObjectMapper O(Params); - return O && O.map("textDocument", R.textDocument) && - O.map("metadata", R.metadata); + return O && O.map("textDocument", R.textDocument); } bool fromJSON(const json::Value &Params, DidCloseTextDocumentParams &R) { Index: clangd/tool/ClangdMain.cpp =================================================================== --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -322,7 +322,7 @@ InputStyle); ClangdLSPServer LSPServer( *Transport, CCOpts, CompileCommandsDirPath, - /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts); + /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts); constexpr int NoShutdownRequestErrorCode = 1; set_thread_name("clangd.main"); return LSPServer.run() ? 0 : NoShutdownRequestErrorCode; Index: test/clangd/extra-flags.test =================================================================== --- test/clangd/extra-flags.test +++ /dev/null @@ -1,52 +0,0 @@ -# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} ---- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"extraFlags":["-Wall"]}}} -# CHECK: "method": "textDocument/publishDiagnostics", -# CHECK-NEXT: "params": { -# CHECK-NEXT: "diagnostics": [ -# CHECK-NEXT: { -# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 28, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 27, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "severity": 2 -# CHECK-NEXT: } -# CHECK-NEXT: ], -# CHECK-NEXT: "uri": "file://{{.*}}/foo.c" -# CHECK-NEXT: } ---- -{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i+1; }"}]}} -# CHECK: "method": "textDocument/publishDiagnostics", -# CHECK-NEXT: "params": { -# CHECK-NEXT: "diagnostics": [ -# CHECK-NEXT: { -# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 28, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 27, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "severity": 2 -# CHECK-NEXT: } -# CHECK-NEXT: ], -# CHECK-NEXT: "uri": "file://{{.*}}/foo.c" -# CHECK-NEXT: } ---- -{"jsonrpc":"2.0","id":5,"method":"shutdown"} ---- -{"jsonrpc":"2.0","method":"exit"} - - Index: unittests/clangd/GlobalCompilationDatabaseTests.cpp =================================================================== --- unittests/clangd/GlobalCompilationDatabaseTests.cpp +++ unittests/clangd/GlobalCompilationDatabaseTests.cpp @@ -33,6 +33,61 @@ testPath("foo/bar.h"))); } +static tooling::CompileCommand cmd(StringRef File, StringRef Arg) { + return tooling::CompileCommand(testRoot(), File, {"clang", Arg, File}, ""); +} + +class OverlayCDBTest : public ::testing::Test { + class BaseCDB : public GlobalCompilationDatabase { + public: + Optional + getCompileCommand(StringRef File) const override { + if (File == testPath("foo.cc")) + return cmd(File, "-DA=1"); + return None; + } + + tooling::CompileCommand getFallbackCommand(StringRef File) const override { + return cmd(File, "-DA=2"); + } + }; + +protected: + OverlayCDBTest() : Base(llvm::make_unique()) {} + std::unique_ptr Base; +}; + +TEST_F(OverlayCDBTest, GetCompileCommand) { + OverlayCDB CDB(Base.get()); + EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")), + Base->getCompileCommand(testPath("foo.cc"))); + EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None); + + auto Override = cmd(testPath("foo.cc"), "-DA=3"); + CDB.setCompileCommand(testPath("foo.cc"), Override); + EXPECT_EQ(CDB.getCompileCommand(testPath("foo.cc")), Override); + EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), llvm::None); + CDB.setCompileCommand(testPath("missing.cc"), Override); + EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), Override); +} + +TEST_F(OverlayCDBTest, GetFallbackCommand) { + OverlayCDB CDB(Base.get(), {"-DA=4"}); + EXPECT_THAT(CDB.getFallbackCommand(testPath("bar.cc")).CommandLine, + ElementsAre("clang", "-DA=2", testPath("bar.cc"), "-DA=4")); +} + +TEST_F(OverlayCDBTest, NoBase) { + OverlayCDB CDB(nullptr, {"-DA=6"}); + EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), None); + auto Override = cmd(testPath("bar.cc"), "-DA=5"); + CDB.setCompileCommand(testPath("bar.cc"), Override); + EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), Override); + + EXPECT_THAT(CDB.getFallbackCommand(testPath("foo.cc")).CommandLine, + ElementsAre("clang", testPath("foo.cc"), "-DA=6")); +} + } // namespace } // namespace clangd } // namespace clang