whisperity added inline comments.

Comment at: clang-tidy/ClangTidy.cpp:91
-  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 

Comment at: clang-tidy/tool/CMakeLists.txt:19
+  clangFrontend
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.


cfe-commits mailing list

Reply via email to