ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed.
Sorry for the delay ================ Comment at: clang-tidy/ClangTidy.cpp:93 + vfs::OverlayFileSystem &OverlayFS) { + if (OverlayFile.empty()) + return; ---------------- `DiagnosticsEngine& ` seems to be used to report errors in the source code, while the things we're seeing here are errors when initializing `clang-tidy`. Could we move the code that creates vfs into `ClangTidyMain.cpp` and report errors directly into `llvm::errs`, similar to how we do that for other flags like `list-checks`, etc. ================ Comment at: clang-tidy/ClangTidy.cpp:115 ErrorReporter(ClangTidyContext &Context, bool ApplyFixes) - : Files(FileSystemOptions()), DiagOpts(new DiagnosticOptions()), + : OverlayFS(new vfs::OverlayFileSystem(vfs::getRealFileSystem())), + Files(FileSystemOptions(), OverlayFS), ---------------- We should pass `BaseFS` here as a parameter, similar to how we do that in `ClangTool`. Piping it here from `clangdTidyMain` seems trivial. ================ Comment at: clang-tidy/ClangTidy.cpp:556 + if (Context.getOptions().VfsOverlay) { + pushVfsOverlayFromFile(*Context.getOptions().VfsOverlay, ---------------- We should construct vfs before passing it to `ClangTool` and not modify it later. I suggest creating `BaseFS` in `clangTidyMain`, see other comments too. ================ Comment at: clang-tidy/ClangTidyOptions.h:114 + /// over the real file system + llvm::Optional<std::string> VfsOverlay; }; ---------------- I think we should avoid putting `VfsOverlay` into `ClangTidyOptions`. Is there any reason why having a flag in `ClangTidyMain.cpp` is not enough? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41535 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits