Author: hans Date: Mon Jul 22 11:17:23 2019 New Revision: 366718 URL: http://llvm.org/viewvc/llvm-project?rev=366718&view=rev Log: Merging r366455 and r366559: ------------------------------------------------------------------------ r366455 | kadircet | 2019-07-18 18:13:23 +0200 (Thu, 18 Jul 2019) | 9 lines
[clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64860 ------------------------------------------------------------------------ ------------------------------------------------------------------------ r366559 | kadircet | 2019-07-19 12:18:52 +0200 (Fri, 19 Jul 2019) | 19 lines Revert "Revert r366458, r366467 and r366468" This reverts commit 9c377105da0be7c2c9a3c70035ce674c71b846af. [clangd][BackgroundIndexLoader] Directly store DependentTU while loading shard Summary: We were deferring the population of DependentTU field in LoadedShard until BackgroundIndexLoader was consumed. This actually triggers a use after free since the shards FileToTU was pointing at could've been moved while consuming the Loader. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64980 ------------------------------------------------------------------------ Added: clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexLoader.cpp - copied unchanged from r366559, clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexLoader.h - copied unchanged from r366559, clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/sub_dir/ - copied from r366559, clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/ Removed: clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.h Modified: clang-tools-extra/branches/release_90/ (props changed) clang-tools-extra/branches/release_90/clangd/CMakeLists.txt clang-tools-extra/branches/release_90/clangd/ClangdServer.cpp clang-tools-extra/branches/release_90/clangd/FS.cpp clang-tools-extra/branches/release_90/clangd/FS.h clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/branches/release_90/clangd/index/Background.cpp clang-tools-extra/branches/release_90/clangd/index/Background.h clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexStorage.cpp clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.cpp clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.h clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/definition.jsonrpc clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.cpp clang-tools-extra/branches/release_90/clangd/test/background-index.test clang-tools-extra/branches/release_90/clangd/unittests/BackgroundIndexTests.cpp clang-tools-extra/branches/release_90/clangd/unittests/GlobalCompilationDatabaseTests.cpp Propchange: clang-tools-extra/branches/release_90/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Mon Jul 22 11:17:23 2019 @@ -1 +1 @@ -/clang-tools-extra/trunk:366443,366451,366687 +/clang-tools-extra/trunk:366443,366451,366455,366559,366687 Modified: clang-tools-extra/branches/release_90/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/CMakeLists.txt?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/CMakeLists.txt (original) +++ clang-tools-extra/branches/release_90/clangd/CMakeLists.txt Mon Jul 22 11:17:23 2019 @@ -73,6 +73,7 @@ add_clang_library(clangDaemon XRefs.cpp index/Background.cpp + index/BackgroundIndexLoader.cpp index/BackgroundIndexStorage.cpp index/BackgroundQueue.cpp index/BackgroundRebuild.cpp Modified: clang-tools-extra/branches/release_90/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/ClangdServer.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/ClangdServer.cpp Mon Jul 22 11:17:23 2019 @@ -127,7 +127,8 @@ ClangdServer::ClangdServer(const GlobalC if (Opts.BackgroundIndex) { BackgroundIdx = llvm::make_unique<BackgroundIndex>( Context::current().clone(), FSProvider, CDB, - BackgroundIndexStorage::createDiskBackedStorageFactory()); + BackgroundIndexStorage::createDiskBackedStorageFactory( + [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); })); AddIndex(BackgroundIdx.get()); } if (DynamicIdx) Modified: clang-tools-extra/branches/release_90/clangd/FS.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/FS.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/FS.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/FS.cpp Mon Jul 22 11:17:23 2019 @@ -111,5 +111,11 @@ PreambleFileStatusCache::getConsumingFS( return llvm::IntrusiveRefCntPtr<CacheVFS>(new CacheVFS(std::move(FS), *this)); } +Path removeDots(PathRef File) { + llvm::SmallString<128> CanonPath(File); + llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true); + return CanonPath.str().str(); +} + } // namespace clangd } // namespace clang Modified: clang-tools-extra/branches/release_90/clangd/FS.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/FS.h?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/FS.h (original) +++ clang-tools-extra/branches/release_90/clangd/FS.h Mon Jul 22 11:17:23 2019 @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H +#include "Path.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/Optional.h" #include "llvm/Support/VirtualFileSystem.h" @@ -65,6 +66,13 @@ private: llvm::StringMap<llvm::vfs::Status> StatCache; }; +/// Returns a version of \p File that doesn't contain dots and dot dots. +/// e.g /a/b/../c -> /a/c +/// /a/b/./c -> /a/b/c +/// FIXME: We should avoid encountering such paths in clangd internals by +/// filtering everything we get over LSP, CDB, etc. +Path removeDots(PathRef File); + } // namespace clangd } // namespace clang Modified: clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp Mon Jul 22 11:17:23 2019 @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "GlobalCompilationDatabase.h" +#include "FS.h" #include "Logger.h" #include "Path.h" #include "clang/Frontend/CompilerInvocation.h" @@ -15,6 +16,7 @@ #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include <string> @@ -147,12 +149,15 @@ DirectoryBasedGlobalCompilationDatabase: getCDBInDirLocked(*CompileCommandsDir); Result.PI.SourceRoot = *CompileCommandsDir; } else { - actOnAllParentDirectories( - Request.FileName, [this, &SentBroadcast, &Result](PathRef Path) { - std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path); - Result.PI.SourceRoot = Path; - return Result.CDB != nullptr; - }); + // Traverse the canonical version to prevent false positives. i.e.: + // src/build/../a.cc can detect a CDB in /src/build if not canonicalized. + actOnAllParentDirectories(removeDots(Request.FileName), + [this, &SentBroadcast, &Result](PathRef Path) { + std::tie(Result.CDB, SentBroadcast) = + getCDBInDirLocked(Path); + Result.PI.SourceRoot = Path; + return Result.CDB != nullptr; + }); } if (!Result.CDB) @@ -209,7 +214,8 @@ void DirectoryBasedGlobalCompilationData actOnAllParentDirectories(File, [&](PathRef Path) { if (DirectoryHasCDB.lookup(Path)) { if (Path == Result.PI.SourceRoot) - GovernedFiles.push_back(File); + // Make sure listeners always get a canonical path for the file. + GovernedFiles.push_back(removeDots(File)); // Stop as soon as we hit a CDB. return true; } @@ -248,7 +254,7 @@ OverlayCDB::getCompileCommand(PathRef Fi llvm::Optional<tooling::CompileCommand> Cmd; { std::lock_guard<std::mutex> Lock(Mutex); - auto It = Commands.find(File); + auto It = Commands.find(removeDots(File)); if (It != Commands.end()) Cmd = It->second; } @@ -272,20 +278,24 @@ tooling::CompileCommand OverlayCDB::getF void OverlayCDB::setCompileCommand( PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) { + // We store a canonical version internally to prevent mismatches between set + // and get compile commands. Also it assures clients listening to broadcasts + // doesn't receive different names for the same file. + std::string CanonPath = removeDots(File); { std::unique_lock<std::mutex> Lock(Mutex); if (Cmd) - Commands[File] = std::move(*Cmd); + Commands[CanonPath] = std::move(*Cmd); else - Commands.erase(File); + Commands.erase(CanonPath); } - OnCommandChanged.broadcast({File}); + OnCommandChanged.broadcast({CanonPath}); } llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const { { std::lock_guard<std::mutex> Lock(Mutex); - auto It = Commands.find(File); + auto It = Commands.find(removeDots(File)); if (It != Commands.end()) return ProjectInfo{}; } Modified: clang-tools-extra/branches/release_90/clangd/index/Background.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/index/Background.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/index/Background.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/index/Background.cpp Mon Jul 22 11:17:23 2019 @@ -10,6 +10,7 @@ #include "ClangdUnit.h" #include "Compiler.h" #include "Context.h" +#include "FSProvider.h" #include "Headers.h" #include "Logger.h" #include "Path.h" @@ -18,6 +19,7 @@ #include "Threading.h" #include "Trace.h" #include "URI.h" +#include "index/BackgroundIndexLoader.h" #include "index/FileIndex.h" #include "index/IndexAction.h" #include "index/MemIndex.h" @@ -28,6 +30,8 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Driver/Types.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" @@ -42,6 +46,7 @@ #include <atomic> #include <chrono> #include <condition_variable> +#include <cstddef> #include <memory> #include <mutex> #include <numeric> @@ -49,6 +54,8 @@ #include <random> #include <string> #include <thread> +#include <utility> +#include <vector> namespace clang { namespace clangd { @@ -119,6 +126,18 @@ llvm::SmallString<128> getAbsolutePath(c } return AbsolutePath; } + +bool shardIsStale(const LoadedShard &LS, llvm::vfs::FileSystem *FS) { + auto Buf = FS->getBufferForFile(LS.AbsolutePath); + if (!Buf) { + elog("Background-index: Couldn't read {0} to validate stored index: {1}", + LS.AbsolutePath, Buf.getError().message()); + // There is no point in indexing an unreadable file. + return false; + } + return digest(Buf->get()->getBuffer()) != LS.Digest; +} + } // namespace BackgroundIndex::BackgroundIndex( @@ -156,14 +175,14 @@ BackgroundQueue::Task BackgroundIndex::c log("Enqueueing {0} commands for indexing", ChangedFiles.size()); SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size())); - auto NeedsReIndexing = loadShards(std::move(ChangedFiles)); + auto NeedsReIndexing = loadProject(std::move(ChangedFiles)); // Run indexing for files that need to be updated. std::shuffle(NeedsReIndexing.begin(), NeedsReIndexing.end(), std::mt19937(std::random_device{}())); std::vector<BackgroundQueue::Task> Tasks; Tasks.reserve(NeedsReIndexing.size()); - for (auto &Elem : NeedsReIndexing) - Tasks.push_back(indexFileTask(std::move(Elem.first), Elem.second)); + for (auto &Cmd : NeedsReIndexing) + Tasks.push_back(indexFileTask(std::move(Cmd))); Queue.append(std::move(Tasks)); }); @@ -178,13 +197,12 @@ static llvm::StringRef filenameWithoutEx } BackgroundQueue::Task -BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd, - BackgroundIndexStorage *Storage) { - BackgroundQueue::Task T([this, Storage, Cmd] { +BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd) { + BackgroundQueue::Task T([this, Cmd] { // We can't use llvm::StringRef here since we are going to // move from Cmd during the call below. const std::string FileName = Cmd.Filename; - if (auto Error = index(std::move(Cmd), Storage)) + if (auto Error = index(std::move(Cmd))) elog("Indexing {0} failed: {1}", FileName, std::move(Error)); }); T.QueuePri = IndexFile; @@ -207,7 +225,7 @@ void BackgroundIndex::boostRelated(llvm: void BackgroundIndex::update( llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap<ShardVersion> &ShardVersionsSnapshot, - BackgroundIndexStorage *IndexStorage, bool HadErrors) { + bool HadErrors) { // Partition symbols/references into files. struct File { llvm::DenseSet<const Symbol *> Symbols; @@ -291,22 +309,21 @@ void BackgroundIndex::update( // We need to store shards before updating the index, since the latter // consumes slabs. // FIXME: Also skip serializing the shard if it is already up-to-date. - if (IndexStorage) { - IndexFileOut Shard; - Shard.Symbols = SS.get(); - Shard.Refs = RS.get(); - Shard.Relations = RelS.get(); - Shard.Sources = IG.get(); - - // Only store command line hash for main files of the TU, since our - // current model keeps only one version of a header file. - if (Path == MainFile) - Shard.Cmd = Index.Cmd.getPointer(); - - if (auto Error = IndexStorage->storeShard(Path, Shard)) - elog("Failed to write background-index shard for file {0}: {1}", Path, - std::move(Error)); - } + BackgroundIndexStorage *IndexStorage = IndexStorageFactory(Path); + IndexFileOut Shard; + Shard.Symbols = SS.get(); + Shard.Refs = RS.get(); + Shard.Relations = RelS.get(); + Shard.Sources = IG.get(); + + // Only store command line hash for main files of the TU, since our + // current model keeps only one version of a header file. + if (Path == MainFile) + Shard.Cmd = Index.Cmd.getPointer(); + + if (auto Error = IndexStorage->storeShard(Path, Shard)) + elog("Failed to write background-index shard for file {0}: {1}", Path, + std::move(Error)); { std::lock_guard<std::mutex> Lock(ShardVersionsMu); @@ -329,8 +346,7 @@ void BackgroundIndex::update( } } -llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd, - BackgroundIndexStorage *IndexStorage) { +llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) { trace::Span Tracer("BackgroundIndex"); SPAN_ATTACH(Tracer, "file", Cmd.Filename); auto AbsolutePath = getAbsolutePath(Cmd); @@ -424,176 +440,78 @@ llvm::Error BackgroundIndex::index(tooli for (auto &It : *Index.Sources) It.second.Flags |= IncludeGraphNode::SourceFlag::HadErrors; } - update(AbsolutePath, std::move(Index), ShardVersionsSnapshot, IndexStorage, - HadErrors); + update(AbsolutePath, std::move(Index), ShardVersionsSnapshot, HadErrors); Rebuilder.indexedTU(); return llvm::Error::success(); } -std::vector<BackgroundIndex::Source> -BackgroundIndex::loadShard(const tooling::CompileCommand &Cmd, - BackgroundIndexStorage *IndexStorage, - llvm::StringSet<> &LoadedShards) { - struct ShardInfo { - std::string AbsolutePath; - std::unique_ptr<IndexFileIn> Shard; - FileDigest Digest = {}; - bool CountReferences = false; - bool HadErrors = false; - }; - std::vector<ShardInfo> IntermediateSymbols; - // Make sure we don't have duplicate elements in the queue. Keys are absolute - // paths. - llvm::StringSet<> InQueue; - auto FS = FSProvider.getFileSystem(); - // Dependencies of this TU, paired with the information about whether they - // need to be re-indexed or not. - std::vector<Source> Dependencies; - std::queue<Source> ToVisit; - std::string AbsolutePath = getAbsolutePath(Cmd).str(); - // Up until we load the shard related to a dependency it needs to be - // re-indexed. - ToVisit.emplace(AbsolutePath, true); - InQueue.insert(AbsolutePath); - // Goes over each dependency. - while (!ToVisit.empty()) { - Dependencies.push_back(std::move(ToVisit.front())); - // Dependencies is not modified during the rest of the loop, so it is safe - // to keep the reference. - auto &CurDependency = Dependencies.back(); - ToVisit.pop(); - // If we have already seen this shard before(either loaded or failed) don't - // re-try again. Since the information in the shard won't change from one TU - // to another. - if (!LoadedShards.try_emplace(CurDependency.Path).second) { - // If the dependency needs to be re-indexed, first occurence would already - // have detected that, so we don't need to issue it again. - CurDependency.NeedsReIndexing = false; - continue; - } +// Restores shards for \p MainFiles from index storage. Then checks staleness of +// those shards and returns a list of TUs that needs to be indexed to update +// staleness. +std::vector<tooling::CompileCommand> +BackgroundIndex::loadProject(std::vector<std::string> MainFiles) { + std::vector<tooling::CompileCommand> NeedsReIndexing; - auto Shard = IndexStorage->loadShard(CurDependency.Path); - if (!Shard || !Shard->Sources) { - // File will be returned as requiring re-indexing to caller. - vlog("Failed to load shard: {0}", CurDependency.Path); - continue; - } - // These are the edges in the include graph for current dependency. - for (const auto &I : *Shard->Sources) { - auto U = URI::parse(I.getKey()); - if (!U) - continue; - auto AbsolutePath = URI::resolve(*U, CurDependency.Path); - if (!AbsolutePath) - continue; - // Add file as dependency if haven't seen before. - if (InQueue.try_emplace(*AbsolutePath).second) - ToVisit.emplace(*AbsolutePath, true); - // The node contains symbol information only for current file, the rest is - // just edges. - if (*AbsolutePath != CurDependency.Path) - continue; - - // We found source file info for current dependency. - assert(I.getValue().Digest != FileDigest{{0}} && "Digest is empty?"); - ShardInfo SI; - SI.AbsolutePath = CurDependency.Path; - SI.Shard = std::move(Shard); - SI.Digest = I.getValue().Digest; - SI.CountReferences = - I.getValue().Flags & IncludeGraphNode::SourceFlag::IsTU; - SI.HadErrors = - I.getValue().Flags & IncludeGraphNode::SourceFlag::HadErrors; - IntermediateSymbols.push_back(std::move(SI)); - // Check if the source needs re-indexing. - // Get the digest, skip it if file doesn't exist. - auto Buf = FS->getBufferForFile(CurDependency.Path); - if (!Buf) { - elog("Couldn't get buffer for file: {0}: {1}", CurDependency.Path, - Buf.getError().message()); - continue; - } - // If digests match then dependency doesn't need re-indexing. - // FIXME: Also check for dependencies(sources) of this shard and compile - // commands for cache invalidation. - CurDependency.NeedsReIndexing = - digest(Buf->get()->getBuffer()) != I.getValue().Digest; - } - } - // Load shard information into background-index. + Rebuilder.startLoading(); + // Load shards for all of the mainfiles. + const std::vector<LoadedShard> Result = + loadIndexShards(MainFiles, IndexStorageFactory, CDB); + size_t LoadedShards = 0; { + // Update in-memory state. std::lock_guard<std::mutex> Lock(ShardVersionsMu); - // This can override a newer version that is added in another thread, - // if this thread sees the older version but finishes later. This - // should be rare in practice. - for (const ShardInfo &SI : IntermediateSymbols) { + for (auto &LS : Result) { + if (!LS.Shard) + continue; auto SS = - SI.Shard->Symbols - ? llvm::make_unique<SymbolSlab>(std::move(*SI.Shard->Symbols)) + LS.Shard->Symbols + ? llvm::make_unique<SymbolSlab>(std::move(*LS.Shard->Symbols)) : nullptr; - auto RS = SI.Shard->Refs - ? llvm::make_unique<RefSlab>(std::move(*SI.Shard->Refs)) + auto RS = LS.Shard->Refs + ? llvm::make_unique<RefSlab>(std::move(*LS.Shard->Refs)) : nullptr; auto RelS = - SI.Shard->Relations - ? llvm::make_unique<RelationSlab>(std::move(*SI.Shard->Relations)) + LS.Shard->Relations + ? llvm::make_unique<RelationSlab>(std::move(*LS.Shard->Relations)) : nullptr; - ShardVersion &SV = ShardVersions[SI.AbsolutePath]; - SV.Digest = SI.Digest; - SV.HadErrors = SI.HadErrors; + ShardVersion &SV = ShardVersions[LS.AbsolutePath]; + SV.Digest = LS.Digest; + SV.HadErrors = LS.HadErrors; + ++LoadedShards; - IndexedSymbols.update(SI.AbsolutePath, std::move(SS), std::move(RS), - std::move(RelS), SI.CountReferences); + IndexedSymbols.update(LS.AbsolutePath, std::move(SS), std::move(RS), + std::move(RelS), LS.CountReferences); } } - if (!IntermediateSymbols.empty()) - Rebuilder.loadedTU(); + Rebuilder.loadedShard(LoadedShards); + Rebuilder.doneLoading(); - return Dependencies; -} + auto FS = FSProvider.getFileSystem(); + llvm::DenseSet<PathRef> TUsToIndex; + // We'll accept data from stale shards, but ensure the files get reindexed + // soon. + for (auto &LS : Result) { + if (!shardIsStale(LS, FS.get())) + continue; + PathRef TUForFile = LS.DependentTU; + assert(!TUForFile.empty() && "File without a TU!"); -// Goes over each changed file and loads them from index. Returns the list of -// TUs that had out-of-date/no shards. -std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> -BackgroundIndex::loadShards(std::vector<std::string> ChangedFiles) { - std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> - NeedsReIndexing; - // Keeps track of the files that will be reindexed, to make sure we won't - // re-index same dependencies more than once. Keys are AbsolutePaths. - llvm::StringSet<> FilesToIndex; - // Keeps track of the loaded shards to make sure we don't perform redundant - // disk IO. Keys are absolute paths. - llvm::StringSet<> LoadedShards; - Rebuilder.startLoading(); - for (const auto &File : ChangedFiles) { - auto Cmd = CDB.getCompileCommand(File); + // FIXME: Currently, we simply schedule indexing on a TU whenever any of + // its dependencies needs re-indexing. We might do it smarter by figuring + // out a minimal set of TUs that will cover all the stale dependencies. + // FIXME: Try looking at other TUs if no compile commands are available + // for this TU, i.e TU was deleted after we performed indexing. + TUsToIndex.insert(TUForFile); + } + + for (PathRef TU : TUsToIndex) { + auto Cmd = CDB.getCompileCommand(TU); if (!Cmd) continue; - - std::string ProjectRoot; - if (auto PI = CDB.getProjectInfo(File)) - ProjectRoot = std::move(PI->SourceRoot); - - BackgroundIndexStorage *IndexStorage = IndexStorageFactory(ProjectRoot); - auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards); - for (const auto &Dependency : Dependencies) { - if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path)) - continue; - // FIXME: Currently, we simply schedule indexing on a TU whenever any of - // its dependencies needs re-indexing. We might do it smarter by figuring - // out a minimal set of TUs that will cover all the stale dependencies. - vlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.", - Cmd->Filename, Dependency.Path); - NeedsReIndexing.push_back({std::move(*Cmd), IndexStorage}); - // Mark all of this TU's dependencies as to-be-indexed so that we won't - // try to re-index those. - for (const auto &Dependency : Dependencies) - FilesToIndex.insert(Dependency.Path); - break; - } + NeedsReIndexing.emplace_back(std::move(*Cmd)); } - Rebuilder.doneLoading(); + return NeedsReIndexing; } Modified: clang-tools-extra/branches/release_90/clangd/index/Background.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/index/Background.h?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/index/Background.h (original) +++ clang-tools-extra/branches/release_90/clangd/index/Background.h Mon Jul 22 11:17:23 2019 @@ -12,6 +12,7 @@ #include "Context.h" #include "FSProvider.h" #include "GlobalCompilationDatabase.h" +#include "Path.h" #include "SourceCode.h" #include "Threading.h" #include "index/BackgroundRebuild.h" @@ -49,15 +50,17 @@ public: virtual std::unique_ptr<IndexFileIn> loadShard(llvm::StringRef ShardIdentifier) const = 0; - // The factory provides storage for each CDB. + // The factory provides storage for each File. // It keeps ownership of the storage instances, and should manage caching // itself. Factory must be threadsafe and never returns nullptr. - using Factory = - llvm::unique_function<BackgroundIndexStorage *(llvm::StringRef)>; + using Factory = llvm::unique_function<BackgroundIndexStorage *(PathRef)>; // Creates an Index Storage that saves shards into disk. Index storage uses - // CDBDirectory + ".clangd/index/" as the folder to save shards. - static Factory createDiskBackedStorageFactory(); + // CDBDirectory + ".clangd/index/" as the folder to save shards. CDBDirectory + // is the first directory containing a CDB in parent directories of a file, or + // user's home directory if none was found, e.g. standard library headers. + static Factory createDiskBackedStorageFactory( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo); }; // A priority queue of tasks which can be run on (external) worker threads. @@ -157,15 +160,14 @@ private: /// information on IndexStorage. void update(llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap<ShardVersion> &ShardVersionsSnapshot, - BackgroundIndexStorage *IndexStorage, bool HadErrors); + bool HadErrors); // configuration const FileSystemProvider &FSProvider; const GlobalCompilationDatabase &CDB; Context BackgroundContext; - llvm::Error index(tooling::CompileCommand, - BackgroundIndexStorage *IndexStorage); + llvm::Error index(tooling::CompileCommand); FileSymbols IndexedSymbols; BackgroundIndexRebuilder Rebuilder; @@ -173,25 +175,13 @@ private: std::mutex ShardVersionsMu; BackgroundIndexStorage::Factory IndexStorageFactory; - struct Source { - std::string Path; - bool NeedsReIndexing; - Source(llvm::StringRef Path, bool NeedsReIndexing) - : Path(Path), NeedsReIndexing(NeedsReIndexing) {} - }; - // Loads the shards for a single TU and all of its dependencies. Returns the - // list of sources and whether they need to be re-indexed. - std::vector<Source> loadShard(const tooling::CompileCommand &Cmd, - BackgroundIndexStorage *IndexStorage, - llvm::StringSet<> &LoadedShards); - // Tries to load shards for the ChangedFiles. - std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> - loadShards(std::vector<std::string> ChangedFiles); + // Tries to load shards for the MainFiles and their dependencies. + std::vector<tooling::CompileCommand> + loadProject(std::vector<std::string> MainFiles); BackgroundQueue::Task changedFilesTask(const std::vector<std::string> &ChangedFiles); - BackgroundQueue::Task indexFileTask(tooling::CompileCommand Cmd, - BackgroundIndexStorage *Storage); + BackgroundQueue::Task indexFileTask(tooling::CompileCommand Cmd); // from lowest to highest priority enum QueuePriority { Modified: clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexStorage.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexStorage.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexStorage.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/index/BackgroundIndexStorage.cpp Mon Jul 22 11:17:23 2019 @@ -6,13 +6,21 @@ // //===----------------------------------------------------------------------===// +#include "GlobalCompilationDatabase.h" #include "Logger.h" +#include "Path.h" #include "index/Background.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" +#include <functional> namespace clang { namespace clangd { @@ -118,12 +126,21 @@ public: // Creates and owns IndexStorages for multiple CDBs. class DiskBackedIndexStorageManager { public: - DiskBackedIndexStorageManager() - : IndexStorageMapMu(llvm::make_unique<std::mutex>()) {} + DiskBackedIndexStorageManager( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo) + : IndexStorageMapMu(llvm::make_unique<std::mutex>()), + GetProjectInfo(std::move(GetProjectInfo)) { + llvm::SmallString<128> HomeDir; + llvm::sys::path::home_directory(HomeDir); + this->HomeDir = HomeDir.str().str(); + } - // Creates or fetches to storage from cache for the specified CDB. - BackgroundIndexStorage *operator()(llvm::StringRef CDBDirectory) { + // Creates or fetches to storage from cache for the specified project. + BackgroundIndexStorage *operator()(PathRef File) { std::lock_guard<std::mutex> Lock(*IndexStorageMapMu); + Path CDBDirectory = HomeDir; + if (auto PI = GetProjectInfo(File)) + CDBDirectory = PI->SourceRoot; auto &IndexStorage = IndexStorageMap[CDBDirectory]; if (!IndexStorage) IndexStorage = create(CDBDirectory); @@ -131,21 +148,28 @@ public: } private: - std::unique_ptr<BackgroundIndexStorage> create(llvm::StringRef CDBDirectory) { - if (CDBDirectory.empty()) + std::unique_ptr<BackgroundIndexStorage> create(PathRef CDBDirectory) { + if (CDBDirectory.empty()) { + elog("Tried to create storage for empty directory!"); return llvm::make_unique<NullStorage>(); + } return llvm::make_unique<DiskBackedIndexStorage>(CDBDirectory); } + Path HomeDir; + llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap; std::unique_ptr<std::mutex> IndexStorageMapMu; + + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo; }; } // namespace BackgroundIndexStorage::Factory -BackgroundIndexStorage::createDiskBackedStorageFactory() { - return DiskBackedIndexStorageManager(); +BackgroundIndexStorage::createDiskBackedStorageFactory( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo) { + return DiskBackedIndexStorageManager(std::move(GetProjectInfo)); } } // namespace clangd Modified: clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.cpp Mon Jul 22 11:17:23 2019 @@ -78,13 +78,13 @@ void BackgroundIndexRebuilder::idle() { void BackgroundIndexRebuilder::startLoading() { std::lock_guard<std::mutex> Lock(Mu); if (!Loading) - LoadedTUs = 0; + LoadedShards = 0; ++Loading; } -void BackgroundIndexRebuilder::loadedTU() { +void BackgroundIndexRebuilder::loadedShard(size_t ShardCount) { std::lock_guard<std::mutex> Lock(Mu); assert(Loading); - ++LoadedTUs; + LoadedShards += ShardCount; } void BackgroundIndexRebuilder::doneLoading() { maybeRebuild("after loading index from disk", [this] { @@ -93,7 +93,7 @@ void BackgroundIndexRebuilder::doneLoadi if (Loading) // was loading multiple batches concurrently return false; // rebuild once the last batch is done. // Rebuild if we loaded any shards, or if we stopped an indexedTU rebuild. - return LoadedTUs > 0 || enoughTUsToRebuild(); + return LoadedShards > 0 || enoughTUsToRebuild(); }); } Modified: clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.h?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.h (original) +++ clang-tools-extra/branches/release_90/clangd/index/BackgroundRebuild.h Mon Jul 22 11:17:23 2019 @@ -17,6 +17,7 @@ #include "index/FileIndex.h" #include "index/Index.h" #include "llvm/Support/Threading.h" +#include <cstddef> namespace clang { namespace clangd { @@ -61,7 +62,7 @@ public: // sessions may happen concurrently. void startLoading(); // Called to indicate some shards were actually loaded from disk. - void loadedTU(); + void loadedShard(size_t ShardCount); // Called to indicate we're finished loading shards from disk. // May rebuild (if any were loaded). void doneLoading(); @@ -89,7 +90,7 @@ private: unsigned IndexedTUsAtLastRebuild = 0; // Are we loading shards? May be multiple concurrent sessions. unsigned Loading = 0; - unsigned LoadedTUs; // In the current loading session. + unsigned LoadedShards; // In the current loading session. SwapIndex *Target; FileSymbols *Source; Modified: clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/definition.jsonrpc URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/definition.jsonrpc?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/definition.jsonrpc (original) +++ clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/definition.jsonrpc Mon Jul 22 11:17:23 2019 @@ -18,7 +18,7 @@ "uri": "file://DIRECTORY/bar.cpp", "languageId": "cpp", "version": 1, - "text": "#include \"foo.h\"\nint main(){\nreturn foo();\n}" + "text": "#include \"sub_dir/foo.h\"\nint main(){\nreturn foo();\n}" } } } Modified: clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.cpp Mon Jul 22 11:17:23 2019 @@ -1,2 +1,2 @@ -#include "foo.h" +#include "sub_dir/foo.h" int foo() { return 42; } Removed: clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.h?rev=366717&view=auto ============================================================================== --- clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.h (original) +++ clang-tools-extra/branches/release_90/clangd/test/Inputs/background-index/foo.h (removed) @@ -1,4 +0,0 @@ -#ifndef FOO_H -#define FOO_H -int foo(); -#endif Modified: clang-tools-extra/branches/release_90/clangd/test/background-index.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/test/background-index.test?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/test/background-index.test (original) +++ clang-tools-extra/branches/release_90/clangd/test/background-index.test Mon Jul 22 11:17:23 2019 @@ -5,7 +5,8 @@ # RUN: rm -rf %t # RUN: cp -r %S/Inputs/background-index %t # Need to embed the correct temp path in the actual JSON-RPC requests. -# RUN: sed -i -e "s|DIRECTORY|%t|" %t/* +# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc +# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json # We're editing bar.cpp, which includes foo.h. # foo() is declared in foo.h and defined in foo.cpp. @@ -14,6 +15,7 @@ # Test that the index is writing files in the expected location. # RUN: ls %t/.clangd/index/foo.cpp.*.idx +# RUN: ls %t/sub_dir/.clangd/index/foo.h.*.idx # Test the index is read from disk: delete code and restart clangd. # RUN: rm %t/foo.cpp Modified: clang-tools-extra/branches/release_90/clangd/unittests/BackgroundIndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/unittests/BackgroundIndexTests.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/unittests/BackgroundIndexTests.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/unittests/BackgroundIndexTests.cpp Mon Jul 22 11:17:23 2019 @@ -211,7 +211,7 @@ TEST_F(BackgroundIndexTest, ShardStorage OverlayCDB CDB(/*Base=*/nullptr); BackgroundIndex Idx(Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; }); - CDB.setCompileCommand(testPath("root"), Cmd); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache. @@ -335,7 +335,7 @@ TEST_F(BackgroundIndexTest, ShardStorage OverlayCDB CDB(/*Base=*/nullptr); BackgroundIndex Idx(Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; }); - CDB.setCompileCommand(testPath("root"), Cmd); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache. @@ -353,7 +353,7 @@ TEST_F(BackgroundIndexTest, ShardStorage OverlayCDB CDB(/*Base=*/nullptr); BackgroundIndex Idx(Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; }); - CDB.setCompileCommand(testPath("root"), Cmd); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); ASSERT_TRUE(Idx.blockUntilIdleForTest()); } EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache. @@ -621,8 +621,8 @@ TEST_F(BackgroundIndexRebuilderTest, Ind TEST_F(BackgroundIndexRebuilderTest, LoadingShards) { Rebuilder.startLoading(); - Rebuilder.loadedTU(); - Rebuilder.loadedTU(); + Rebuilder.loadedShard(10); + Rebuilder.loadedShard(20); EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); })); // No rebuild for no shards. @@ -631,11 +631,11 @@ TEST_F(BackgroundIndexRebuilderTest, Loa // Loads can overlap. Rebuilder.startLoading(); - Rebuilder.loadedTU(); + Rebuilder.loadedShard(1); Rebuilder.startLoading(); - Rebuilder.loadedTU(); + Rebuilder.loadedShard(1); EXPECT_FALSE(checkRebuild([&] { Rebuilder.doneLoading(); })); - Rebuilder.loadedTU(); + Rebuilder.loadedShard(1); EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); })); // No rebuilding for indexed files while loading. Modified: clang-tools-extra/branches/release_90/clangd/unittests/GlobalCompilationDatabaseTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/unittests/GlobalCompilationDatabaseTests.cpp?rev=366718&r1=366717&r2=366718&view=diff ============================================================================== --- clang-tools-extra/branches/release_90/clangd/unittests/GlobalCompilationDatabaseTests.cpp (original) +++ clang-tools-extra/branches/release_90/clangd/unittests/GlobalCompilationDatabaseTests.cpp Mon Jul 22 11:17:23 2019 @@ -10,6 +10,7 @@ #include "Path.h" #include "TestFS.h" +#include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" @@ -31,6 +32,7 @@ using ::testing::AllOf; using ::testing::Contains; using ::testing::ElementsAre; using ::testing::EndsWith; +using ::testing::HasSubstr; using ::testing::IsEmpty; using ::testing::Not; using ::testing::StartsWith; @@ -247,9 +249,10 @@ TEST(GlobalCompilationDatabaseTest, Disc }); File = FS.Root; - llvm::sys::path::append(File, "a.cc"); + llvm::sys::path::append(File, "build", "..", "a.cc"); DB.getCompileCommand(File.str()); - EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc"))); + EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf( + EndsWith("a.cc"), Not(HasSubstr(".."))))); DiscoveredFiles.clear(); File = FS.Root; @@ -282,6 +285,27 @@ TEST(GlobalCompilationDatabaseTest, Disc } } +TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) { + OverlayCDB DB(nullptr); + std::vector<std::string> DiscoveredFiles; + auto Sub = + DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) { + DiscoveredFiles = Changes; + }); + + llvm::SmallString<128> Root(testRoot()); + llvm::sys::path::append(Root, "build", "..", "a.cc"); + DB.setCompileCommand(Root.str(), tooling::CompileCommand()); + EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(testPath("a.cc"))); + DiscoveredFiles.clear(); + + llvm::SmallString<128> File(testRoot()); + llvm::sys::path::append(File, "blabla", "..", "a.cc"); + + EXPECT_TRUE(DB.getCompileCommand(File)); + EXPECT_TRUE(DB.getProjectInfo(File)); +} + } // namespace } // namespace clangd } // namespace clang _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
