[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-26 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Thanks for fixing this :) 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://

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Adrian McCarthy via Phabricator via cfe-commits
amccarth added a comment. LGTM. 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

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6d52c4819294: Rewrite MSVC toolchain discovery with VFS (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. We don't use a VFS for registry searching because we just take the value given by the registry and pass it on, rather than checking for the existence of specific files/directories. But yeah they're probably all absolute paths so it shouldn't matter in those cases. Re

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 326455. aeubanks added a comment. remove extra parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ https://reviews.llvm.org/D97437 Files: clang/lib/Driver/ToolChains/MSVC.cpp Index: clang/li

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread 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 oc

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. Looks like -ivfsoverlay doesn't take effect in the driver. And -working-directory also changes the program's working directory since there's no vfs in the driver so it uses the actual fs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. In `Linux.cpp` I only see uses of VFS, no uses of `llvm::sys::fs` so I think it should work there. And using clangd on vscode on Linux, it works with --sysroot. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ htt

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. (aside: Do you know if the sysroot vs clangd stuff works on non-win? I happened to see https://bugs.llvm.org/show_bug.cgi?id=27909) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ https://reviews.llvm.org/D97437 _

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-25 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Re test: Maybe some combination of `-ivfsoverlay` (as a `/clang:` flag I suppose) and `/winsysroot` can be used to test this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97437/new/ https://reviews.llvm.org/D97437 ___

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-24 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. not exactly sure how to test this... 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

[PATCH] D97437: Rewrite MSVC toolchain discovery with VFS

2021-02-24 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision. aeubanks added reviewers: rnk, thakis, amccarth. Herald added subscribers: usaxena95, kadircet. aeubanks requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. This fixes an issue where the toolchain d