[PATCH] D142724: [clang] use FileEntryRef for SUBMODULE_TOPHEADER

[Richard Howell via Phabricator via cfe-commits]

rmaz added inline comments.


================
Comment at: clang/lib/Lex/ModuleMap.cpp:663
       Result->IsInferred = true;
-      Result->addTopHeader(File);
+      Result->addTopHeader(File->getLastRef());
 
----------------
jansvoboda11 wrote:
> rmaz wrote:
> > jansvoboda11 wrote:
> > > How much work would be to refactor `File` to be `FileEntryRef`? Would be good if we didn't need to resort to `getLastRef()`.
> > It is unfortunately a fair bit. I spent some time on it but hit a roadblock with the FileManagers VirtualFileEntries:
> > 
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileManager.h#L70-L71
> > 
> > We would require this to be refactored to FileEntry too, as it is used here to provide a UID mapping to FileEntry pointers which needs to be changed to refs:
> > 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/FileManager.cpp#L614-L633
> > 
> > I can put up a follow on diff once this one is shipped if preferred?
> I was able to refactor the UID mapping to use `FileEntryRef` in D142780. Given that, I think it would be preferable to propagate these into `findOrCreateModuleForHeaderInUmbrellaDir()` and remove `getLastRef()`.
Nice, i'll rebase this and add the remaining changes once its shipped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142724