[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/lib/Serialization/ModuleManager.cpp:183
       // Get a buffer of the file and close the file descriptor when done.
-      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+      Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
     }
----------------
rsmith wrote:
> dexonsmith wrote:
> > vsapsai wrote:
> > > Made this change because if we don't have a valid module but opened a corresponding .pcm file earlier, there is a high chance that .pcm file was rebuilt.
> > Please add a comment in the code explaining that.
> This change is proving really bad for us. This prevents using `mmap` for loading module files, and instead forces the entire file to be loaded every time. Please revert.
Can we limit the revert to explicit vs. implicit module builds?  The scenario Volodymyr was defending against is implicit-only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72860