whisperity created this revision.
whisperity added reviewers: alexfh, klimek.
whisperity added a project: clang.
Herald added subscribers: dkrupp, rnkovacs.

This patch extends upon https://reviews.llvm.org/D41947 because the interface 
that was landed from that patch isn't much user-friendly.

Injecting a custom virtual file system into the tool is a dangerous operation. 
If the real file system (where the installed Clang's headers reside) are not 
mirrored, it only turns out at `ClangTool::run()` that something was not 
mounted properly. Originally, the execution of a `ClangTool` always used the 
real filesystem.

Starting with https://reviews.llvm.org/D41947, the client code setting the tool 
up must manually create the OverlayFS  (which is, in turn, added as an 
OverlayFS into the tool' inner OverlayFS) containing the real system and the 
user's intention. I believe this logic should not be duplicated in client code, 
and the more traditional use-case of overlaying the filesystem view with some 
files, but by default keeping the real deal beneath it must be reflected better 
on the interface. Client code can now much more **explicitly** specify the 
complete cessation of real filesystem usage.


Repository:
  rC Clang

https://reviews.llvm.org/D45094

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===================================================================
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -402,24 +402,48 @@
   EXPECT_FALSE(Found);
 }
 
-TEST(ClangToolTest, BaseVirtualFileSystemUsage) {
+TEST(ClangToolVFSTest, MustGiveFileSystem) {
+  FixedCompilationDatabase Compilations("/", std::vector<std::string>());
+
+  EXPECT_DEATH(ClangTool(Compilations, std::vector<std::string>(),
+                         std::make_shared<PCHContainerOperations>(),
+                         nullptr, false),
+               "without providing a FileSystem");
+}
+
+TEST(ClangToolVFSTest, VirtualFileSystemUsage) {
   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);
+    new vfs::InMemoryFileSystem);
 
   InMemoryFileSystem->addFile(
-      "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+    "/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
 
-  ClangTool Tool(Compilations, std::vector<std::string>(1, "a.cpp"),
-                 std::make_shared<PCHContainerOperations>(), OverlayFileSystem);
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cpp"),
+                 std::make_shared<PCHContainerOperations>(),
+                 InMemoryFileSystem, false);
   std::unique_ptr<FrontendActionFactory> Action(
-      newFrontendActionFactory<SyntaxOnlyAction>());
+    newFrontendActionFactory<SyntaxOnlyAction>());
   EXPECT_EQ(0, Tool.run(Action.get()));
 }
 
+TEST(ClangToolVFSTest, VFSDoesntContainEveryFile) {
+  FixedCompilationDatabase Compilations("/", std::vector<std::string>());
+  llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
+    new vfs::InMemoryFileSystem);
+
+  InMemoryFileSystem->addFile(
+    "/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include <cstdio>\n"
+                                                   "int main() {}"));
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cpp"),
+                 std::make_shared<PCHContainerOperations>(),
+                 InMemoryFileSystem, false);
+  std::unique_ptr<FrontendActionFactory> Action(
+    newFrontendActionFactory<SyntaxOnlyAction>());
+  EXPECT_NE(0, Tool.run(Action.get()));
+}
+
 // Check getClangStripDependencyFileAdjuster doesn't strip args after -MD/-MMD.
 TEST(ClangToolTest, StripDependencyFileAdjuster) {
   FixedCompilationDatabase Compilations("/", {"-MD", "-c", "-MMD", "-w"});
Index: lib/Tooling/Tooling.cpp
===================================================================
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -358,13 +358,22 @@
 ClangTool::ClangTool(const CompilationDatabase &Compilations,
                      ArrayRef<std::string> SourcePaths,
                      std::shared_ptr<PCHContainerOperations> PCHContainerOps,
-                     IntrusiveRefCntPtr<vfs::FileSystem> BaseFS)
+                     IntrusiveRefCntPtr<vfs::FileSystem> FileSystem,
+                     bool AlsoUseRealFileSystem)
     : Compilations(Compilations), SourcePaths(SourcePaths),
       PCHContainerOps(std::move(PCHContainerOps)),
-      OverlayFileSystem(new vfs::OverlayFileSystem(std::move(BaseFS))),
-      InMemoryFileSystem(new vfs::InMemoryFileSystem),
-      Files(new FileManager(FileSystemOptions(), OverlayFileSystem)) {
+      InMemoryFileSystem(new vfs::InMemoryFileSystem) {
+  assert(!(!AlsoUseRealFileSystem && !FileSystem) && "ClangTool initialized "
+         "without providing a FileSystem to get files from!");
+  if (AlsoUseRealFileSystem) {
+    OverlayFileSystem = new vfs::OverlayFileSystem(vfs::getRealFileSystem());
+    if (FileSystem)
+      OverlayFileSystem->pushOverlay(FileSystem);
+  }
+  else
+    OverlayFileSystem = new vfs::OverlayFileSystem(FileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+  Files = new FileManager(FileSystemOptions(), OverlayFileSystem);
   appendArgumentsAdjuster(getClangStripOutputAdjuster());
   appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster());
   appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
Index: include/clang/Tooling/Tooling.h
===================================================================
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -296,22 +296,24 @@
 /// arguments adjuster by calling the appendArgumentsAdjuster() method.
 class ClangTool {
 public:
-  /// \brief Constructs a clang tool to run over a list of files.
+  /// \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.
+  ///        clang modules.
+  /// \param FileSystem The Virtual File System that is used to retrieve file
+  ///        contents by the tool.
+  /// \param AlsoUseRealFileSystem Sets the Clang tool to also use the machine's
+  ///        filesystem as the lowest layer to retrieve files from.
   ClangTool(const CompilationDatabase &Compilations,
             ArrayRef<std::string> SourcePaths,
             std::shared_ptr<PCHContainerOperations> PCHContainerOps =
                 std::make_shared<PCHContainerOperations>(),
-            IntrusiveRefCntPtr<vfs::FileSystem> BaseFS =
-                vfs::getRealFileSystem());
+            IntrusiveRefCntPtr<vfs::FileSystem> FileSystem = nullptr,
+            bool AlsoUseRealFileSystem = true);
 
   ~ClangTool();
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to