replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[clangd] Make PreambleStatusCache handle filenames more carefully 

Summary:
 - when we hit the cache, the reported filename should be that of the
   cache query, not that of the cache store. This matches behaviors of
   common FSes, and avoids triggering difficult edge cases in
   FileManager when files are being moved around concurrently.
 - filename comparisons (both cache queries and == mainfile checks)
   should fold away . and .. in paths. These can appear when relative
   paths occur in compile_commands.json. (gn does this).

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D63931

llvm-svn: 364740
This commit is contained in:
Sam McCall 2019-07-01 10:11:18 +00:00
parent 4f878fe3a7
commit 9cca81344c
2 changed files with 25 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
clang-tools-extra/clangd/FS.cpp
View File

@ -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<llvm::vfs::Status>
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;
}

12
clang-tools-extra/clangd/unittests/FSTests.cpp
View File

@ -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