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

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 14 09:28:15 PDT 2024


================
@@ -1313,11 +1313,19 @@ OptionalFileEntryRef HeaderSearch::LookupSubframeworkHeader(
 // File Info Management.
 //===----------------------------------------------------------------------===//
 
+static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI,
+                                       ModuleMap::ModuleHeaderRole Role) {
+  if (ModuleMap::isModular(Role))
+    return !HFI->isModuleHeader || HFI->isTextualModuleHeader;
+  else if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader))
+    return !HFI->isTextualModuleHeader;
+  else
+    return false;
----------------
jansvoboda11 wrote:

I think we can remove the nesting.

```suggestion
  if (ModuleMap::isModular(Role))
    return !HFI->isModuleHeader || HFI->isTextualModuleHeader;
 if (!HFI->isModuleHeader && (Role & ModuleMap::TextualHeader))
    return !HFI->isTextualModuleHeader;
  return false;
```

I'm curious if something like this would have the same semantics? We wouldn't have to keep the functions in sync this way:

```c++
static bool moduleMembershipNeedsMerge(const HeaderFileInfo *HFI,
                                        ModuleMap::ModuleHeaderRole Role) {
  HeaderFileInfo MergedHFICopy = *HFI;
  mergeHeaderFileInfoModuleBits(MergedHFICopy, ModuleMap::isModular(Role), ModuleMap::isTextualModular(Role));
  return MergedHFICopy != *HFI;
}
```

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


More information about the cfe-commits mailing list