bnbarham accepted this revision. bnbarham added inline comments.
================ Comment at: clang/include/clang/Basic/SourceManager.h:146-155 /// References the file which the contents were actually loaded from. /// /// Can be different from 'Entry' if we overridden the contents of one file /// with the contents of another file. const FileEntry *ContentsEntry; /// The filename that is used to access OrigEntry. ---------------- I haven't checked all uses, but do we still need all of `OrigEntry`/`ContentsEntry`/`Filename`? Seems like a single `FileEntryRef` should encapsulate all the information we need there. ================ Comment at: clang/include/clang/Basic/SourceManager.h:1062 + Optional<FileEntryRef> F = sloc.getFile().getContentCache().OrigEntry; + return F ? &F->getFileEntry() : nullptr; } ---------------- jansvoboda11 wrote: > Could you wrap `F` in `OptionalFileEntryRefDegradesToFileEntryPtr` to handle > the conversion for you? `OrigEntry` is a `OptionalFileEntryRefDegradesToFileEntryPtr`, so this should just be able to be `return slot.getFile().getContentCache().OrigEntry`? ================ Comment at: clang/lib/Lex/ModuleMap.cpp:1030 + Optional<FileEntryRef> ModuleMapRef = getModuleMapFileForUniquing(Parent); + ModuleMapFile = ModuleMapRef ? *ModuleMapRef : nullptr; + } ---------------- jansvoboda11 wrote: > Nit: Use `OptionalFileEntryRefDegradesToFileEntryPtr`? Maybe we should return > that from `getModuleMapFileForUniquing` in the first place. I like the idea of returning `OptionalFileEntryRefDegradesToFileEntryPtr` until we move all this to use `FileEntryRef` as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135220/new/ https://reviews.llvm.org/D135220 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
