[llvm-branch-commits] [clang-tools-extra-branch] r367135 - Merging r367112:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Jul 26 10:42:03 PDT 2019
Author: hans
Date: Fri Jul 26 10:42:03 2019
New Revision: 367135
URL: http://llvm.org/viewvc/llvm-project?rev=367135&view=rev
Log:
Merging r367112:
------------------------------------------------------------------------
r367112 | sammccall | 2019-07-26 16:07:11 +0200 (Fri, 26 Jul 2019) | 11 lines
[clangd] Fix background index not triggering on windows due to case mismatch.
Summary:
This isn't a general fix to all paths where we assume case-sensitivity, it's
a minimally-invasive fix targeting the llvm 9 branch.
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D65320
------------------------------------------------------------------------
Modified:
clang-tools-extra/branches/release_90/ (props changed)
clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.h
Propchange: clang-tools-extra/branches/release_90/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jul 26 10:42:03 2019
@@ -1 +1 @@
-/clang-tools-extra/trunk:366443,366451,366455,366559,366687,366811,366880,366900,366991-366992
+/clang-tools-extra/trunk:366443,366451,366455,366559,366687,366811,366880,366900,366991-366992,367112
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=367135&r1=367134&r2=367135&view=diff
==============================================================================
--- clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.cpp Fri Jul 26 10:42:03 2019
@@ -117,20 +117,41 @@ DirectoryBasedGlobalCompilationDatabase:
return None;
}
-std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+// This class is not the only place this problem exists.
+// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
+
+static std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+ return Path.lower();
+#else
+ return Path;
+#endif
+}
+
+static bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+ return A.equals_lower(B);
+#else
+ return A == B;
+#endif
+}
+
+DirectoryBasedGlobalCompilationDatabase::CachedCDB &
DirectoryBasedGlobalCompilationDatabase::getCDBInDirLocked(PathRef Dir) const {
// FIXME(ibiryukov): Invalidate cached compilation databases on changes
- auto CachedIt = CompilationDatabases.find(Dir);
- if (CachedIt != CompilationDatabases.end())
- return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast};
- std::string Error = "";
-
- CachedCDB Entry;
- Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
- auto Result = Entry.CDB.get();
- CompilationDatabases[Dir] = std::move(Entry);
-
- return {Result, false};
+ // FIXME(sammccall): this function hot, avoid copying key when hitting cache.
+ auto Key = maybeCaseFoldPath(Dir);
+ auto R = CompilationDatabases.try_emplace(Key);
+ if (R.second) { // Cache miss, try to load CDB.
+ CachedCDB &Entry = R.first->second;
+ std::string Error = "";
+ Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+ Entry.Path = Dir;
+ }
+ return R.first->second;
}
llvm::Optional<DirectoryBasedGlobalCompilationDatabase::CDBLookupResult>
@@ -139,38 +160,41 @@ DirectoryBasedGlobalCompilationDatabase:
assert(llvm::sys::path::is_absolute(Request.FileName) &&
"path must be absolute");
+ bool ShouldBroadcast = false;
CDBLookupResult Result;
- bool SentBroadcast = false;
{
std::lock_guard<std::mutex> Lock(Mutex);
+ CachedCDB *Entry = nullptr;
if (CompileCommandsDir) {
- std::tie(Result.CDB, SentBroadcast) =
- getCDBInDirLocked(*CompileCommandsDir);
- Result.PI.SourceRoot = *CompileCommandsDir;
+ Entry = &getCDBInDirLocked(*CompileCommandsDir);
} else {
// Traverse the canonical version to prevent false positives. i.e.:
// src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
+ // FIXME(sammccall): this loop is hot, use a union-find-like structure.
actOnAllParentDirectories(removeDots(Request.FileName),
- [this, &SentBroadcast, &Result](PathRef Path) {
- std::tie(Result.CDB, SentBroadcast) =
- getCDBInDirLocked(Path);
- Result.PI.SourceRoot = Path;
- return Result.CDB != nullptr;
+ [&](PathRef Path) {
+ Entry = &getCDBInDirLocked(Path);
+ return Entry->CDB != nullptr;
});
}
- if (!Result.CDB)
+ if (!Entry || !Entry->CDB)
return llvm::None;
// Mark CDB as broadcasted to make sure discovery is performed once.
- if (Request.ShouldBroadcast && !SentBroadcast)
- CompilationDatabases[Result.PI.SourceRoot].SentBroadcast = true;
+ if (Request.ShouldBroadcast && !Entry->SentBroadcast) {
+ Entry->SentBroadcast = true;
+ ShouldBroadcast = true;
+ }
+
+ Result.CDB = Entry->CDB.get();
+ Result.PI.SourceRoot = Entry->Path;
}
// FIXME: Maybe make the following part async, since this can block retrieval
// of compile commands.
- if (Request.ShouldBroadcast && !SentBroadcast)
+ if (ShouldBroadcast)
broadcastCDB(Result);
return Result;
}
@@ -200,9 +224,9 @@ void DirectoryBasedGlobalCompilationData
if (!It.second)
return true;
- auto Res = getCDBInDirLocked(Path);
- It.first->second = Res.first != nullptr;
- return Path == Result.PI.SourceRoot;
+ CachedCDB &Entry = getCDBInDirLocked(Path);
+ It.first->second = Entry.CDB != nullptr;
+ return pathEqual(Path, Result.PI.SourceRoot);
});
}
}
@@ -213,7 +237,7 @@ void DirectoryBasedGlobalCompilationData
// Independent of whether it has an entry for that file or not.
actOnAllParentDirectories(File, [&](PathRef Path) {
if (DirectoryHasCDB.lookup(Path)) {
- if (Path == Result.PI.SourceRoot)
+ if (pathEqual(Path, Result.PI.SourceRoot))
// Make sure listeners always get a canonical path for the file.
GovernedFiles.push_back(removeDots(File));
// Stop as soon as we hit a CDB.
Modified: clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.h?rev=367135&r1=367134&r2=367135&view=diff
==============================================================================
--- clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.h (original)
+++ clang-tools-extra/branches/release_90/clangd/GlobalCompilationDatabase.h Fri Jul 26 10:42:03 2019
@@ -80,8 +80,13 @@ public:
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
private:
- std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
- getCDBInDirLocked(PathRef File) const;
+ /// Caches compilation databases loaded from directories.
+ struct CachedCDB {
+ std::string Path; // Not case-folded.
+ std::unique_ptr<clang::tooling::CompilationDatabase> CDB = nullptr;
+ bool SentBroadcast = false;
+ };
+ CachedCDB &getCDBInDirLocked(PathRef File) const;
struct CDBLookupRequest {
PathRef FileName;
@@ -98,12 +103,7 @@ private:
void broadcastCDB(CDBLookupResult Res) const;
mutable std::mutex Mutex;
- /// Caches compilation databases loaded from directories(keys are
- /// directories).
- struct CachedCDB {
- std::unique_ptr<clang::tooling::CompilationDatabase> CDB = nullptr;
- bool SentBroadcast = false;
- };
+ // Keyed by possibly-case-folded directory path.
mutable llvm::StringMap<CachedCDB> CompilationDatabases;
/// Used for command argument pointing to folder where compile_commands.json
More information about the llvm-branch-commits
mailing list