[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:
> 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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866