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

[Ian Anderson 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));
----------------
ian-twilightcoder wrote:

Oh I see what you're saying. I must have a HFI that says the header is normal-modular, and a role that says it's textual-modular. When I merge in the role it's not going to change the HFI since `isModuleHeader` never gets downgraded, but my matching check will return false. Sneaky.

> It looks like you're considering "modular header" and "textual header" to be mutually exclusive

Yes, I even put an assert in the merging code. But I'm told asserts aren't usually (ever?) compiled in, so I guess that's probably why it's not getting hit. I did notice a test listing a header as normal in one module and textual in another. What does that actually mean though? The test uses it to exploit a bug in `[no_undeclared_includes]` where the compiler will use one module to resolve the include and a different module to check for uses and bypass what the used module says is allowed. Really what does it mean for a header to be in multiple modules at all? Doesn't that just cause non-deterministic behavior in header->module lookup?

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