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

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D48903#1159303, @ioeric wrote:

> 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.


It's hard to tell without seeing an actual failing use case.  I understand the argument, but I think the necessity to work as closely as `RealFileSystem` as possible is important.  Otherwise, it becomes impossible to reproduce bugs happening in the real world in unittests.


Repository:
  rC Clang

https://reviews.llvm.org/D48903