[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

Thu Oct 6 09:36:29 PDT 2022

goncharov added a comment.

In D135220#3840354 <https://reviews.llvm.org/D135220#3840354>, @benlangmuir wrote:

> Okay, I was able to reproduce by symlinking all the 0-byte header files to header0 (any choice probably works).  The behaviour is deterministic before and after my change.
>
> This was only passing by luck in this setup, because it was relying on mutation of `FileEntry->LastRef` which mutates the path of header1 to header3.  We do not actually track the difference in filename here, it just happened to match the behaviour without symlinks for this case because of the mutation.  Note: the mutation is not triggered specifically by the include, it's anything that looks up that path in the file manager, so there is no guarantee that you would get header3 if something triggered accessing the filename header1 again at the wrong time.
>
> I think my change is fine here and we should just update the test files so they will not be accidentally linked. If someone wants to work on tracking the filenames independently for each include that would be fine, but as long as we only have one name it should be the first one not the "one whose path was most recently accessed in FileManager".
>
> @goncharov Was this the only test effected? If so, here's a fix: https://reviews.llvm.org/D135373

I believe so, thank you for the fix! (I was mostly trying to confirm that symlink setups will be fine)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135220/new/

https://reviews.llvm.org/D135220