whisperity 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()), ---------------- ilya-biryukov wrote: > 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. Is it okay interface-wise if the FS used by the `ErrorReporter` is **not** the same as the one used by the module that produced the errors? It seems like an undocumented consensus/convention that the same VFS should have been passed here. ================ Comment at: clang-tidy/tool/CMakeLists.txt:19 clangBasic + clangFrontend clangTidy ---------------- ilya-biryukov wrote: > Why do we need an extra dependency? In the current state of the patch, `clang-tidy/tool/ClangTidyMain.cpp` constructs the `ClangTool` which constructor requires a `std::shared_ptr<PCHContainerOperations>`. `PCHContainerOperations`'s definition and implementation is part of the `clangFrontend` library, and without this dependency, there would be a symbol resolution error. https://reviews.llvm.org/D45095 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
