diff --git a/clang-tools-extra/clangd/FS.cpp b/clang-tools-extra/clangd/FS.cpp index aae15b55172a..aca9061d5c53 100644 --- a/clang-tools-extra/clangd/FS.cpp +++ b/clang-tools-extra/clangd/FS.cpp @@ -10,20 +10,25 @@ #include "clang/Basic/LLVM.h" #include "llvm/ADT/None.h" #include "llvm/Support/Path.h" +#include "llvm/Support/VirtualFileSystem.h" namespace clang { namespace clangd { -PreambleFileStatusCache::PreambleFileStatusCache(llvm::StringRef MainFilePath) - : MainFilePath(MainFilePath) { +PreambleFileStatusCache::PreambleFileStatusCache(llvm::StringRef MainFilePath){ assert(llvm::sys::path::is_absolute(MainFilePath)); + llvm::SmallString<256> MainFileCanonical(MainFilePath); + llvm::sys::path::remove_dots(MainFileCanonical, /*remove_dot_dot=*/true); + this->MainFilePath = MainFileCanonical.str(); } void PreambleFileStatusCache::update(const llvm::vfs::FileSystem &FS, llvm::vfs::Status S) { + // Canonicalize path for later lookup, which is usually by absolute path. llvm::SmallString<32> PathStore(S.getName()); if (FS.makeAbsolute(PathStore)) return; + llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true); // Do not cache status for the main file. if (PathStore == MainFilePath) return; @@ -33,9 +38,15 @@ void PreambleFileStatusCache::update(const llvm::vfs::FileSystem &FS, llvm::Optional PreambleFileStatusCache::lookup(llvm::StringRef File) const { - auto I = StatCache.find(File); + // Canonicalize to match the cached form. + // Lookup tends to be first by absolute path, so no need to make absolute. + llvm::SmallString<256> PathLookup(File); + llvm::sys::path::remove_dots(PathLookup, /*remove_dot_dot=*/true); + + auto I = StatCache.find(PathLookup); if (I != StatCache.end()) - return I->getValue(); + // Returned Status name should always match the requested File. + return llvm::vfs::Status::copyWithNewName(I->getValue(), File); return None; } diff --git a/clang-tools-extra/clangd/unittests/FSTests.cpp b/clang-tools-extra/clangd/unittests/FSTests.cpp index 044452cae1d4..4887b0a23144 100644 --- a/clang-tools-extra/clangd/unittests/FSTests.cpp +++ b/clang-tools-extra/clangd/unittests/FSTests.cpp @@ -34,7 +34,7 @@ TEST(FSTests, PreambleStatusCache) { // Main file is not cached. EXPECT_FALSE(StatCache.lookup(testPath("main")).hasValue()); - llvm::vfs::Status S("fake", llvm::sys::fs::UniqueID(0, 0), + llvm::vfs::Status S("fake", llvm::sys::fs::UniqueID(123, 456), std::chrono::system_clock::now(), 0, 0, 1024, llvm::sys::fs::file_type::regular_file, llvm::sys::fs::all_all); @@ -42,7 +42,15 @@ TEST(FSTests, PreambleStatusCache) { auto ConsumeFS = StatCache.getConsumingFS(FS); auto Cached = ConsumeFS->status(testPath("fake")); EXPECT_TRUE(Cached); - EXPECT_EQ(Cached->getName(), S.getName()); + EXPECT_EQ(Cached->getName(), testPath("fake")); + EXPECT_EQ(Cached->getUniqueID(), S.getUniqueID()); + + // fake and temp/../fake should hit the same cache entry. + // However, the Status returned reflects the actual path requested. + auto CachedDotDot = ConsumeFS->status(testPath("temp/../fake")); + EXPECT_TRUE(CachedDotDot); + EXPECT_EQ(CachedDotDot->getName(), testPath("temp/../fake")); + EXPECT_EQ(CachedDotDot->getUniqueID(), S.getUniqueID()); } } // namespace