kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

It was used inside buildCompilerInvocation to speed up stats. But
preambleStatCache doesn't contain stat information performed while
building compiler invocation. So it was an unnecessary optimization.

This prepares the grounds for changing prepareCompilerInstance to take a
FSProvider instead of a VFS.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81719

Files:
  clang-tools-extra/clangd/Preamble.cpp


Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -207,35 +207,25 @@
 
 /// Scans the preprocessor directives in the preamble section of the file by
 /// running preprocessor over \p Contents. Returned includes do not contain
-/// resolved paths. \p VFS and \p Cmd is used to build the compiler invocation,
-/// which might stat/read files.
+/// resolved paths. \p Cmd is used to build the compiler invocation, which 
might
+/// stat/read files.
 llvm::Expected<ScannedPreamble>
-scanPreamble(llvm::StringRef Contents,
-             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+scanPreamble(llvm::StringRef Contents, const FileSystemProvider *FSProvider,
              const tooling::CompileCommand &Cmd) {
-  // FIXME: Change PreambleStatCache to operate on FileSystemProvider rather
-  // than vfs::FileSystem, that way we can just use ParseInputs without this
-  // hack.
-  auto GetFSProvider = [](llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
-    class VFSProvider : public FileSystemProvider {
+  auto EmptyFSProvider = [] {
+    class EmptyProvider : public FileSystemProvider {
     public:
-      VFSProvider(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
-          : VFS(std::move(FS)) {}
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
       getFileSystem() const override {
-        return VFS;
+        return new llvm::vfs::InMemoryFileSystem;
       }
-
-    private:
-      const llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
     };
-    return std::make_unique<VFSProvider>(std::move(FS));
+    return std::make_unique<EmptyProvider>();
   };
-  auto FSProvider = GetFSProvider(std::move(VFS));
   // Build and run Preprocessor over the preamble.
   ParseInputs PI;
   PI.Contents = Contents.str();
-  PI.FSProvider = FSProvider.get();
+  PI.FSProvider = FSProvider;
   PI.CompileCommand = Cmd;
   IgnoringDiagConsumer IgnoreDiags;
   auto CI = buildCompilerInvocation(PI, IgnoreDiags);
@@ -255,7 +245,7 @@
       std::move(CI), nullptr, std::move(PreambleContents),
       // Provide an empty FS to prevent preprocessor from performing IO. This
       // also implies missing resolved paths for includes.
-      new llvm::vfs::InMemoryFileSystem, IgnoreDiags);
+      EmptyFSProvider()->getFileSystem(), IgnoreDiags);
   if (Clang->getFrontendOpts().Inputs.empty())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "compiler instance had no inputs");
@@ -409,8 +399,6 @@
   trace::Span Tracer("CreatePreamblePatch");
   SPAN_ATTACH(Tracer, "File", FileName);
   assert(llvm::sys::path::is_absolute(FileName) && "relative FileName!");
-  auto VFS =
-      Baseline.StatCache->getConsumingFS(Modified.FSProvider->getFileSystem());
   // First scan preprocessor directives in Baseline and Modified. These will be
   // used to figure out newly added directives in Modified. Scanning can fail,
   // the code just bails out and creates an empty patch in such cases, as:
@@ -421,14 +409,15 @@
   //   there's nothing to do but generate an empty patch.
   auto BaselineScan = scanPreamble(
       // Contents needs to be null-terminated.
-      Baseline.Preamble.getContents().str(), VFS, Modified.CompileCommand);
+      Baseline.Preamble.getContents().str(), Modified.FSProvider,
+      Modified.CompileCommand);
   if (!BaselineScan) {
     elog("Failed to scan baseline of {0}: {1}", FileName,
          BaselineScan.takeError());
     return PreamblePatch::unmodified(Baseline);
   }
-  auto ModifiedScan =
-      scanPreamble(Modified.Contents, std::move(VFS), Modified.CompileCommand);
+  auto ModifiedScan = scanPreamble(Modified.Contents, Modified.FSProvider,
+                                   Modified.CompileCommand);
   if (!ModifiedScan) {
     elog("Failed to scan modified contents of {0}: {1}", FileName,
          ModifiedScan.takeError());


Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -207,35 +207,25 @@
 
 /// Scans the preprocessor directives in the preamble section of the file by
 /// running preprocessor over \p Contents. Returned includes do not contain
-/// resolved paths. \p VFS and \p Cmd is used to build the compiler invocation,
-/// which might stat/read files.
+/// resolved paths. \p Cmd is used to build the compiler invocation, which might
+/// stat/read files.
 llvm::Expected<ScannedPreamble>
-scanPreamble(llvm::StringRef Contents,
-             llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+scanPreamble(llvm::StringRef Contents, const FileSystemProvider *FSProvider,
              const tooling::CompileCommand &Cmd) {
-  // FIXME: Change PreambleStatCache to operate on FileSystemProvider rather
-  // than vfs::FileSystem, that way we can just use ParseInputs without this
-  // hack.
-  auto GetFSProvider = [](llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
-    class VFSProvider : public FileSystemProvider {
+  auto EmptyFSProvider = [] {
+    class EmptyProvider : public FileSystemProvider {
     public:
-      VFSProvider(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
-          : VFS(std::move(FS)) {}
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
       getFileSystem() const override {
-        return VFS;
+        return new llvm::vfs::InMemoryFileSystem;
       }
-
-    private:
-      const llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
     };
-    return std::make_unique<VFSProvider>(std::move(FS));
+    return std::make_unique<EmptyProvider>();
   };
-  auto FSProvider = GetFSProvider(std::move(VFS));
   // Build and run Preprocessor over the preamble.
   ParseInputs PI;
   PI.Contents = Contents.str();
-  PI.FSProvider = FSProvider.get();
+  PI.FSProvider = FSProvider;
   PI.CompileCommand = Cmd;
   IgnoringDiagConsumer IgnoreDiags;
   auto CI = buildCompilerInvocation(PI, IgnoreDiags);
@@ -255,7 +245,7 @@
       std::move(CI), nullptr, std::move(PreambleContents),
       // Provide an empty FS to prevent preprocessor from performing IO. This
       // also implies missing resolved paths for includes.
-      new llvm::vfs::InMemoryFileSystem, IgnoreDiags);
+      EmptyFSProvider()->getFileSystem(), IgnoreDiags);
   if (Clang->getFrontendOpts().Inputs.empty())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "compiler instance had no inputs");
@@ -409,8 +399,6 @@
   trace::Span Tracer("CreatePreamblePatch");
   SPAN_ATTACH(Tracer, "File", FileName);
   assert(llvm::sys::path::is_absolute(FileName) && "relative FileName!");
-  auto VFS =
-      Baseline.StatCache->getConsumingFS(Modified.FSProvider->getFileSystem());
   // First scan preprocessor directives in Baseline and Modified. These will be
   // used to figure out newly added directives in Modified. Scanning can fail,
   // the code just bails out and creates an empty patch in such cases, as:
@@ -421,14 +409,15 @@
   //   there's nothing to do but generate an empty patch.
   auto BaselineScan = scanPreamble(
       // Contents needs to be null-terminated.
-      Baseline.Preamble.getContents().str(), VFS, Modified.CompileCommand);
+      Baseline.Preamble.getContents().str(), Modified.FSProvider,
+      Modified.CompileCommand);
   if (!BaselineScan) {
     elog("Failed to scan baseline of {0}: {1}", FileName,
          BaselineScan.takeError());
     return PreamblePatch::unmodified(Baseline);
   }
-  auto ModifiedScan =
-      scanPreamble(Modified.Contents, std::move(VFS), Modified.CompileCommand);
+  auto ModifiedScan = scanPreamble(Modified.Contents, Modified.FSProvider,
+                                   Modified.CompileCommand);
   if (!ModifiedScan) {
     elog("Failed to scan modified contents of {0}: {1}", FileName,
          ModifiedScan.takeError());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to