[llvm-branch-commits] [clang-tools-extra-branch] r366718 - Merging r366455 and r366559:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jul 22 11:17:24 PDT 2019
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
More information about the llvm-branch-commits
mailing list