[PATCH] D64860: [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 17 08:44:42 PDT 2019

sammccall added inline comments.

Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:77
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
+  llvm::SmallString<128> CanonPath(File);
+  llvm::sys::path::remove_dots(CanonPath, true);
This patch lacks a bit of context (why are we changing this behavior).
Based on offline discussion, it's not clear to me why this particular change to getFallbackCommand is necessary or desirable. (Compared to others, which seem like fairly obvious bugs, even if the reason to fix them isn't obvious)

Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:144
          "path must be absolute");
+  llvm::SmallString<128> CanonPath(Request.FileName);
+  llvm::sys::path::remove_dots(CanonPath, true);
nit: can you pull out a local (or FS.h) RemoveDots helper that accepts StringRef and returns std::string?

Most of the usages in this file could be inlined and I think that would read better. Not concerned about performance I think. (But returning SmallString is OK if you are)

Comment at: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp:307
+  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_EQ(DB.getFallbackCommand(File).Directory, testRoot());
Is this actually important/desirable?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list