This revision was automatically updated to reflect the committed changes.
Closed by commit rL305298: [clangd] Store references instead of unique_ptrs in
ClangdServer. (authored by ibiryukov).
Changed prior to commit:
https://reviews.llvm.org/D34148?vs=102333&id=102344#toc
Repository:
rL LLVM
https://reviews.llvm.org/D34148
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -214,11 +214,6 @@
return replacePtrsInDump(DumpWithMemLocs);
}
-template <class T>
-std::unique_ptr<T> getAndMove(std::unique_ptr<T> Ptr, T *&Output) {
- Output = Ptr.get();
- return Ptr;
-}
} // namespace
class ClangdVFSTest : public ::testing::Test {
@@ -243,22 +238,19 @@
PathRef SourceFileRelPath, StringRef SourceContents,
std::vector<std::pair<PathRef, StringRef>> ExtraFiles = {},
bool ExpectErrors = false) {
- MockFSProvider *FS;
- ErrorCheckingDiagConsumer *DiagConsumer;
- ClangdServer Server(
- llvm::make_unique<MockCompilationDatabase>(),
- getAndMove(llvm::make_unique<ErrorCheckingDiagConsumer>(),
- DiagConsumer),
- getAndMove(llvm::make_unique<MockFSProvider>(), FS),
- /*RunSynchronously=*/false);
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*RunSynchronously=*/false);
for (const auto &FileWithContents : ExtraFiles)
- FS->Files[getVirtualTestFilePath(FileWithContents.first)] =
+ FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
FileWithContents.second;
auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath);
Server.addDocument(SourceFilename, SourceContents);
auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
- EXPECT_EQ(ExpectErrors, DiagConsumer->hadErrorInLastDiags());
+ EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags());
return Result;
}
};
@@ -299,13 +291,11 @@
}
TEST_F(ClangdVFSTest, Reparse) {
- MockFSProvider *FS;
- ErrorCheckingDiagConsumer *DiagConsumer;
- ClangdServer Server(
- llvm::make_unique<MockCompilationDatabase>(),
- getAndMove(llvm::make_unique<ErrorCheckingDiagConsumer>(), DiagConsumer),
- getAndMove(llvm::make_unique<MockFSProvider>(), FS),
- /*RunSynchronously=*/false);
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*RunSynchronously=*/false);
const auto SourceContents = R"cpp(
#include "foo.h"
@@ -315,34 +305,32 @@
auto FooCpp = getVirtualTestFilePath("foo.cpp");
auto FooH = getVirtualTestFilePath("foo.h");
- FS->Files[FooH] = "int a;";
- FS->Files[FooCpp] = SourceContents;
+ FS.Files[FooH] = "int a;";
+ FS.Files[FooCpp] = SourceContents;
Server.addDocument(FooCpp, SourceContents);
auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
Server.addDocument(FooCpp, "");
auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
Server.addDocument(FooCpp, SourceContents);
auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
EXPECT_EQ(DumpParse1, DumpParse2);
EXPECT_NE(DumpParse1, DumpParseEmpty);
}
TEST_F(ClangdVFSTest, ReparseOnHeaderChange) {
- MockFSProvider *FS;
- ErrorCheckingDiagConsumer *DiagConsumer;
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
- ClangdServer Server(
- llvm::make_unique<MockCompilationDatabase>(),
- getAndMove(llvm::make_unique<ErrorCheckingDiagConsumer>(), DiagConsumer),
- getAndMove(llvm::make_unique<MockFSProvider>(), FS),
- /*RunSynchronously=*/false);
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*RunSynchronously=*/false);
const auto SourceContents = R"cpp(
#include "foo.h"
@@ -352,50 +340,47 @@
auto FooCpp = getVirtualTestFilePath("foo.cpp");
auto FooH = getVirtualTestFilePath("foo.h");
- FS->Files[FooH] = "int a;";
- FS->Files[FooCpp] = SourceContents;
+ FS.Files[FooH] = "int a;";
+ FS.Files[FooCpp] = SourceContents;
Server.addDocument(FooCpp, SourceContents);
auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
- FS->Files[FooH] = "";
+ FS.Files[FooH] = "";
Server.forceReparse(FooCpp);
auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_TRUE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- FS->Files[FooH] = "int a;";
+ FS.Files[FooH] = "int a;";
Server.forceReparse(FooCpp);
auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
- EXPECT_FALSE(DiagConsumer->hadErrorInLastDiags());
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
EXPECT_EQ(DumpParse1, DumpParse2);
EXPECT_NE(DumpParse1, DumpParseDifferent);
}
TEST_F(ClangdVFSTest, CheckVersions) {
- MockFSProvider *FS;
- ErrorCheckingDiagConsumer *DiagConsumer;
-
- ClangdServer Server(
- llvm::make_unique<MockCompilationDatabase>(),
- getAndMove(llvm::make_unique<ErrorCheckingDiagConsumer>(), DiagConsumer),
- getAndMove(llvm::make_unique<MockFSProvider>(), FS),
- /*RunSynchronously=*/true);
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*RunSynchronously=*/true);
auto FooCpp = getVirtualTestFilePath("foo.cpp");
const auto SourceContents = "int a;";
- FS->Files[FooCpp] = SourceContents;
- FS->Tag = "123";
+ FS.Files[FooCpp] = SourceContents;
+ FS.Tag = "123";
Server.addDocument(FooCpp, SourceContents);
- EXPECT_EQ(DiagConsumer->lastVFSTag(), FS->Tag);
- EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS->Tag);
+ EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
+ EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
- FS->Tag = "321";
+ FS.Tag = "321";
Server.addDocument(FooCpp, SourceContents);
- EXPECT_EQ(DiagConsumer->lastVFSTag(), FS->Tag);
- EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS->Tag);
+ EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag);
+ EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag);
}
class ClangdCompletionTest : public ClangdVFSTest {
@@ -410,11 +395,11 @@
};
TEST_F(ClangdCompletionTest, CheckContentsOverride) {
- MockFSProvider *FS;
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
- ClangdServer Server(llvm::make_unique<MockCompilationDatabase>(),
- llvm::make_unique<ErrorCheckingDiagConsumer>(),
- getAndMove(llvm::make_unique<MockFSProvider>(), FS),
+ ClangdServer Server(CDB, DiagConsumer, FS,
/*RunSynchronously=*/false);
auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -433,7 +418,7 @@
// string literal of the SourceContents starts with a newline(it's easy to
// miss).
Position CompletePos = {2, 8};
- FS->Files[FooCpp] = SourceContents;
+ FS.Files[FooCpp] = SourceContents;
Server.addDocument(FooCpp, SourceContents);
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -136,10 +136,9 @@
/// diagnostics for tracked files).
class ClangdServer {
public:
- ClangdServer(std::unique_ptr<GlobalCompilationDatabase> CDB,
- std::unique_ptr<DiagnosticsConsumer> DiagConsumer,
- std::unique_ptr<FileSystemProvider> FSProvider,
- bool RunSynchronously);
+ ClangdServer(GlobalCompilationDatabase &CDB,
+ DiagnosticsConsumer &DiagConsumer,
+ FileSystemProvider &FSProvider, bool RunSynchronously);
/// Add a \p File to the list of tracked C++ files or update the contents if
/// \p File is already tracked. Also schedules parsing of the AST for it on a
@@ -181,9 +180,9 @@
std::string dumpAST(PathRef File);
private:
- std::unique_ptr<GlobalCompilationDatabase> CDB;
- std::unique_ptr<DiagnosticsConsumer> DiagConsumer;
- std::unique_ptr<FileSystemProvider> FSProvider;
+ GlobalCompilationDatabase &CDB;
+ DiagnosticsConsumer &DiagConsumer;
+ FileSystemProvider &FSProvider;
DraftStore DraftMgr;
ClangdUnitStore Units;
std::shared_ptr<PCHContainerOperations> PCHs;
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -138,12 +138,11 @@
RequestCV.notify_one();
}
-ClangdServer::ClangdServer(std::unique_ptr<GlobalCompilationDatabase> CDB,
- std::unique_ptr<DiagnosticsConsumer> DiagConsumer,
- std::unique_ptr<FileSystemProvider> FSProvider,
+ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
+ DiagnosticsConsumer &DiagConsumer,
+ FileSystemProvider &FSProvider,
bool RunSynchronously)
- : CDB(std::move(CDB)), DiagConsumer(std::move(DiagConsumer)),
- FSProvider(std::move(FSProvider)),
+ : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
PCHs(std::make_shared<PCHContainerOperations>()),
WorkScheduler(RunSynchronously) {}
@@ -157,11 +156,11 @@
assert(FileContents.Draft &&
"No contents inside a file that was scheduled for reparse");
- auto TaggedFS = FSProvider->getTaggedFileSystem();
+ auto TaggedFS = FSProvider.getTaggedFileSystem();
Units.runOnUnit(
- FileStr, *FileContents.Draft, *CDB, PCHs, TaggedFS.Value,
+ FileStr, *FileContents.Draft, CDB, PCHs, TaggedFS.Value,
[&](ClangdUnit const &Unit) {
- DiagConsumer->onDiagnosticsReady(
+ DiagConsumer.onDiagnosticsReady(
FileStr, make_tagged(Unit.getLocalDiagnostics(), TaggedFS.Tag));
});
});
@@ -198,11 +197,11 @@
}
std::vector<CompletionItem> Result;
- auto TaggedFS = FSProvider->getTaggedFileSystem();
+ auto TaggedFS = FSProvider.getTaggedFileSystem();
// It would be nice to use runOnUnitWithoutReparse here, but we can't
// guarantee the correctness of code completion cache here if we don't do the
// reparse.
- Units.runOnUnit(File, *OverridenContents, *CDB, PCHs, TaggedFS.Value,
+ Units.runOnUnit(File, *OverridenContents, CDB, PCHs, TaggedFS.Value,
[&](ClangdUnit &Unit) {
Result = Unit.codeComplete(*OverridenContents, Pos,
TaggedFS.Value);
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H
#include "ClangdServer.h"
+#include "GlobalCompilationDatabase.h"
#include "Path.h"
#include "Protocol.h"
#include "clang/Tooling/Core/Replacement.h"
@@ -34,7 +35,17 @@
private:
class LSPProtocolCallbacks;
- class LSPDiagnosticsConsumer;
+ class LSPDiagnosticsConsumer : public DiagnosticsConsumer {
+ public:
+ LSPDiagnosticsConsumer(ClangdLSPServer &Server);
+
+ virtual void
+ onDiagnosticsReady(PathRef File,
+ Tagged<std::vector<DiagWithFixIts>> Diagnostics);
+
+ private:
+ ClangdLSPServer &Server;
+ };
std::vector<clang::tooling::Replacement>
getFixIts(StringRef File, const clangd::Diagnostic &D);
@@ -56,6 +67,13 @@
DiagnosticToReplacementMap;
/// Caches FixIts per file and diagnostics
llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
+
+ // Various ClangdServer parameters go here. It's important they're created
+ // before ClangdServer.
+ DirectoryBasedGlobalCompilationDatabase CDB;
+ LSPDiagnosticsConsumer DiagConsumer;
+ RealFileSystemProvider FSProvider;
+
// Server must be the last member of the class to allow its destructor to exit
// the worker thread that may otherwise run an async callback on partially
// destructed instance of ClangdLSPServer.
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -38,18 +38,14 @@
} // namespace
-class ClangdLSPServer::LSPDiagnosticsConsumer : public DiagnosticsConsumer {
-public:
- LSPDiagnosticsConsumer(ClangdLSPServer &Server) : Server(Server) {}
-
- virtual void onDiagnosticsReady(PathRef File,
- Tagged<std::vector<DiagWithFixIts>> Diagnostics) {
- Server.consumeDiagnostics(File, Diagnostics.Value);
- }
-
-private:
- ClangdLSPServer &Server;
-};
+ClangdLSPServer::LSPDiagnosticsConsumer::LSPDiagnosticsConsumer(
+ ClangdLSPServer &Server)
+ : Server(Server) {}
+
+void ClangdLSPServer::LSPDiagnosticsConsumer::onDiagnosticsReady(
+ PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) {
+ Server.consumeDiagnostics(File, Diagnostics.Value);
+}
class ClangdLSPServer::LSPProtocolCallbacks : public ProtocolCallbacks {
public:
@@ -196,11 +192,8 @@
}
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, bool RunSynchronously)
- : Out(Out),
- Server(llvm::make_unique<DirectoryBasedGlobalCompilationDatabase>(),
- llvm::make_unique<LSPDiagnosticsConsumer>(*this),
- llvm::make_unique<RealFileSystemProvider>(),
- RunSynchronously) {}
+ : Out(Out), DiagConsumer(*this),
+ Server(CDB, DiagConsumer, FSProvider, RunSynchronously) {}
void ClangdLSPServer::run(std::istream &In) {
assert(!IsDone && "Run was called before");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
