[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 10 18:04:02 PST 2020


dexonsmith added a comment.

In D92160#2447586 <https://reviews.llvm.org/D92160#2447586>, @dexonsmith wrote:

>   /// Current working directory. Grabbed from the (new) VFS whenever it's
>   /// changed, and updated if the file manager is notified that the VFS's
>   /// CWD changes.
>   std::string CWD;
>   
>   struct RelativePaths {
>     StringMap<FileEntryRef::MapValue> Files;
>     StringMap<llvm::ErrorOr<DirectoryEntry &>> Dirs;
>   };
>   
>   /// Relative entries for the current CWD.
>   RelativePaths RelativeEntries;
>   
>   /// Cached relative entries for other CWDs.
>   Optional<StringMap<RelativePaths>> CachedRelativeEntries;

(If we ended up doing something like this, I would structure it as at least two patches: first refactoring the current semantics to use `RelativeEntries` and `CWD`, likely all NFC / no functionality change, and then a final patch to add `CachedRelativeEntries` and add support for changing directories.)

Another option here is to assume the VFS never changes directories (no need to store the `CWD`), and cache relative entries like this:

  Optional<DenseMap<IntrusiveRefCntPtr<FileSystem>, RelativePaths>>
      CachedRelativeEntries;

Then if ClangTool wants to change directories, it would be expected to maintain an appropriate set of VFS layers (changing directories would be a different call to `getPhysicalFileSystem`).

....but this highlights another concern, which is that even the absolute paths depend on the VFS that's in place, since options like `-ivfsoverlay` can change for different compilation commands. It's not clear to me how well this currently works with a shared `FileManager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92160



More information about the cfe-commits mailing list