[clangd] Avoid cache main file status in preamble.

Summary: Main file can certainly change when reusing preamble.

Reviewers: sammccall

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

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

llvm-svn: 344024

diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp
index 5717bc8..4cc6ef6 100644 (file)
--- a/clang-tools-extra/clangd/ClangdUnit.cpp
+++ b/clang-tools-extra/clangd/ClangdUnit.cpp
@@ -29,6 +29,7 @@
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/raw_ostream.h"
@@ -336,7 +337,9 @@ std::shared_ptr<const PreambleData> clangd::buildPreamble(
     // dirs.
   }
 
-  auto StatCache = llvm::make_unique<PreambleFileStatusCache>();
+  llvm::SmallString<32> AbsFileName(FileName);
+  Inputs.FS->makeAbsolute(AbsFileName);
+  auto StatCache = llvm::make_unique<PreambleFileStatusCache>(AbsFileName);
   auto BuiltPreamble = PrecompiledPreamble::Build(
       CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
       StatCache->getProducingFS(Inputs.FS), PCHs, StoreInMemory,

diff --git a/clang-tools-extra/clangd/FS.cpp b/clang-tools-extra/clangd/FS.cpp
index 3fe0c8e..fa94b96 100644 (file)
--- a/clang-tools-extra/clangd/FS.cpp
+++ b/clang-tools-extra/clangd/FS.cpp
@@ -10,14 +10,23 @@
 #include "FS.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "llvm/ADT/None.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 
+PreambleFileStatusCache::PreambleFileStatusCache(llvm::StringRef MainFilePath)
+    : MainFilePath(MainFilePath) {
+  assert(llvm::sys::path::is_absolute(MainFilePath));
+}
+
 void PreambleFileStatusCache::update(const vfs::FileSystem &FS, vfs::Status S) {
   SmallString<32> PathStore(S.getName());
   if (FS.makeAbsolute(PathStore))
     return;
+  // Do not cache status for the main file.
+  if (PathStore == MainFilePath)
+    return;
   // Stores the latest status in cache as it can change in a preamble build.
   StatCache.insert({PathStore, std::move(S)});
 }

diff --git a/clang-tools-extra/clangd/FS.h b/clang-tools-extra/clangd/FS.h
index 5f0fe62..33a8ac9 100644 (file)
--- a/clang-tools-extra/clangd/FS.h
+++ b/clang-tools-extra/clangd/FS.h
@@ -17,10 +17,10 @@ namespace clang {
 namespace clangd {
 
 /// Records status information for files open()ed or stat()ed during preamble
-/// build, so we can avoid stat()s on the underlying FS when reusing the
-/// preamble. For example, code completion can re-stat files when getting FileID
-/// for source locations stored in preamble (e.g. checking whether a location is
-/// in the main file).
+/// build (except for the main file), so we can avoid stat()s on the underlying
+/// FS when reusing the preamble. For example, code completion can re-stat files
+/// when getting FileID for source locations stored in preamble (e.g. checking
+/// whether a location is in the main file).
 ///
 /// The cache is keyed by absolute path of file name in cached status, as this
 /// is what preamble stores.
@@ -35,7 +35,12 @@ namespace clangd {
 /// Note that the cache is only valid when reusing preamble.
 class PreambleFileStatusCache {
 public:
+  /// \p MainFilePath is the absolute path of the main source file this preamble
+  /// corresponds to. The stat for the main file will not be cached.
+  PreambleFileStatusCache(llvm::StringRef MainFilePath);
+
   void update(const vfs::FileSystem &FS, vfs::Status S);
+
   /// \p Path is a path stored in preamble.
   llvm::Optional<vfs::Status> lookup(llvm::StringRef Path) const;
 
@@ -56,6 +61,7 @@ public:
   getConsumingFS(IntrusiveRefCntPtr<vfs::FileSystem> FS) const;
 
 private:
+  std::string MainFilePath;
   llvm::StringMap<vfs::Status> StatCache;
 };
 

diff --git a/clang-tools-extra/unittests/clangd/FSTests.cpp b/clang-tools-extra/unittests/clangd/FSTests.cpp
index 3b02842..64e6f8a 100644 (file)
--- a/clang-tools-extra/unittests/clangd/FSTests.cpp
+++ b/clang-tools-extra/unittests/clangd/FSTests.cpp
@@ -20,16 +20,20 @@ TEST(FSTests, PreambleStatusCache) {
   llvm::StringMap<std::string> Files;
   Files["x"] = "";
   Files["y"] = "";
+  Files["main"] = "";
   auto FS = buildTestFS(Files);
   FS->setCurrentWorkingDirectory(testRoot());
 
-  PreambleFileStatusCache StatCache;
+  PreambleFileStatusCache StatCache(testPath("main"));
   auto ProduceFS = StatCache.getProducingFS(FS);
   EXPECT_TRUE(ProduceFS->openFileForRead("x"));
   EXPECT_TRUE(ProduceFS->status("y"));
+  EXPECT_TRUE(ProduceFS->status("main"));
 
   EXPECT_TRUE(StatCache.lookup(testPath("x")).hasValue());
   EXPECT_TRUE(StatCache.lookup(testPath("y")).hasValue());
+  // Main file is not cached.
+  EXPECT_FALSE(StatCache.lookup(testPath("main")).hasValue());
 
   vfs::Status S("fake", llvm::sys::fs::UniqueID(0, 0),
                 std::chrono::system_clock::now(), 0, 0, 1024,