Provide default virtual filesystem argument to ClangTool constructor

[vladimir.plyashkun]

Summary

This revision is part of another review.
Check this link for more details - https://reviews.llvm.org/D41535

[ilya-biryukov]

Overall LGTM, but could we add a test that checks the VFS is actually used?

[vladimir.plyashkun]

Implemented test-case to check that BaseFS is actually used in ClangTool

[ilya-biryukov]

Looks good. Do you have commit access or do you need someone to land this patch for you?

[vladimir.plyashkun]

In D41947#980298, @ilya-biryukov wrote:

Looks good. Do you have commit access or do you need someone to land this patch for you?

No, i don't have commit access.

[ilya-biryukov]

In D41947#980305, @vladimir.plyashkun wrote:

In D41947#980298, @ilya-biryukov wrote:

Looks good. Do you have commit access or do you need someone to land this patch for you?

No, i don't have commit access.

I can land this for you together with the other patch that you're working on.
BTW, have you seen that you could set a child revision in phabricator? That would link these two changes together, making it clear that those changes are closely related.

[ilya-biryukov]

include/clang/Tooling/Tooling.h

299

NIT: LLVM coding style requires full stop at the end of comments.
Could we also rephrase it to not mention the OverlayFileSystem, the important bit seems to be that we use it for all fs operations. Something like:

/// \param BaseFS VFS used for all underlying file accesses when running the tool.

[vladimir.plyashkun]

Fixed comment

include/clang/Tooling/Tooling.h

public:
   /// \brief Constructs a clang tool to run over a list of files.
   ///
   /// \param Compilations The CompilationDatabase which contains the compile
   ///        command lines for the given source paths.
   /// \param SourcePaths The source files to run over. If a source files is
   ///        not found in Compilations, it is skipped.
   /// \param PCHContainerOps The PCHContainerOperations for loading and creating
   /// clang modules.
+  /// \param BaseFS VFS used for all underlying file accesses when running the
+  /// tool.
   ClangTool(const CompilationDatabase &Compilations,
             ArrayRef<std::string> SourcePaths,
             std::shared_ptr<PCHContainerOperations> PCHContainerOps =
-                std::make_shared<PCHContainerOperations>());
+                std::make_shared<PCHContainerOperations>(),
+            IntrusiveRefCntPtr<vfs::FileSystem> BaseFS =
+                vfs::getRealFileSystem());
 
   ~ClangTool();
 
   /// \brief Set a \c DiagnosticConsumer to use during parsing.
   void setDiagnosticConsumer(DiagnosticConsumer *DiagConsumer) {
     this->DiagConsumer = DiagConsumer;
   }
 

lib/Tooling/Tooling.cpp

bool FrontendActionFactory::runInvocation(
   const bool Success = Compiler.ExecuteAction(*ScopedToolAction);
 
   Files->clearStatCaches();
   return Success;
 }
 
 ClangTool::ClangTool(const CompilationDatabase &Compilations,
                      ArrayRef<std::string> SourcePaths,
-                     std::shared_ptr<PCHContainerOperations> PCHContainerOps)
+                     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
+                     IntrusiveRefCntPtr<vfs::FileSystem> BaseFS)
     : Compilations(Compilations), SourcePaths(SourcePaths),
       PCHContainerOps(std::move(PCHContainerOps)),
-      OverlayFileSystem(new vfs::OverlayFileSystem(vfs::getRealFileSystem())),
+      OverlayFileSystem(new vfs::OverlayFileSystem(BaseFS)),
       InMemoryFileSystem(new vfs::InMemoryFileSystem),
       Files(new FileManager(FileSystemOptions(), OverlayFileSystem)),
       DiagConsumer(nullptr) {
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   appendArgumentsAdjuster(getClangStripOutputAdjuster());
   appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster());
   appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 }

unittests/Tooling/ToolingTest.cpp

TEST(ClangToolTest, ArgumentAdjusters) {
   Tool.clearArgumentsAdjusters();
   Tool.appendArgumentsAdjuster(CheckSyntaxOnlyAdjuster);
   Tool.appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster());
   Tool.run(Action.get());
   EXPECT_TRUE(Ran);
   EXPECT_FALSE(Found);
 }
 
+TEST(ClangToolTest, BaseVirtualFileSystemUsage) {
+  FixedCompilationDatabase Compilations("/", std::vector<std::string>());
+  llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> OverlayFileSystem(
+      new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+  llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
+      new vfs::InMemoryFileSystem);
+  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+
+  InMemoryFileSystem->addFile(
+      "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "a.cpp"),
+                 std::make_shared<PCHContainerOperations>(), OverlayFileSystem);
+  std::unique_ptr<FrontendActionFactory> Action(
+      newFrontendActionFactory<SyntaxOnlyAction>());
+  EXPECT_EQ(0, Tool.run(Action.get()));
+}
+
 // Check getClangStripDependencyFileAdjuster doesn't strip args after -MD/-MMD.
 TEST(ClangToolTest, StripDependencyFileAdjuster) {
   FixedCompilationDatabase Compilations("/", {"-MD", "-c", "-MMD", "-w"});
 
   ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc"));
   Tool.mapVirtualFile("/a.cc", "void a() {}");
 
   std::unique_ptr<FrontendActionFactory> Action(