ilya-biryukov added subscribers: hokein, alexfh.
ilya-biryukov 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()),
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
+  clangFrontend
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`?

cfe-commits mailing list

Reply via email to