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