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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

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

> In https://reviews.llvm.org/D48903#1155403, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D48903#1154846, @simark wrote:
> >
> > > With the `InMemoryFileSystem`, this now returns a non-real path.  The result is that we fill `RealPathName` with that non-real path.  I see two options here:
> > >
> > > 1. Either the FileManager is wrong to assume that `File::getName` returns a real path, and should call `FS->getRealPath` to do the job.
> > > 2. If the contract is that the ` File::getName` interface should return a real path, then we should fix the `File::getName` implementation to do that somehow.
> > >
> > >   I would opt for 1.  This way, we could compute the `RealPathName` field even if we don't open the file (unlike currently).
> >
> >
> > I'd also say `FileManager` should use `FileSystem::getRealPath`. The code that does it differently was there before `FileSystem::getRealPath` was added.
> >  And `RealFile` should probably not do any magic in `getName`, we could add a separate method for (`getRealName`?) if that's absolutely needed.
> >
> > Refactorings in that area would probably break stuff and won't be trivial and I don't think this change should be blocked by those. So I'd be happy if this landed right away with a FIXME in `FileManager` mentioning that `InMemoryFileSystem` might give surprising results there.
> >  @ioeric added `FileSystem::getRealPath`, he may more ideas on how we should proceed.
>
>
> Sorry for being late in the discussion. I'm not sure if `getRealPath` is the right thing to do in `FileManager` as it also supports virtual files added via `FileManager::getVirtualFile`, and they don't necessary have a real path. This would also break users of `ClangTool::mapVirtualFile` when the mapped files are relative. As a matter of fact, I'm already seeing related breakages in our downstream branch :(
>
> Would you mind reverting this patch for now so that we can come up with a solution to address those use cases?
>
> Sorry again about missing the discussion earlier!


Sorry, having taken a closer look, it seems that only `ClangTool::mapVirtualFile` would be affected. I'll try to see if there is an easy fix.


Repository:
  rC Clang

https://reviews.llvm.org/D48903