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

Fri Jan 27 12:08:26 PST 2023

jansvoboda11 added inline comments.

================
Comment at: clang/lib/Lex/ModuleMap.cpp:663
       Result->IsInferred = true;
-      Result->addTopHeader(File);
+      Result->addTopHeader(File->getLastRef());

----------------
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()`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142724