[PATCH] D53641: [clangd] Remove didOpen extraFlags extension.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345969: [clangd] Remove didOpen extraFlags extension.
(authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53641?vs=170858&id=172341#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53641
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/extra-flags.test
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -366,8 +366,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;
@@ -811,17 +809,5 @@
->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.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
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/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -121,12 +121,6 @@
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 *CD
[PATCH] D53641: [clangd] Remove didOpen extraFlags extension.
sammccall added a comment. BTW I'm holding off submitting this until we decide the fate of https://reviews.llvm.org/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. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53641: [clangd] Remove didOpen extraFlags extension.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Thanks for cleaning this up. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53641: [clangd] Remove didOpen extraFlags extension.
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
This was added in https://reviews.llvm.org/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
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53641
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/extra-flags.test
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: 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/GlobalCompi
