[clang-tools-extra] r366455 - [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 11:17:43 PDT 2019
Merged this and r366559 to the 9 branch in r366718.
On Thu, Jul 18, 2019 at 9:13 AM Kadir Cetinkaya via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list