[clang-tools-extra] r345969 - [clangd] Remove didOpen extraFlags extension.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 2 06:06:55 PDT 2018


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"}
-
-




More information about the cfe-commits mailing list