Author: sammccall Date: Fri Nov 2 06:06:55 2018 New Revision: 345969 URL: http://llvm.org/viewvc/llvm-project?rev=345969&view=rev Log: [clangd] Remove didOpen extraFlags extension.
Summary: This was added in D34947 to support YCM, but YCM actually provides *all* args, and this was never actually used. Meanwhile, we grew another extension that allows specifying all args. I did find one user of this extension: https://github.com/thomasjo/atom-ide-cpp. I'll reach out, there are multiple good alternatives: - compile_commands.txt can serve the same purpose as .clang_complete there - we can add an extension to support setting the fallback command Reviewers: ilya-biryukov Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits Differential Revision: https://reviews.llvm.org/D53641 Removed: clang-tools-extra/trunk/test/clangd/extra-flags.test Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Nov 2 06:06:55 2018 @@ -366,8 +366,6 @@ void ClangdLSPServer::onShutdown(const S 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; @@ -811,17 +809,5 @@ bool ClangdLSPServer::CompilationDB::set ->setCompilationCommandForFile(File, std::move(CompilationCommand)); } -void ClangdLSPServer::CompilationDB::setExtraFlagsForFile( - PathRef File, std::vector<std::string> ExtraFlags) { - if (!IsDirectoryBased) { - elog("Trying to set extra flags for {0} while using in-memory compilation " - "database", - File); - return; - } - static_cast<DirectoryBasedGlobalCompilationDatabase *>(CDB.get()) - ->setExtraFlagsForFile(File, std::move(ExtraFlags)); -} - } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Nov 2 06:06:55 2018 @@ -121,12 +121,6 @@ private: 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<std::string> ExtraFlags); - /// Returns a CDB that should be used to get compile commands for the /// current instance of ClangdLSPServer. GlobalCompilationDatabase &getCDB() { return *CDB; } Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original) +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Fri Nov 2 06:06:55 2018 @@ -41,45 +41,14 @@ Optional<tooling::CompileCommand> 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<std::string> ExtraFlags) { - std::lock_guard<std::mutex> Lock(Mutex); - ExtraFlagsForFile[File] = std::move(ExtraFlags); -} - -void DirectoryBasedGlobalCompilationDatabase::addExtraFlags( - PathRef File, tooling::CompileCommand &C) const { - std::lock_guard<std::mutex> 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 Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original) +++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Fri Nov 2 06:06:55 2018 @@ -59,16 +59,9 @@ public: llvm::Optional<tooling::CompileCommand> 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<std::string> 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,8 +69,6 @@ private: mutable llvm::StringMap<std::unique_ptr<clang::tooling::CompilationDatabase>> CompilationDatabases; - /// Stores extra flags per file. - llvm::StringMap<std::vector<std::string>> ExtraFlagsForFile; /// Used for command argument pointing to folder where compile_commands.json /// is located. llvm::Optional<Path> CompileCommandsDir; Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Fri Nov 2 06:06:55 2018 @@ -119,14 +119,6 @@ bool fromJSON(const json::Value &Params, 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, 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) { Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=345969&r1=345968&r2=345969&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Fri Nov 2 06:06:55 2018 @@ -175,11 +175,6 @@ struct Location { llvm::json::Value toJSON(const Location &); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Location &); -struct Metadata { - std::vector<std::string> 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 @@ bool fromJSON(const llvm::json::Value &, struct DidOpenTextDocumentParams { /// The document that was opened. TextDocumentItem textDocument; - - /// Extension storing per-file metadata, such as compilation flags. - llvm::Optional<Metadata> metadata; }; bool fromJSON(const llvm::json::Value &, DidOpenTextDocumentParams &); Removed: clang-tools-extra/trunk/test/clangd/extra-flags.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/extra-flags.test?rev=345968&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clangd/extra-flags.test (original) +++ clang-tools-extra/trunk/test/clangd/extra-flags.test (removed) @@ -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"} - - _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
