[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang/lib/Tooling/Inclusions/Header.cpp:64
+  // particular preprocessor state, usually set up by another header.
+  return !isDontIncludeMeHeader(SM.getBufferData(SM.translateFile(FE)));
+}
----------------
sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > kadircet wrote:
> > > > `translateFile` actually does a linear scan over all the slocentries, so i think it's better for this API to be based on FileID. (later on we can easily get fileentry from fileid in constant time)
> > > You can get content through the fileentry directly, no?
> > oh thanks for reminding that i forgot to mention it here, yes we can get that, but that would misbehave in the case of virtual/remapped files. so can you actually add a comment here about why we should get the contents from source manager?
> But even when that remapping happens at the sourcemanager level, it happens on fileentries, not FileIDs (how would the latter even work?)
> 
> So SM.getMemoryBufferForFileOrNone(FileEntry)?
oh i didn't know about `getMemoryBufferForFileOrNone`, looks like it should do.

> But even when that remapping happens at the sourcemanager level, it happens on fileentries, not FileIDs (how would the latter even work?)

i was just looking at contentcache, which is a 1:1 mapping for each FileID and has fileentries, one for the original file and the other for providing the contents (or a virtual buffer). i didn't notice the overrideinfo map was also exposed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137697