sammccall created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65320
Files:
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.h
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -80,7 +80,8 @@
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
private:
- std::pair<tooling::CompilationDatabase *, /*SentBroadcast*/ bool>
+ std::tuple<tooling::CompilationDatabase *, /*SentBroadcast*/ bool,
+ /*CanonDir*/ std::string>
getCDBInDirLocked(PathRef File) const;
struct CDBLookupRequest {
@@ -101,9 +102,11 @@
/// Caches compilation databases loaded from directories(keys are
/// directories).
struct CachedCDB {
+ std::string Path; // Not case-folded.
std::unique_ptr<clang::tooling::CompilationDatabase> CDB = nullptr;
bool SentBroadcast = false;
};
+ // Keyed by possibly-case-folded path!
mutable llvm::StringMap<CachedCDB> CompilationDatabases;
/// Used for command argument pointing to folder where compile_commands.json
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -117,20 +117,48 @@
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 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
+}
+
+// If a CDB is returned, *CanonDir is it's original, non-case-folded directory.
+std::tuple<tooling::CompilationDatabase *, /*SentBroadcast*/ bool,
+ /*CanonPath*/ std::string>
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};
+ auto Key = maybeCaseFoldPath(Dir);
+ auto CachedIt = CompilationDatabases.find(Key);
+ if (CachedIt != CompilationDatabases.end()) {
+ return {CachedIt->second.CDB.get(), CachedIt->second.SentBroadcast,
+ CachedIt->second.Path};
+ }
std::string Error = "";
CachedCDB Entry;
Entry.CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+ Entry.Path = Dir;
auto Result = Entry.CDB.get();
- CompilationDatabases[Dir] = std::move(Entry);
+ CompilationDatabases[Key] = std::move(Entry);
- return {Result, false};
+ return {Result, false, Dir};
}
llvm::Optional<DirectoryBasedGlobalCompilationDatabase::CDBLookupResult>
@@ -145,17 +173,16 @@
{
std::lock_guard<std::mutex> Lock(Mutex);
if (CompileCommandsDir) {
- std::tie(Result.CDB, SentBroadcast) =
+ std::tie(Result.CDB, SentBroadcast, Result.PI.SourceRoot) =
getCDBInDirLocked(*CompileCommandsDir);
- Result.PI.SourceRoot = *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.
actOnAllParentDirectories(removeDots(Request.FileName),
[this, &SentBroadcast, &Result](PathRef Path) {
- std::tie(Result.CDB, SentBroadcast) =
+ std::tie(Result.CDB, SentBroadcast,
+ Result.PI.SourceRoot) =
getCDBInDirLocked(Path);
- Result.PI.SourceRoot = Path;
return Result.CDB != nullptr;
});
}
@@ -201,8 +228,8 @@
return true;
auto Res = getCDBInDirLocked(Path);
- It.first->second = Res.first != nullptr;
- return Path == Result.PI.SourceRoot;
+ It.first->second = std::get<0>(Res) != nullptr;
+ return pathEqual(Path, Result.PI.SourceRoot);
});
}
}
@@ -213,7 +240,7 @@
// 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
