[clangd] Remove didOpen extraFlags extension.

[sammccall]

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

[ilya-biryukov]

LGTM! Thanks for cleaning this up.

[sammccall]

BTW I'm holding off submitting this until we decide the fate of D53688.
If we're going to add that extension, I'd like to land the two patches together so the former user of this extension can rely on *one* of them being available.

clangd/ClangdLSPServer.h

public:
     /// 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<std::string> 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<GlobalCompilationDatabase> CDB,
                   bool IsDirectoryBased)
         : CDB(std::move(CDB)), IsDirectoryBased(IsDirectoryBased) {}

clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onShutdown(const ShutdownParams &Params,
   // Do essentially nothing, just say we're ready to exit.
   ShutdownRequestReceived = true;
   Reply(nullptr);
 }
 
 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;
 
   DraftMgr.addDraft(File, Contents);
   Server->addDocument(File, Contents, WantDiagnostics::Yes);
 }
 
 void ClangdLSPServer::onDocumentDidChange(
elog("Trying to set compile command for {0} while using directory-based "
          "compilation database",
          File);
     return false;
   }
   return static_cast<InMemoryCompilationDb *>(CDB.get())
       ->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

clangd/GlobalCompilationDatabase.h

DirectoryBasedGlobalCompilationDatabase(
       llvm::Optional<Path> CompileCommandsDir);
   ~DirectoryBasedGlobalCompilationDatabase() override;
 
   /// Scans File's parents looking for compilation databases.
   /// Any extra flags will be added.
   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
   /// directories).
   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;
 };
 
 /// 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 {

clangd/GlobalCompilationDatabase.cpp

 
 DirectoryBasedGlobalCompilationDatabase::
     ~DirectoryBasedGlobalCompilationDatabase() = default;
 
 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
   auto CachedIt = CompilationDatabases.find(Dir);
   if (CachedIt != CompilationDatabases.end())
     return CachedIt->second.get();
   std::string Error = "";
   auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);

clangd/Protocol.h

struct Location {
 
   friend bool operator<(const Location &LHS, const Location &RHS) {
     return std::tie(LHS.uri, LHS.range) < std::tie(RHS.uri, RHS.range);
   }
 };
 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.
   Range range;
 
   /// The string to be inserted. For delete operations use an
   /// empty string.
   std::string newText;
struct InitializeParams {
   /// User-provided initialization options.
   InitializationOptions initializationOptions;
 };
 bool fromJSON(const llvm::json::Value &, InitializeParams &);
 
 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 &);
 
 struct DidCloseTextDocumentParams {
   /// The document that was closed.
   TextDocumentIdentifier textDocument;
 };
 bool fromJSON(const llvm::json::Value &, DidCloseTextDocumentParams &);

clangd/Protocol.cpp

 }
 
 bool fromJSON(const json::Value &Params, TextDocumentItem &R) {
   json::ObjectMapper O(Params);
   return O && O.map("uri", R.uri) && O.map("languageId", R.languageId) &&
          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);
 }
 
 json::Value toJSON(const TextEdit &P) {
   return json::Object{
       {"range", P.range},
bool fromJSON(const json::Value &Params, InitializeParams &R) {
   O.map("capabilities", R.capabilities);
   O.map("trace", R.trace);
   O.map("initializationOptions", R.initializationOptions);
   return true;
 }
 
 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) {
   json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument);
 }
 
 bool fromJSON(const json::Value &Params, DidChangeTextDocumentParams &R) {

test/clangd/extra-flags.test

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