ilya-biryukov added subscribers: hokein, alexfh. ilya-biryukov added inline comments.
================ Comment at: clang-tidy/ClangTidy.cpp:91 public: - ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, - llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) - : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()), + ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, ClangTool &Tool) + : Files(FileSystemOptions(), Tool.getFiles().getVirtualFileSystem()), ---------------- Why do we need to change the signature of `ErrorReporter` constructor? Broadening the accepted interface does not seem right. It only needs the vfs and the clients could get vfs from `ClangTool` themselves. ================ Comment at: clang-tidy/ClangTidy.h:229 void runClangTidy(clang::tidy::ClangTidyContext &Context, - const tooling::CompilationDatabase &Compilations, - ArrayRef<std::string> InputFiles, - llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS, + clang::tooling::ClangTool &Tool, ProfileData *Profile = nullptr); ---------------- I'm not sure if passing `ClangTool` here is an improvement. Let's ask someone more familiar with clang-tidy. @hokein, @alexfh, WDYT? ================ Comment at: clang-tidy/tool/CMakeLists.txt:19 clangBasic + clangFrontend clangTidy ---------------- Why do we need an extra dependency? ================ Comment at: clang-tidy/tool/ClangTidyMain.cpp:432 } - llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS( - VfsOverlay.empty() ? vfs::getRealFileSystem() + llvm::IntrusiveRefCntPtr<vfs::FileSystem> VirtualFileSystem( + VfsOverlay.empty() ? nullptr ---------------- Maybe use a shorter name, e.g. `FileSystem`? https://reviews.llvm.org/D45095 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits