<div dir="ltr"><div dir="ltr">Sorry! r343800 should fix.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 4, 2018 at 6:33 PM <<a href="mailto:douglas.yung@sony.com">douglas.yung@sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Sam,<br>
<br>
Your change is causing a build failure on one of the build bots. Can you take a look?<br>
<br>
<a href="http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/33139" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/33139</a><br>
<br>
FAILED: tools/clang/tools/extra/unittests/clangd/CMakeFiles/ClangdTests.dir/TestTU.cpp.o <br>
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/unittests/clangd -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/unittests/clangd -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/include -Itools/clang/include -Iinclude -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/include -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/utils/unittest/googletest/include -I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -UNDEBUG -Wno-variadic-macros -fno-exceptions -fno-rtti -MD -MT tools/clang/tools/extra/unittests/clangd/CMakeFiles/ClangdTests.dir/TestTU.cpp.o -MF tools/clang/tools/extra/unittests/clangd/CMakeFiles/ClangdTests.dir/TestTU.cpp.o.d -o tools/clang/tools/extra/unittests/clangd/CMakeFiles/ClangdTests.dir/TestTU.cpp.o -c /home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/unittests/clangd/TestTU.cpp<br>
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/unittests/clangd/TestTU.cpp: In member function ‘std::unique_ptr<clang::clangd::SymbolIndex> clang::clangd::TestTU::index() const’:<br>
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/unittests/clangd/TestTU.cpp:56:10: error: could not convert ‘Idx’ from ‘std::unique_ptr<clang::clangd::FileIndex>’ to ‘std::unique_ptr<clang::clangd::SymbolIndex>’<br>
return Idx;<br>
^<br>
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/unittests/clangd/TestTU.cpp:57:1: warning: control reaches end of non-void function [-Wreturn-type]<br>
}<br>
^<br>
<br>
Douglas Yung<br>
<br>
> -----Original Message-----<br>
> From: cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>] On Behalf<br>
> Of Sam McCall via cfe-commits<br>
> Sent: Thursday, October 04, 2018 7:20<br>
> To: <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> Subject: [clang-tools-extra] r343780 - [clangd] expose MergedIndex<br>
> class<br>
> <br>
> Author: sammccall<br>
> Date: Thu Oct 4 07:20:22 2018<br>
> New Revision: 343780<br>
> <br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=343780&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=343780&view=rev</a><br>
> Log:<br>
> [clangd] expose MergedIndex class<br>
> <br>
> Summary:<br>
> This allows inheriting from it, so index() can ga away and allowing<br>
> TestTU::index) to be fixed.<br>
> <br>
> Reviewers: ioeric<br>
> <br>
> Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-<br>
> commits<br>
> <br>
> Differential Revision: <a href="https://reviews.llvm.org/D52250" rel="noreferrer" target="_blank">https://reviews.llvm.org/D52250</a><br>
> <br>
> Modified:<br>
> clang-tools-extra/trunk/clangd/ClangdServer.cpp<br>
> clang-tools-extra/trunk/clangd/ClangdServer.h<br>
> clang-tools-extra/trunk/clangd/index/FileIndex.cpp<br>
> clang-tools-extra/trunk/clangd/index/FileIndex.h<br>
> clang-tools-extra/trunk/clangd/index/Merge.cpp<br>
> clang-tools-extra/trunk/clangd/index/Merge.h<br>
> clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp<br>
> clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp<br>
> clang-tools-extra/trunk/unittests/clangd/TestTU.cpp<br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/ClangdServer.cpp?rev=343780&r1=343779&r2=343780&view<br>
> =diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)<br>
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct 4 07:20:22<br>
> 2018<br>
> @@ -117,20 +117,17 @@ ClangdServer::ClangdServer(GlobalCompila<br>
> : nullptr,<br>
> Opts.UpdateDebounce, Opts.RetentionPolicy) {<br>
> if (DynamicIdx && Opts.StaticIndex) {<br>
> - MergedIndex = mergeIndex(&DynamicIdx->index(), Opts.StaticIndex);<br>
> - Index = MergedIndex.get();<br>
> + MergedIdx =<br>
> + llvm::make_unique<MergedIndex>(DynamicIdx.get(),<br>
> Opts.StaticIndex);<br>
> + Index = MergedIdx.get();<br>
> } else if (DynamicIdx)<br>
> - Index = &DynamicIdx->index();<br>
> + Index = DynamicIdx.get();<br>
> else if (Opts.StaticIndex)<br>
> Index = Opts.StaticIndex;<br>
> else<br>
> Index = nullptr;<br>
> }<br>
> <br>
> -const SymbolIndex *ClangdServer::dynamicIndex() const {<br>
> - return DynamicIdx ? &DynamicIdx->index() : nullptr;<br>
> -}<br>
> -<br>
> void ClangdServer::setRootPath(PathRef RootPath) {<br>
> auto FS = FSProvider.getFileSystem();<br>
> auto Status = FS->status(RootPath);<br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/ClangdServer.h?rev=343780&r1=343779&r2=343780&view=d<br>
> iff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)<br>
> +++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Oct 4 07:20:22<br>
> 2018<br>
> @@ -206,7 +206,7 @@ public:<br>
> <br>
> /// Returns the active dynamic index if one was built.<br>
> /// This can be useful for testing, debugging, or observing memory<br>
> usage.<br>
> - const SymbolIndex *dynamicIndex() const;<br>
> + const SymbolIndex *dynamicIndex() const { return DynamicIdx.get(); }<br>
> <br>
> // Blocks the main thread until the server is idle. Only for use in<br>
> tests.<br>
> // Returns false if the timeout expires.<br>
> @@ -244,7 +244,7 @@ private:<br>
> // If present, an index of symbols in open files. Read via *Index.<br>
> std::unique_ptr<FileIndex> DynamicIdx;<br>
> // If present, storage for the merged static/dynamic index. Read via<br>
> *Index.<br>
> - std::unique_ptr<SymbolIndex> MergedIndex;<br>
> + std::unique_ptr<SymbolIndex> MergedIdx;<br>
> <br>
> // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)<br>
> llvm::StringMap<llvm::Optional<FuzzyFindRequest>><br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/index/FileIndex.cpp?rev=343780&r1=343779&r2=343780&v<br>
> iew=diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)<br>
> +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Thu Oct 4<br>
> 07:20:22 2018<br>
> @@ -152,10 +152,10 @@ std::unique_ptr<SymbolIndex> FileSymbols<br>
> }<br>
> <br>
> FileIndex::FileIndex(std::vector<std::string> URISchemes)<br>
> - : URISchemes(std::move(URISchemes)),<br>
> + : MergedIndex(&MainFileIndex, &PreambleIndex),<br>
> + URISchemes(std::move(URISchemes)),<br>
> PreambleIndex(PreambleSymbols.buildMemIndex()),<br>
> - MainFileIndex(MainFileSymbols.buildMemIndex()),<br>
> - MergedIndex(mergeIndex(&MainFileIndex, &PreambleIndex)) {}<br>
> + MainFileIndex(MainFileSymbols.buildMemIndex()) {}<br>
> <br>
> void FileIndex::updatePreamble(PathRef Path, ASTContext &AST,<br>
> std::shared_ptr<Preprocessor> PP) {<br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/index/FileIndex.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/index/FileIndex.h?rev=343780&r1=343779&r2=343780&vie<br>
> w=diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/index/FileIndex.h (original)<br>
> +++ clang-tools-extra/trunk/clangd/index/FileIndex.h Thu Oct 4<br>
> 07:20:22 2018<br>
> @@ -19,6 +19,7 @@<br>
> #include "ClangdUnit.h"<br>
> #include "Index.h"<br>
> #include "MemIndex.h"<br>
> +#include "Merge.h"<br>
> #include "clang/Lex/Preprocessor.h"<br>
> #include <memory><br>
> <br>
> @@ -59,15 +60,12 @@ private:<br>
> <br>
> /// This manages symbols from files and an in-memory index on all<br>
> symbols.<br>
> /// FIXME: Expose an interface to remove files that are closed.<br>
> -class FileIndex {<br>
> +class FileIndex : public MergedIndex {<br>
> public:<br>
> /// If URISchemes is empty, the default schemes in SymbolCollector<br>
> will be<br>
> /// used.<br>
> FileIndex(std::vector<std::string> URISchemes = {});<br>
> <br>
> - // Presents a merged view of the supplied main-file and preamble<br>
> ASTs.<br>
> - const SymbolIndex &index() const { return *MergedIndex; }<br>
> -<br>
> /// Update preamble symbols of file \p Path with all declarations in<br>
> \p AST<br>
> /// and macros in \p PP.<br>
> void updatePreamble(PathRef Path, ASTContext &AST,<br>
> @@ -102,8 +100,6 @@ private:<br>
> // (Note that symbols *only* in the main file are not indexed).<br>
> FileSymbols MainFileSymbols;<br>
> SwapIndex MainFileIndex;<br>
> -<br>
> - std::unique_ptr<SymbolIndex> MergedIndex; // Merge preamble and<br>
> main index.<br>
> };<br>
> <br>
> /// Retrieves symbols and refs of local top level decls in \p AST<br>
> (i.e.<br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/index/Merge.cpp?rev=343780&r1=343779&r2=343780&view=<br>
> diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)<br>
> +++ clang-tools-extra/trunk/clangd/index/Merge.cpp Thu Oct 4 07:20:22<br>
> 2018<br>
> @@ -17,111 +17,96 @@<br>
> <br>
> namespace clang {<br>
> namespace clangd {<br>
> -namespace {<br>
> <br>
> using namespace llvm;<br>
> <br>
> -class MergedIndex : public SymbolIndex {<br>
> - public:<br>
> - MergedIndex(const SymbolIndex *Dynamic, const SymbolIndex *Static)<br>
> - : Dynamic(Dynamic), Static(Static) {}<br>
> -<br>
> - // FIXME: Deleted symbols in dirty files are still returned (from<br>
> Static).<br>
> - // To identify these eliminate these, we should:<br>
> - // - find the generating file from each Symbol which is<br>
> Static-only<br>
> - // - ask Dynamic if it has that file (needs new<br>
> SymbolIndex method)<br>
> - // - if so, drop the Symbol.<br>
> - bool fuzzyFind(const FuzzyFindRequest &Req,<br>
> - function_ref<void(const Symbol &)> Callback) const<br>
> override {<br>
> - // We can't step through both sources in parallel. So:<br>
> - // 1) query all dynamic symbols, slurping results into a slab<br>
> - // 2) query the static symbols, for each one:<br>
> - // a) if it's not in the dynamic slab, yield it directly<br>
> - // b) if it's in the dynamic slab, merge it and yield the<br>
> result<br>
> - // 3) now yield all the dynamic symbols we haven't processed.<br>
> - trace::Span Tracer("MergedIndex fuzzyFind");<br>
> - bool More = false; // We'll be incomplete if either source was.<br>
> - SymbolSlab::Builder DynB;<br>
> - unsigned DynamicCount = 0;<br>
> - unsigned StaticCount = 0;<br>
> - unsigned MergedCount = 0;<br>
> - More |= Dynamic->fuzzyFind(Req, [&](const Symbol &S) {<br>
> - ++DynamicCount;<br>
> - DynB.insert(S);<br>
> - });<br>
> - SymbolSlab Dyn = std::move(DynB).build();<br>
> -<br>
> - DenseSet<SymbolID> SeenDynamicSymbols;<br>
> - More |= Static->fuzzyFind(Req, [&](const Symbol &S) {<br>
> - auto DynS = Dyn.find(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> - ++StaticCount;<br>
> - if (DynS == Dyn.end())<br>
> - return Callback(S);<br>
> - ++MergedCount;<br>
> - SeenDynamicSymbols.insert(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> - Callback(mergeSymbol(*DynS, S));<br>
> - });<br>
> - SPAN_ATTACH(Tracer, "dynamic", DynamicCount);<br>
> - SPAN_ATTACH(Tracer, "static", StaticCount);<br>
> - SPAN_ATTACH(Tracer, "merged", MergedCount);<br>
> - for (const Symbol &S : Dyn)<br>
> - if (!SeenDynamicSymbols.count(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>))<br>
> - Callback(S);<br>
> - return More;<br>
> - }<br>
> +// FIXME: Deleted symbols in dirty files are still returned (from<br>
> Static).<br>
> +// To identify these eliminate these, we should:<br>
> +// - find the generating file from each Symbol which is<br>
> Static-only<br>
> +// - ask Dynamic if it has that file (needs new SymbolIndex<br>
> method)<br>
> +// - if so, drop the Symbol.<br>
> +bool MergedIndex::fuzzyFind(const FuzzyFindRequest &Req,<br>
> + function_ref<void(const Symbol &)><br>
> Callback) const {<br>
> + // We can't step through both sources in parallel. So:<br>
> + // 1) query all dynamic symbols, slurping results into a slab<br>
> + // 2) query the static symbols, for each one:<br>
> + // a) if it's not in the dynamic slab, yield it directly<br>
> + // b) if it's in the dynamic slab, merge it and yield the result<br>
> + // 3) now yield all the dynamic symbols we haven't processed.<br>
> + trace::Span Tracer("MergedIndex fuzzyFind");<br>
> + bool More = false; // We'll be incomplete if either source was.<br>
> + SymbolSlab::Builder DynB;<br>
> + unsigned DynamicCount = 0;<br>
> + unsigned StaticCount = 0;<br>
> + unsigned MergedCount = 0;<br>
> + More |= Dynamic->fuzzyFind(Req, [&](const Symbol &S) {<br>
> + ++DynamicCount;<br>
> + DynB.insert(S);<br>
> + });<br>
> + SymbolSlab Dyn = std::move(DynB).build();<br>
> +<br>
> + DenseSet<SymbolID> SeenDynamicSymbols;<br>
> + More |= Static->fuzzyFind(Req, [&](const Symbol &S) {<br>
> + auto DynS = Dyn.find(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> + ++StaticCount;<br>
> + if (DynS == Dyn.end())<br>
> + return Callback(S);<br>
> + ++MergedCount;<br>
> + SeenDynamicSymbols.insert(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> + Callback(mergeSymbol(*DynS, S));<br>
> + });<br>
> + SPAN_ATTACH(Tracer, "dynamic", DynamicCount);<br>
> + SPAN_ATTACH(Tracer, "static", StaticCount);<br>
> + SPAN_ATTACH(Tracer, "merged", MergedCount);<br>
> + for (const Symbol &S : Dyn)<br>
> + if (!SeenDynamicSymbols.count(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>))<br>
> + Callback(S);<br>
> + return More;<br>
> +}<br>
> <br>
> - void<br>
> - lookup(const LookupRequest &Req,<br>
> - llvm::function_ref<void(const Symbol &)> Callback) const<br>
> override {<br>
> - trace::Span Tracer("MergedIndex lookup");<br>
> - SymbolSlab::Builder B;<br>
> -<br>
> - Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });<br>
> -<br>
> - auto RemainingIDs = Req.IDs;<br>
> - Static->lookup(Req, [&](const Symbol &S) {<br>
> - const Symbol *Sym = B.find(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> - RemainingIDs.erase(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> - if (!Sym)<br>
> - Callback(S);<br>
> - else<br>
> - Callback(mergeSymbol(*Sym, S));<br>
> - });<br>
> - for (const auto &ID : RemainingIDs)<br>
> - if (const Symbol *Sym = B.find(ID))<br>
> - Callback(*Sym);<br>
> - }<br>
> +void MergedIndex::lookup(<br>
> + const LookupRequest &Req,<br>
> + llvm::function_ref<void(const Symbol &)> Callback) const {<br>
> + trace::Span Tracer("MergedIndex lookup");<br>
> + SymbolSlab::Builder B;<br>
> +<br>
> + Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });<br>
> +<br>
> + auto RemainingIDs = Req.IDs;<br>
> + Static->lookup(Req, [&](const Symbol &S) {<br>
> + const Symbol *Sym = B.find(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> + RemainingIDs.erase(<a href="http://S.ID" rel="noreferrer" target="_blank">S.ID</a>);<br>
> + if (!Sym)<br>
> + Callback(S);<br>
> + else<br>
> + Callback(mergeSymbol(*Sym, S));<br>
> + });<br>
> + for (const auto &ID : RemainingIDs)<br>
> + if (const Symbol *Sym = B.find(ID))<br>
> + Callback(*Sym);<br>
> +}<br>
> <br>
> - void refs(const RefsRequest &Req,<br>
> - llvm::function_ref<void(const Ref &)> Callback) const<br>
> override {<br>
> - trace::Span Tracer("MergedIndex refs");<br>
> - // We don't want duplicated refs from the static/dynamic indexes,<br>
> - // and we can't reliably duplicate them because offsets may differ<br>
> slightly.<br>
> - // We consider the dynamic index authoritative and report all its<br>
> refs,<br>
> - // and only report static index refs from other files.<br>
> - //<br>
> - // FIXME: The heuristic fails if the dynamic index contains a<br>
> file, but all<br>
> - // refs were removed (we will report stale ones from the static<br>
> index).<br>
> - // Ultimately we should explicit check which index has the file<br>
> instead.<br>
> - llvm::StringSet<> DynamicIndexFileURIs;<br>
> - Dynamic->refs(Req, [&](const Ref &O) {<br>
> - DynamicIndexFileURIs.insert(O.Location.FileURI);<br>
> +void MergedIndex::refs(const RefsRequest &Req,<br>
> + llvm::function_ref<void(const Ref &)> Callback)<br>
> const {<br>
> + trace::Span Tracer("MergedIndex refs");<br>
> + // We don't want duplicated refs from the static/dynamic indexes,<br>
> + // and we can't reliably duplicate them because offsets may differ<br>
> slightly.<br>
> + // We consider the dynamic index authoritative and report all its<br>
> refs,<br>
> + // and only report static index refs from other files.<br>
> + //<br>
> + // FIXME: The heuristic fails if the dynamic index contains a file,<br>
> but all<br>
> + // refs were removed (we will report stale ones from the static<br>
> index).<br>
> + // Ultimately we should explicit check which index has the file<br>
> instead.<br>
> + llvm::StringSet<> DynamicIndexFileURIs;<br>
> + Dynamic->refs(Req, [&](const Ref &O) {<br>
> + DynamicIndexFileURIs.insert(O.Location.FileURI);<br>
> + Callback(O);<br>
> + });<br>
> + Static->refs(Req, [&](const Ref &O) {<br>
> + if (!DynamicIndexFileURIs.count(O.Location.FileURI))<br>
> Callback(O);<br>
> - });<br>
> - Static->refs(Req, [&](const Ref &O) {<br>
> - if (!DynamicIndexFileURIs.count(O.Location.FileURI))<br>
> - Callback(O);<br>
> - });<br>
> - }<br>
> -<br>
> - size_t estimateMemoryUsage() const override {<br>
> - return Dynamic->estimateMemoryUsage() + Static-<br>
> >estimateMemoryUsage();<br>
> - }<br>
> -<br>
> -private:<br>
> - const SymbolIndex *Dynamic, *Static;<br>
> -};<br>
> -} // namespace<br>
> + });<br>
> +}<br>
> <br>
> Symbol mergeSymbol(const Symbol &L, const Symbol &R) {<br>
> assert(<a href="http://L.ID" rel="noreferrer" target="_blank">L.ID</a> == <a href="http://R.ID" rel="noreferrer" target="_blank">R.ID</a>);<br>
> @@ -169,10 +154,5 @@ Symbol mergeSymbol(const Symbol &L, cons<br>
> return S;<br>
> }<br>
> <br>
> -std::unique_ptr<SymbolIndex> mergeIndex(const SymbolIndex *Dynamic,<br>
> - const SymbolIndex *Static) {<br>
> - return llvm::make_unique<MergedIndex>(Dynamic, Static);<br>
> -}<br>
> -<br>
> } // namespace clangd<br>
> } // namespace clang<br>
> <br>
> Modified: clang-tools-extra/trunk/clangd/index/Merge.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/clangd/index/Merge.h?rev=343780&r1=343779&r2=343780&view=di<br>
> ff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/clangd/index/Merge.h (original)<br>
> +++ clang-tools-extra/trunk/clangd/index/Merge.h Thu Oct 4 07:20:22<br>
> 2018<br>
> @@ -20,7 +20,7 @@ namespace clangd {<br>
> // Returned symbol may contain data owned by either source.<br>
> Symbol mergeSymbol(const Symbol &L, const Symbol &R);<br>
> <br>
> -// mergeIndex returns a composite index based on two provided Indexes:<br>
> +// MergedIndex is a composite index based on two provided Indexes:<br>
> // - the Dynamic index covers few files, but is relatively up-to-<br>
> date.<br>
> // - the Static index covers a bigger set of files, but is relatively<br>
> stale.<br>
> // The returned index attempts to combine results, and avoid<br>
> duplicates.<br>
> @@ -28,8 +28,25 @@ Symbol mergeSymbol(const Symbol &L, cons<br>
> // FIXME: We don't have a mechanism in Index to track deleted symbols<br>
> and<br>
> // refs in dirty files, so the merged index may return stale symbols<br>
> // and refs from Static index.<br>
> -std::unique_ptr<SymbolIndex> mergeIndex(const SymbolIndex *Dynamic,<br>
> - const SymbolIndex *Static);<br>
> +class MergedIndex : public SymbolIndex {<br>
> + const SymbolIndex *Dynamic, *Static;<br>
> +<br>
> +public:<br>
> + // The constructor does not access the symbols.<br>
> + // It's safe to inherit from this class and pass pointers to derived<br>
> members.<br>
> + MergedIndex(const SymbolIndex *Dynamic, const SymbolIndex *Static)<br>
> + : Dynamic(Dynamic), Static(Static) {}<br>
> +<br>
> + bool fuzzyFind(const FuzzyFindRequest &,<br>
> + llvm::function_ref<void(const Symbol &)>) const<br>
> override;<br>
> + void lookup(const LookupRequest &,<br>
> + llvm::function_ref<void(const Symbol &)>) const<br>
> override;<br>
> + void refs(const RefsRequest &,<br>
> + llvm::function_ref<void(const Ref &)>) const override;<br>
> + size_t estimateMemoryUsage() const override {<br>
> + return Dynamic->estimateMemoryUsage() + Static-<br>
> >estimateMemoryUsage();<br>
> + }<br>
> +};<br>
> <br>
> } // namespace clangd<br>
> } // namespace clang<br>
> <br>
> Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=343780&r1=343779&r2<br>
> =343780&view=diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp<br>
> (original)<br>
> +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Thu Oct<br>
> 4 07:20:22 2018<br>
> @@ -120,10 +120,10 @@ TEST(FileSymbolsTest, SnapshotAliveAfter<br>
> EXPECT_THAT(getRefs(*Symbols, ID), RefsAre({FileURI("f1.cc")}));<br>
> }<br>
> <br>
> -std::vector<std::string> match(const FileIndex &I,<br>
> +std::vector<std::string> match(const SymbolIndex &I,<br>
> const FuzzyFindRequest &Req) {<br>
> std::vector<std::string> Matches;<br>
> - I.index().fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> + I.fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> Matches.push_back((Sym.Scope + Sym.Name).str());<br>
> });<br>
> return Matches;<br>
> @@ -147,7 +147,7 @@ TEST(FileIndexTest, CustomizedURIScheme)<br>
> FuzzyFindRequest Req;<br>
> Req.Query = "";<br>
> bool SeenSymbol = false;<br>
> - M.index().fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> + M.fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> EXPECT_EQ(Sym.CanonicalDeclaration.FileURI, "unittest:///f.h");<br>
> SeenSymbol = true;<br>
> });<br>
> @@ -201,7 +201,7 @@ TEST(FileIndexTest, NoIncludeCollected)<br>
> FuzzyFindRequest Req;<br>
> Req.Query = "";<br>
> bool SeenSymbol = false;<br>
> - M.index().fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> + M.fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> EXPECT_TRUE(Sym.IncludeHeaders.empty());<br>
> SeenSymbol = true;<br>
> });<br>
> @@ -225,7 +225,7 @@ vector<Ty> make_vector(Arg A) {}<br>
> Req.Query = "";<br>
> bool SeenVector = false;<br>
> bool SeenMakeVector = false;<br>
> - M.index().fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> + M.fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> if (Sym.Name == "vector") {<br>
> EXPECT_EQ(Sym.Signature, "<class Ty>");<br>
> EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>");<br>
> @@ -324,7 +324,7 @@ TEST(FileIndexTest, Refs) {<br>
> AST = Test2.build();<br>
> Index.updateMain(Test2.Filename, AST);<br>
> <br>
> - EXPECT_THAT(getRefs(Index.index(), Foo.ID),<br>
> + EXPECT_THAT(getRefs(Index, Foo.ID),<br>
> RefsAre({AllOf(RefRange(MainCode.range("foo")),<br>
> FileURI("unittest:///test.cc")),<br>
> AllOf(RefRange(MainCode.range("foo")),<br>
> @@ -338,7 +338,7 @@ TEST(FileIndexTest, CollectMacros) {<br>
> FuzzyFindRequest Req;<br>
> Req.Query = "";<br>
> bool SeenSymbol = false;<br>
> - M.index().fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> + M.fuzzyFind(Req, [&](const Symbol &Sym) {<br>
> EXPECT_EQ(Sym.Name, "CLANGD");<br>
> EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);<br>
> SeenSymbol = true;<br>
> <br>
> Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/unittests/clangd/IndexTests.cpp?rev=343780&r1=343779&r2=343<br>
> 780&view=diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)<br>
> +++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Thu Oct 4<br>
> 07:20:22 2018<br>
> @@ -161,16 +161,16 @@ TEST(MemIndexTest, Lookup) {<br>
> TEST(MergeIndexTest, Lookup) {<br>
> auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}),<br>
> RefSlab()),<br>
> J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}),<br>
> RefSlab());<br>
> - auto M = mergeIndex(I.get(), J.get());<br>
> - EXPECT_THAT(lookup(*M, SymbolID("ns::A")),<br>
> UnorderedElementsAre("ns::A"));<br>
> - EXPECT_THAT(lookup(*M, SymbolID("ns::B")),<br>
> UnorderedElementsAre("ns::B"));<br>
> - EXPECT_THAT(lookup(*M, SymbolID("ns::C")),<br>
> UnorderedElementsAre("ns::C"));<br>
> - EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::B")}),<br>
> + MergedIndex M(I.get(), J.get());<br>
> + EXPECT_THAT(lookup(M, SymbolID("ns::A")),<br>
> UnorderedElementsAre("ns::A"));<br>
> + EXPECT_THAT(lookup(M, SymbolID("ns::B")),<br>
> UnorderedElementsAre("ns::B"));<br>
> + EXPECT_THAT(lookup(M, SymbolID("ns::C")),<br>
> UnorderedElementsAre("ns::C"));<br>
> + EXPECT_THAT(lookup(M, {SymbolID("ns::A"), SymbolID("ns::B")}),<br>
> UnorderedElementsAre("ns::A", "ns::B"));<br>
> - EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::C")}),<br>
> + EXPECT_THAT(lookup(M, {SymbolID("ns::A"), SymbolID("ns::C")}),<br>
> UnorderedElementsAre("ns::A", "ns::C"));<br>
> - EXPECT_THAT(lookup(*M, SymbolID("ns::D")), UnorderedElementsAre());<br>
> - EXPECT_THAT(lookup(*M, {}), UnorderedElementsAre());<br>
> + EXPECT_THAT(lookup(M, SymbolID("ns::D")), UnorderedElementsAre());<br>
> + EXPECT_THAT(lookup(M, {}), UnorderedElementsAre());<br>
> }<br>
> <br>
> TEST(MergeIndexTest, FuzzyFind) {<br>
> @@ -178,7 +178,7 @@ TEST(MergeIndexTest, FuzzyFind) {<br>
> J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}),<br>
> RefSlab());<br>
> FuzzyFindRequest Req;<br>
> Req.Scopes = {"ns::"};<br>
> - EXPECT_THAT(match(*mergeIndex(I.get(), J.get()), Req),<br>
> + EXPECT_THAT(match(MergedIndex(I.get(), J.get()), Req),<br>
> UnorderedElementsAre("ns::A", "ns::B", "ns::C"));<br>
> }<br>
> <br>
> @@ -231,7 +231,7 @@ TEST(MergeTest, PreferSymbolWithDefn) {<br>
> TEST(MergeIndexTest, Refs) {<br>
> FileIndex Dyn({"unittest"});<br>
> FileIndex StaticIndex({"unittest"});<br>
> - auto MergedIndex = mergeIndex(&Dyn.index(), &StaticIndex.index());<br>
> + MergedIndex Merge(&Dyn, &StaticIndex);<br>
> <br>
> const char *HeaderCode = "class Foo;";<br>
> auto HeaderSymbols = TestTU::withHeaderCode("class<br>
> Foo;").headerSymbols();<br>
> @@ -266,7 +266,7 @@ TEST(MergeIndexTest, Refs) {<br>
> RefsRequest Request;<br>
> Request.IDs = {Foo.ID};<br>
> RefSlab::Builder Results;<br>
> - MergedIndex->refs(Request, [&](const Ref &O) {<br>
> Results.insert(Foo.ID, O); });<br>
> + Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O);<br>
> });<br>
> <br>
> EXPECT_THAT(<br>
> std::move(Results).build(),<br>
> <br>
> Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-</a><br>
> extra/trunk/unittests/clangd/TestTU.cpp?rev=343780&r1=343779&r2=343780&<br>
> view=diff<br>
> =======================================================================<br>
> =======<br>
> --- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp (original)<br>
> +++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp Thu Oct 4<br>
> 07:20:22 2018<br>
> @@ -48,11 +48,12 @@ SymbolSlab TestTU::headerSymbols() const<br>
> return indexHeaderSymbols(AST.getASTContext(),<br>
> AST.getPreprocessorPtr());<br>
> }<br>
> <br>
> -// FIXME: This should return a FileIndex with both preamble and main<br>
> index.<br>
> std::unique_ptr<SymbolIndex> TestTU::index() const {<br>
> auto AST = build();<br>
> - auto Content = indexMainDecls(AST);<br>
> - return MemIndex::build(std::move(Content.first),<br>
> std::move(Content.second));<br>
> + auto Idx = llvm::make_unique<FileIndex>();<br>
> + Idx->updatePreamble(Filename, AST.getASTContext(),<br>
> AST.getPreprocessorPtr());<br>
> + Idx->updateMain(Filename, AST);<br>
> + return Idx;<br>
> }<br>
> <br>
> const Symbol &findSymbol(const SymbolSlab &Slab, llvm::StringRef<br>
> QName) {<br>
> <br>
> <br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>