[clang] [clang][modules] HeaderSearch::MarkFileModuleHeader sets textual headers' HeaderFileInfo non-external when it shouldn't (PR #89005)

[Jan Svoboda via cfe-commits]

================
@@ -308,5 +308,73 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
   EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderFileInfoMerge) {
+  auto AddHeader = [&](std::string HeaderPath) -> FileEntryRef {
+    VFS->addFile(HeaderPath, 0,
+                 llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+                 /*User=*/std::nullopt, /*Group=*/std::nullopt,
+                 llvm::sys::fs::file_type::regular_file);
+    return *FileMgr.getOptionalFileRef(HeaderPath);
+  };
+
+  class MockExternalHeaderFileInfoSource : public ExternalHeaderFileInfoSource {
+    HeaderFileInfo GetHeaderFileInfo(FileEntryRef FE) {
+      HeaderFileInfo HFI;
+      auto FileName = FE.getName();
+      if (FileName == ModularPath)
+        HFI.mergeModuleMembership(ModuleMap::NormalHeader);
+      else if (FileName == TextualPath)
+        HFI.mergeModuleMembership(ModuleMap::TextualHeader);
+      HFI.External = true;
+      HFI.IsValid = true;
+      return HFI;
+    }
+
+  public:
+    std::string ModularPath = "/modular.h";
+    std::string TextualPath = "/textual.h";
+  };
+
+  auto ExternalSource = new MockExternalHeaderFileInfoSource();
+  Search.SetExternalSource(ExternalSource);
+
+  // Everything should start out external.
+  auto ModularFE = AddHeader(ExternalSource->ModularPath);
+  auto TextualFE = AddHeader(ExternalSource->TextualPath);
+  EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+  EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+  // Marking the same role should keep it external
+  Search.MarkFileModuleHeader(ModularFE, ModuleMap::NormalHeader,
+                              /*isCompilingModuleHeader=*/false);
+  Search.MarkFileModuleHeader(TextualFE, ModuleMap::TextualHeader,
+                              /*isCompilingModuleHeader=*/false);
+  EXPECT_TRUE(Search.getExistingFileInfo(ModularFE)->External);
+  EXPECT_TRUE(Search.getExistingFileInfo(TextualFE)->External);
+
+  // textual -> modular should update the HFI, but modular -> textual should be
+  // a no-op.
+  Search.MarkFileModuleHeader(ModularFE, ModuleMap::TextualHeader,
+                              /*isCompilingModuleHeader=*/false);
+  Search.MarkFileModuleHeader(TextualFE, ModuleMap::NormalHeader,
+                              /*isCompilingModuleHeader=*/false);
+  auto ModularFI = Search.getExistingFileInfo(ModularFE);
+  auto TextualFI = Search.getExistingFileInfo(TextualFE);
+  EXPECT_TRUE(ModularFI->External);
+  EXPECT_TRUE(ModularFI->isModuleHeader);
+  EXPECT_FALSE(ModularFI->isTextualModuleHeader);
+  EXPECT_FALSE(TextualFI->External);
+  EXPECT_TRUE(ModularFI->isModuleHeader);
+  EXPECT_FALSE(ModularFI->isTextualModuleHeader);
----------------
jansvoboda11 wrote:

I suspect this is supposed to check `TextualFI`.

```suggestion
  EXPECT_TRUE(TextualFI->isModuleHeader);
  EXPECT_FALSE(TextualFI->isTextualModuleHeader);
```

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