[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

[Nikolai Kosjar via Phabricator via cfe-commits]

nik marked an inline comment as done.
nik added inline comments.


================
Comment at: lib/Basic/SourceManager.cpp:1594
         SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-        if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) {
+        if (MainFile && *SourceFileName == llvm::sys::path::filename(MainFile->getName())) {
           SourceFileUID = getActualFileUID(SourceFile);
----------------
ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > Can this actually be`null`? 
> > The comment for OrigEntry states that it might be null:
> > 
> >     /// Reference to the file entry representing this ContentCache.
> >     ///
> >     /// This reference does not own the FileEntry object.
> >     ///
> >     /// It is possible for this to be NULL if the ContentCache encapsulates
> >     /// an imaginary text buffer.
> >     const FileEntry *OrigEntry;
> > 
> > Note also that further down in this function, a null check for MainContentCache->OrigEntry is done, so I believe that this was just forgotten here (since there is also no assert) and we are the first one running into this with the introduced call to SourceMgr.translateFile(File).
> > 
> > I've tried to understand why we end up with a nullptr here, but this goes beyond me in the time I've taken for this. What I've observed is that module code path (LibclangReparseTest.ReparseWithModule vs LibclangReparseTest.Reparse) creates at least a MemoryBuffer more (in ModuleMap::parseModuleMapFile; possibly the one referred with "imaginary text buffer" from the comment) and I suspect this to be somehow related with the OrigEntry nullptr.
> Should we check for equality of `MainContentCache->ContentsEntry` in that case? I guess this is what should be set for "imaginary" files.
Does not help in this particular case as ContentsEntry is also a nullptr. All the members of the ContentsCache seem to be either nullptr or 0 as that particular point of execution.

How to continue?

a) Accept as is.
b) Ask someone knowledgeable for whom this could be a no-brainer that could respond hopefully soon. Any candidate?
c) Or should I dig deeper? This can take quite some time as this area is new for me.



Repository:
  rC Clang

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

https://reviews.llvm.org/D53866