[PATCH] D42517: [clangd] RFC: Pass Context implicitly using TLS.
sammccall updated this revision to Diff 131564. sammccall added a comment. Rebase on the Span changes that landed as r323511 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42517 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Context.cpp clangd/Context.h clangd/GlobalCompilationDatabase.cpp clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/Trace.cpp clangd/Trace.h clangd/XRefs.cpp clangd/XRefs.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/index/Merge.cpp unittests/clangd/FileIndexTests.cpp unittests/clangd/IndexTests.cpp unittests/clangd/TraceTests.cpp Index: unittests/clangd/TraceTests.cpp === --- unittests/clangd/TraceTests.cpp +++ unittests/clangd/TraceTests.cpp @@ -78,8 +78,8 @@ auto JSONTracer = trace::createJSONTracer(OS); trace::Session Session(*JSONTracer); { - trace::Span Tracer(Context::empty(), "A"); - trace::log(Tracer.Ctx, "B"); + trace::Span Tracer("A"); + trace::log("B"); } } Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -92,8 +92,7 @@ std::vector match(const SymbolIndex &I, const FuzzyFindRequest &Req) { std::vector Matches; - auto Ctx = Context::empty(); - I.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) { + I.fuzzyFind(Req, [&](const Symbol &Sym) { Matches.push_back( (Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str()); }); Index: unittests/clangd/FileIndexTests.cpp === --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -76,35 +76,32 @@ std::vector match(const SymbolIndex &I, const FuzzyFindRequest &Req) { std::vector Matches; - auto Ctx = Context::empty(); - I.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) { + I.fuzzyFind(Req, [&](const Symbol &Sym) { Matches.push_back((Sym.Scope + Sym.Name).str()); }); return Matches; } /// Create an ParsedAST for \p Code. Returns None if \p Code is empty. llvm::Optional build(std::string Path, llvm::StringRef Code) { - Context Ctx = Context::empty(); if (Code.empty()) return llvm::None; const char *Args[] = {"clang", "-xc++", Path.c_str()}; auto CI = createInvocationFromCommandLine(Args); auto Buf = llvm::MemoryBuffer::getMemBuffer(Code); - auto AST = ParsedAST::Build(Ctx, std::move(CI), nullptr, std::move(Buf), + auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf), std::make_shared(), vfs::getRealFileSystem()); assert(AST.hasValue()); return std::move(*AST); } TEST(FileIndexTest, IndexAST) { FileIndex M; - auto Ctx = Context::empty(); M.update( - Ctx, "f1", + "f1", build("f1", "namespace ns { void f() {} class X {}; }").getPointer()); FuzzyFindRequest Req; @@ -115,9 +112,8 @@ TEST(FileIndexTest, NoLocal) { FileIndex M; - auto Ctx = Context::empty(); M.update( - Ctx, "f1", + "f1", build("f1", "namespace ns { void f() { int local = 0; } class X {}; }") .getPointer()); @@ -128,12 +124,11 @@ TEST(FileIndexTest, IndexMultiASTAndDeduplicate) { FileIndex M; - auto Ctx = Context::empty(); M.update( - Ctx, "f1", + "f1", build("f1", "namespace ns { void f() {} class X {}; }").getPointer()); M.update( - Ctx, "f2", + "f2", build("f2", "namespace ns { void ff() {} class X {}; }").getPointer()); FuzzyFindRequest Req; @@ -144,31 +139,28 @@ TEST(FileIndexTest, RemoveAST) { FileIndex M; - auto Ctx = Context::empty(); M.update( - Ctx, "f1", + "f1", build("f1", "namespace ns { void f() {} class X {}; }").getPointer()); FuzzyFindRequest Req; Req.Query = ""; Req.Scopes = {"ns::"}; EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X")); - M.update(Ctx, "f1", nullptr); + M.update("f1", nullptr); EXPECT_THAT(match(M, Req), UnorderedElementsAre()); } TEST(FileIndexTest, RemoveNonExisting) { FileIndex M; - auto Ctx = Context::empty(); - M.update(Ctx, "no", nullptr); + M.update("no", nullptr); EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre()); } TEST(FileIndexTest, IgnoreClassMembers) { FileIndex M; - auto Ctx = Context::empty(); - M.update(Ctx, "f1", + M.update("f1", build("f1", "c
[PATCH] D42517: [clangd] RFC: Pass Context implicitly using TLS.
ilya-biryukov added a comment. LG. Let's move the APIs to callbacks and start passing Contexts everywhere. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42517: [clangd] RFC: Pass Context implicitly using TLS.
sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek. This is mostly to show what it would look like. I think it's pretty nice! The lit tests pass, but the unit tests don't build because I ripped out some std::future APIs and just return void, so the tests have no way to sync. These probably need to be converted to callbacks first (callback APIs play nicely with implicit contexts). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42517 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Context.cpp clangd/Context.h clangd/Function.h clangd/GlobalCompilationDatabase.cpp clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Logger.cpp clangd/Logger.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/Trace.cpp clangd/Trace.h clangd/XRefs.cpp clangd/XRefs.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/MemIndex.h clangd/index/Merge.cpp unittests/clangd/TraceTests.cpp Index: unittests/clangd/TraceTests.cpp === --- unittests/clangd/TraceTests.cpp +++ unittests/clangd/TraceTests.cpp @@ -78,8 +78,8 @@ auto JSONTracer = trace::createJSONTracer(OS); trace::Session Session(*JSONTracer); { - trace::Span S(Context::empty(), "A"); - trace::log(Context::empty(), "B"); + Context Ctx = trace::span(Context::empty(), "A"); + trace::log(Ctx, "B"); } } Index: clangd/index/Merge.cpp === --- clangd/index/Merge.cpp +++ clangd/index/Merge.cpp @@ -24,7 +24,7 @@ // - find the generating file from each Symbol which is Static-only // - ask Dynamic if it has that file (needs new SymbolIndex method) // - if so, drop the Symbol. - bool fuzzyFind(const Context &Ctx, const FuzzyFindRequest &Req, + bool fuzzyFind(const FuzzyFindRequest &Req, function_ref Callback) const override { // We can't step through both sources in parallel. So: // 1) query all dynamic symbols, slurping results into a slab @@ -34,13 +34,12 @@ // 3) now yield all the dynamic symbols we haven't processed. bool More = false; // We'll be incomplete if either source was. SymbolSlab::Builder DynB; - More |= - Dynamic->fuzzyFind(Ctx, Req, [&](const Symbol &S) { DynB.insert(S); }); + More |= Dynamic->fuzzyFind(Req, [&](const Symbol &S) { DynB.insert(S); }); SymbolSlab Dyn = std::move(DynB).build(); DenseSet SeenDynamicSymbols; Symbol::Details Scratch; - More |= Static->fuzzyFind(Ctx, Req, [&](const Symbol &S) { + More |= Static->fuzzyFind(Req, [&](const Symbol &S) { auto DynS = Dyn.find(S.ID); if (DynS == Dyn.end()) return Callback(S); Index: clangd/index/MemIndex.h === --- clangd/index/MemIndex.h +++ clangd/index/MemIndex.h @@ -28,7 +28,7 @@ static std::unique_ptr build(SymbolSlab Slab); bool - fuzzyFind(const Context &Ctx, const FuzzyFindRequest &Req, + fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const override; private: Index: clangd/index/MemIndex.cpp === --- clangd/index/MemIndex.cpp +++ clangd/index/MemIndex.cpp @@ -29,7 +29,7 @@ } bool MemIndex::fuzzyFind( -const Context &Ctx, const FuzzyFindRequest &Req, +const FuzzyFindRequest &Req, llvm::function_ref Callback) const { assert(!StringRef(Req.Query).contains("::") && "There must be no :: in query."); Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -10,7 +10,6 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H -#include "../Context.h" #include "clang/Index/IndexSymbol.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -244,7 +243,7 @@ /// Returns true if the result list is complete, false if it was truncated due /// to MaxCandidateCount virtual bool - fuzzyFind(const Context &Ctx, const FuzzyFindRequest &Req, + fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const = 0; // FIXME: add interfaces for more index use cases: Index: clangd/index/FileIndex.h === --- clangd/index/FileIndex.h +++ clangd/index/FileIndex.h @@ -17,7 +17,6 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H #include "..