dexonsmith added inline comments.
================ Comment at: clang/include/clang/Basic/SourceManager.h:135-139 /// 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; ---------------- It's possible this could be factored out as a follow-up, and/or moved up to the Named ContentCache level. (Not sure... asking a question really...) ================ Comment at: clang/include/clang/Basic/SourceManager.h:147-163 /// Indicates whether the buffer itself was provided to override /// the actual file contents. /// /// When true, the original entry may be a virtual file that does not /// exist. unsigned BufferOverridden : 1; ---------------- Have you thought about whether it makes sense for these fields to be shared for all `FileEntryRef`s to the same `FileEntry`? (Maybe it does! just checking) ================ Comment at: clang/include/clang/Basic/SourceManager.h:261-263 + /// FIXME: Make non-optional using a virtual file as needed, remove \c + /// Filename and use \c OrigEntry.getNameAsRequested() instead. + OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry; ---------------- I think this patch should (or does?) implement this FIXME. ================ Comment at: clang/include/clang/Basic/SourceManager.h:265-268 + /// The filename that is used to access OrigEntry. + /// + /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name. + StringRef Filename; ---------------- I think you can delete this field. You're effectively implementing the FIXME. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137304/new/ https://reviews.llvm.org/D137304 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
