[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 11 14:50:32 PDT 2018


ioeric added a comment.

After looking at a few more use cases of the in-memory file system, I think a problem we need to address is the consistency of file paths you get from clang when using in-mem vfs.  The clang-move tests that you have mitigated in https://reviews.llvm.org/D48951 could be an example.

For example, suppose you have header search directories `-I/path/to/include` and `-I.` in your compile command. When preprocessor searches for an #include "x.h", it will try to stat "/path/to/include/x.h" and "./x.h" and take the first one that exists. This can introduce indeterminism for the path (./x.h or /abs/x.h) you later get for the header file, e.g. when you try to look up file name by `FileID` through `SourceManager`. The path you get for a `FileEntry` or `FileID`  would depend on how clang looks up a file and how a file is first opened into `SourceManager`/`FileManager`.  It seems that the current behavior of clangd + in-memory file system would give you paths that are relative to the working directory for both cases. I'm not sure if that's the best behavior, but I think the consistency has its value. For example, in unit tests where in-memory file systems are heavily used, it's important to have a way to compare the reported file path (e.g. file path corresponding to a source location) with the expected paths. We could choose to always return real path, which could be potentially expensive, or we could require users to always compare real paths (it's unclear how well this would work though e.g. ClangTool doesn't expose its vfs), but they need to be enforced by the API. Otherwise, I worry it would cause more confusions for folks who use clang with in-memory file system in the future.


Repository:
  rC Clang

https://reviews.llvm.org/D48903





More information about the cfe-commits mailing list