[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

I think we also do some registry searching, which is not virtualized. I guess those are all absolute references and paths, so if you use clangd on the same machine, it should just work.

It occurs to me that this code is suddenly much more unit testable now that it uses a VFS, but it doesn't seem reasonable to request unit tests.



================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:361
 // from clang itself to prevent clang from falling back to itself.
-static std::string FindVisualStudioExecutable(const ToolChain &TC,
+static std::string FindVisualStudioExecutable(llvm::vfs::FileSystem &VFS,
+                                              const ToolChain &TC,
----------------
This function already takes TC, which has TC.getVFS, so I think you can skip the extra parameter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97437/new/

https://reviews.llvm.org/D97437