[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader creates extra HeaderFileInfo, breaks PCM reuse (PR #89005)

[Richard Smith via cfe-commits]

================
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef HeaderSearch::LookupSubframeworkHeader(
 // File Info Management.
 //===----------------------------------------------------------------------===//
 
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+                                  ModuleMap::ModuleHeaderRole Role) {
+  return (HFI->isModuleHeader == ModuleMap::isModular(Role)) &&
+         (HFI->isTextualModuleHeader ==
+          ((Role & ModuleMap::TextualHeader) != 0));
----------------
zygoloid wrote:

Should this be:
```suggestion
  return (HFI->isModuleHeader == ModuleMap::isModular(Role)) &&
         (HFI->isModuleHeader ||
          HFI->isTextualModuleHeader ==
              ((Role & ModuleMap::TextualHeader) != 0));
```
It looks like you're considering "modular header" and "textual header" to be mutually exclusive in `HeaderFileInfo` when merging, so presumably we should do the same here. I don't think this would make a difference for our use case, though.

Incidentally, this choice of meaning for `isTextualModuleHeader` seems a little confusing to me from a modeling perspective, given that a header can absolutely be modular in one module and textual in another, but I think it's fine from a correctness standpoint. I think it'd be clearer to either use a three-value enum here, or to independently track whether the header is textual or modular and change the check below to `isTextualModuleHeader && !isModuleHeader`. But that's not relevant for this PR.

https://github.com/llvm/llvm-project/pull/89005