[cfe-dev] RFC: Improving Clang's FileManager

[Alex L via cfe-dev]

On Wed, 31 Jul 2019 at 12:29, Alex L <arphaman at gmail.com> wrote:

> Hi,
>
> As it stands right now, Clang's FileManager doesn't model file entries
> with multiple names correctly. Whenever the `getFile` method is called with
> a path it hasn't seen before, it will return a FileEntry whose name is
> guaranteed to be that path. However, this guarantee is lost whenever a
> symlink file that points to original path is opened using `getFile`, as the
> `name` of the FileEntry is overwritten with the symlink path. This happens
> because the FileManager uniques the FileEntries using inodes, which means
> that symlinks always get the same file entry as the original file.
>
> I would like to fix this issue as it's blocking clang-scan-deps from
> reusing the FileManager across multiple invocations. Here's my idea:
>
> - Return FileEntryRef that stores the FileEntry * and a Name.
> - Don't store the Name in the FileEntry (keep the RealPathName) still.
> - A comparison between FileEntryRef will compare the pointer values of
> FileEntries, so the code that compares the pointer values right now can
> still compare two FileEntryRefs with the equivalent result (important for
> modules).
> - DenseMaps will still mostly use the FileEntry * values to preserve the
> existing behavior which expects one FileEntry per inode.
>

I posted a patch that introduces FileEntryRef with a parallel FileManager
API and migrates some clients that are used to deal with includes. You can
find it here:
https://reviews.llvm.org/D65907

Cheers,
Alex


>
> Additionally to fixing the issue described above, Harlan (CCed) and I
> would like to also perform the following minor cleanups and improvements:
>
> - Start using error_code/ErrorOr for FileManager for improved API.
> - Get rid of the MemorizeStatCalls stat cache, which isn't actually used
> right now by Clang (probably a bug)!
> - Cache the VFS stat value in the FileEntryRef itself, so that the
> FileManager can actually have proper caching that works.
>
> Please let me know if you have any feedback. I'm hoping to post a patch
> for FileEntryRef sometime in the next 1-2 weeks.
>
> Thanks,
> Alex
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190807/014a563f/attachment.html>