[PATCH] D77942: [Preamble] Invalidate preamble when missing headers become present.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:101
+
+  // We always receive FileNotFound followed by InclusionDirective.
+  // We want the former, so we're completely sure the file was missing.
----------------
kadircet wrote:
> is there a common recovery logic implemented in clang that makes this logic worth it?
> because none of the existing clients seems to be implementing it.
> 
> I would rather check for `File == nullptr` in `InclusionDirective` and maybe put a fixme saying that this might still be set by a "recovering ppcallback".
> 
> Up to you though.
Done. I think a fixme is disingenuous though if we deliberately un-fixed it :-)
Left a non-committal comment.


================
Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:590
+  for (const auto &F : MissingFiles) {
+    // A missing file may be "provided" by an override buffer.
+    if (OverridenFileBuffers.count(F.getKey()))
----------------
kadircet wrote:
> i believe it can also be provided by an overridefile. maybe also check this above while going over `RemappedFiles`. i.e:
> 
> ```
> for(... &R : ..RemappedFiles) {
>    if(!stat(R.second)) return false;
>    // A missing file has been remapped into an existing file.
>    if(MissingFiles.count(R.first)) return false;
>  ....
> }
> ```
Decided to just add a new set containing abs paths for everything that got overridden.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942