<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 31 Jul 2019 at 12:29, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,<div><br></div><div>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.</div><div><br></div><div>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:</div><div><br></div><div>- Return FileEntryRef that stores the FileEntry * and a Name.</div><div>- Don't store the Name in the FileEntry (keep the RealPathName) still.</div><div>- 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).</div><div>- DenseMaps will still mostly use the FileEntry * values to preserve the existing behavior which expects one FileEntry per inode.</div></div></div></div></div></div></blockquote><div><br></div><div><div>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:</div><div><a href="https://reviews.llvm.org/D65907">https://reviews.llvm.org/D65907</a><br></div><div><br></div><div>Cheers,</div><div>Alex</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><br></div><div>Additionally to fixing the issue described above, Harlan (CCed) and I would like to also perform the following minor cleanups and improvements:</div><div><br></div><div>- Start using error_code/ErrorOr for FileManager for improved API.</div><div>- Get rid of the MemorizeStatCalls stat cache, which isn't actually used right now by Clang (probably a bug)!</div><div>- Cache the VFS stat value in the FileEntryRef itself, so that the FileManager can actually have proper caching that works.</div><div><br></div><div>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.</div><div><br></div><div>Thanks,</div><div>Alex</div></div></div></div></div></div>
</blockquote></div></div></div>