[clang-tools-extra] r366455 - [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 18 09:13:23 PDT 2019
Author: kadircet
Date: Thu Jul 18 09:13:23 2019
New Revision: 366455
URL: http://llvm.org/viewvc/llvm-project?rev=366455&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clangd/FS.cpp
clang-tools-extra/trunk/clangd/FS.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
Modified: clang-tools-extra/trunk/clangd/FS.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FS.cpp?rev=366455&r1=366454&r2=366455&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FS.cpp (original)
+++ clang-tools-extra/trunk/clangd/FS.cpp Thu Jul 18 09:13: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/trunk/clangd/FS.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FS.h?rev=366455&r1=366454&r2=366455&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FS.h (original)
+++ clang-tools-extra/trunk/clangd/FS.h Thu Jul 18 09:13: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/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=366455&r1=366454&r2=366455&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Thu Jul 18 09:13: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/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp?rev=366455&r1=366454&r2=366455&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/GlobalCompilationDatabaseTests.cpp Thu Jul 18 09:13: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 cfe-commits
mailing list