[PATCH] D137304: [clang] Store filename per include instead of mutating filename

[Ben Barham via Phabricator via cfe-commits]

bnbarham added inline comments.


================
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;
 
----------------
benlangmuir wrote:
> dexonsmith wrote:
> > 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)
> Yes, I think this split actually makes the behaviour clearer. The remaining fields are related to the contents, while the extracted fields are for the name.  You only need one SourceLineCache, etc. for one set of file contents.
Makes sense to me. Might be worth adding a comment for that when you're done + fixing up the comment on `ContentsEntry` since it still references `Entry` (which I assume is what `OrigEntry` used to be called?).


================
Comment at: clang/include/clang/Basic/SourceManager.h:314
   /// Return a FileInfo object.
-  static FileInfo get(SourceLocation IL, ContentCache &Con,
-                      CharacteristicKind FileCharacter, StringRef Filename) {
+  static FileInfo get(SourceLocation IL, NamedContentCache &Con,
+                      CharacteristicKind FileCharacter) {
----------------
I believe `NamedContentCache` can be `const` here now that we're not setting its filename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137304