[PATCH] D124816: [Tooling] use different FileManager for each CWD

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added reviewers: benlangmuir, bnbarham, jansvoboda11.
dexonsmith added a comment.

Two high-level comments:

- Making these functions `virtual` makes it harder to reason about possible implementations, but I don't think that's necessary for what you're doing here. Why not expose `ToolFileManager` as a container of filemanagers which ToolInvocation takes by parameter, then calls `getOrCreateFileManager()` on at time of need passing relevant arguments (e.g., CWD)?
  - This could also create/manage the relevant VFS instance, to which the FileManager is fundamentally tied. See also below.
- I don't think this goes far enough. The FileManager keeps state for any accessed file, which can be wrong any time the VFS changes at all, not just if the `CWD` is different. E.g., different `-ivfsoverlay` options are passed, or someone is reading from an `InMemoryFileSystem` vs. from disk.
  - AFAICT, every call to `FileManager::setVirtualFileSystem()` is fundamentally unsound unless the new FS is equivalent to the old one or the filemanager hasn't been used yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124816