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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to