[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 24 09:05:09 PDT 2024


https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/89441

>From 9165d6086e2570198fba1c333d0c9cb9c09037c7 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 19 Apr 2024 12:13:06 -0700
Subject: [PATCH 1/8] [clang][modules] Allow module map files with textual
 header be non-affecting

---
 clang/lib/Serialization/ASTWriter.cpp         | 10 ++++---
 ...e-non-affecting-module-map-files-textual.c | 26 +++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)
 create mode 100644 clang/test/Modules/prune-non-affecting-module-map-files-textual.c

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 8a4b36207c4734..4825c245a4b846 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,7 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
       continue;
 
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+    if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+                 !HFI->isCompilingModuleHeader))
       continue;
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -2055,11 +2056,12 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
 
     // Get the file info. Skip emitting this file if we have no information on
     // it as a header file (in which case HFI will be null) or if it hasn't
-    // changed since it was loaded. Also skip it if it's for a modular header
-    // from a different module; in that case, we rely on the module(s)
+    // changed since it was loaded. Also skip it if it's for a non-excluded
+    // header from a different module; in that case, we rely on the module(s)
     // containing the header to provide this information.
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+    if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+                 !HFI->isCompilingModuleHeader))
       continue;
 
     // Massage the file path into an appropriate form.
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
new file mode 100644
index 00000000000000..8f8f00560b1834
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -0,0 +1,26 @@
+// This test checks that a module map with a textual header can be marked as
+// non-affecting.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- A.modulemap
+module A { textual header "A.h" }
+//--- B.modulemap
+module B { header "B.h" export * }
+//--- A.h
+typedef int A_int;
+//--- B.h
+#include "A.h"
+typedef A_int B_int;
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A.pcm \
+// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
+// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
+// RUN:                                    -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: diff %t/B0.pcm %t/B1.pcm

>From 17b1860b25de99f8b2b2e047ef1590b8ede6648b Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 22 Apr 2024 09:28:26 -0700
Subject: [PATCH 2/8] [clang][modules] Mark 'use'd modules as affecting

---
 clang/lib/Serialization/ASTWriter.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4825c245a4b846..8c303fa67e3023 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -238,6 +238,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
     CollectIncludingMapsFromAncestors(CurrentModule);
     for (const Module *ImportedModule : CurrentModule->Imports)
       CollectIncludingMapsFromAncestors(ImportedModule);
+    for (const Module *UsedModule : CurrentModule->DirectUses)
+      CollectIncludingMapsFromAncestors(UsedModule);
     for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
       CollectIncludingMapsFromAncestors(UndeclaredModule);
   }

>From 9eeea3f468432ec810a9e09696cf6a527c7db233 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 07:51:17 -0700
Subject: [PATCH 3/8] [clang][modules] Do consider module maps of included
 textual headers affecting

---
 clang/lib/Serialization/ASTWriter.cpp            | 16 +++++++++-------
 ...rune-non-affecting-module-map-files-textual.c | 14 ++++----------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 8c303fa67e3023..24f5c3ebbc1457 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,8 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
       continue;
 
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
-                 !HFI->isCompilingModuleHeader))
+    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
+        (HFI->isTextualModuleHeader && !PP.alreadyIncluded(*File)))
       continue;
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -2058,12 +2058,14 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
 
     // Get the file info. Skip emitting this file if we have no information on
     // it as a header file (in which case HFI will be null) or if it hasn't
-    // changed since it was loaded. Also skip it if it's for a non-excluded
-    // header from a different module; in that case, we rely on the module(s)
-    // containing the header to provide this information.
+    // changed since it was loaded. Also skip it if it's for a modular header
+    // from a different module; in that case, we rely on the module(s)
+    // containing the header to provide this information. Also skip it if it's
+    // for a textual header from a different module that has not been included;
+    // in that case, we don't need the information at all.
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
-                 !HFI->isCompilingModuleHeader))
+    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
+        (HFI->isTextualModuleHeader && !PP->alreadyIncluded(*File)))
       continue;
 
     // Massage the file path into an appropriate form.
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
index 8f8f00560b1834..485e360d72bec1 100644
--- a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -1,5 +1,5 @@
 // This test checks that a module map with a textual header can be marked as
-// non-affecting.
+// non-affecting if its header did not get included.
 
 // RUN: rm -rf %t && mkdir %t
 // RUN: split-file %s %t
@@ -7,20 +7,14 @@
 //--- A.modulemap
 module A { textual header "A.h" }
 //--- B.modulemap
-module B { header "B.h" export * }
+module B { header "B.h" }
 //--- A.h
-typedef int A_int;
 //--- B.h
-#include "A.h"
-typedef A_int B_int;
-
-// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A.pcm \
-// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
 
 // RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
-// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
 
 // RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
-// RUN:                                    -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+// RUN:                                    -fmodule-map-file=%t/B.modulemap
 
 // RUN: diff %t/B0.pcm %t/B1.pcm

>From ddc6bf9af7f84123a93ed9bb13138013a73456f9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 12:47:22 -0700
Subject: [PATCH 4/8] Reject all non-included headers from that are
 !isCompilingModuleHeader, not just textual module headers. Also remove
 DirectUses.

---
 clang/lib/Serialization/ASTWriter.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 24f5c3ebbc1457..d10be3fca3194e 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,8 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
       continue;
 
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
-        (HFI->isTextualModuleHeader && !PP.alreadyIncluded(*File)))
+    if (!HFI || (!HFI->isCompilingModuleHeader &&
+                 (HFI->isModuleHeader || !PP.alreadyIncluded(*File))))
       continue;
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -238,8 +238,6 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
     CollectIncludingMapsFromAncestors(CurrentModule);
     for (const Module *ImportedModule : CurrentModule->Imports)
       CollectIncludingMapsFromAncestors(ImportedModule);
-    for (const Module *UsedModule : CurrentModule->DirectUses)
-      CollectIncludingMapsFromAncestors(UsedModule);
     for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
       CollectIncludingMapsFromAncestors(UndeclaredModule);
   }
@@ -2061,11 +2059,11 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
     // changed since it was loaded. Also skip it if it's for a modular header
     // from a different module; in that case, we rely on the module(s)
     // containing the header to provide this information. Also skip it if it's
-    // for a textual header from a different module that has not been included;
-    // in that case, we don't need the information at all.
+    // for any header not from this module that has not been included; in that
+    // case, we don't need the information at all.
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
-        (HFI->isTextualModuleHeader && !PP->alreadyIncluded(*File)))
+    if (!HFI || (!HFI->isCompilingModuleHeader &&
+                 (HFI->isModuleHeader || !PP->alreadyIncluded(*File))))
       continue;
 
     // Massage the file path into an appropriate form.

>From 2993a63f83b6d74eef4b5815dda08d6b4fe8f441 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 12:42:40 -0700
Subject: [PATCH 5/8] Don't add modules for textual headers to
 `ModulesToProcess`

---
 clang/lib/Serialization/ASTWriter.cpp | 63 +++++++++++++++------------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d10be3fca3194e..4265ab42eda8e0 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -171,35 +171,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
            .ModulesPruneNonAffectingModuleMaps)
     return std::nullopt;
 
-  SmallVector<const Module *> ModulesToProcess{RootModule};
-
   const HeaderSearch &HS = PP.getHeaderSearchInfo();
-
-  SmallVector<OptionalFileEntryRef, 16> FilesByUID;
-  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
-
-  if (FilesByUID.size() > HS.header_file_size())
-    FilesByUID.resize(HS.header_file_size());
-
-  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
-    OptionalFileEntryRef File = FilesByUID[UID];
-    if (!File)
-      continue;
-
-    const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (!HFI->isCompilingModuleHeader &&
-                 (HFI->isModuleHeader || !PP.alreadyIncluded(*File))))
-      continue;
-
-    for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
-      if (!KH.getModule())
-        continue;
-      ModulesToProcess.push_back(KH.getModule());
-    }
-  }
-
   const ModuleMap &MM = HS.getModuleMap();
-  SourceManager &SourceMgr = PP.getSourceManager();
+  const SourceManager &SourceMgr = PP.getSourceManager();
 
   std::set<const FileEntry *> ModuleMaps;
   auto CollectIncludingModuleMaps = [&](FileID FID, FileEntryRef F) {
@@ -234,12 +208,45 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
     }
   };
 
-  for (const Module *CurrentModule : ModulesToProcess) {
+  // Handle all the affecting modules referenced from the root module.
+
+  std::queue<const Module *> Q;
+  Q.push(RootModule);
+  while (!Q.empty()) {
+    const Module *CurrentModule = Q.front();
+    Q.pop();
+
     CollectIncludingMapsFromAncestors(CurrentModule);
     for (const Module *ImportedModule : CurrentModule->Imports)
       CollectIncludingMapsFromAncestors(ImportedModule);
     for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
       CollectIncludingMapsFromAncestors(UndeclaredModule);
+
+    for (auto *M : CurrentModule->submodules())
+      Q.push(M);
+  }
+
+  // Handle textually-included headers that belong to other modules.
+
+  SmallVector<OptionalFileEntryRef, 16> FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+    FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+    OptionalFileEntryRef File = FilesByUID[UID];
+    if (!File)
+      continue;
+
+    const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
+    if (!HFI || (!HFI->isCompilingModuleHeader &&
+                 (HFI->isModuleHeader || !PP.alreadyIncluded(*File))))
+      continue;
+
+    for (const auto &KH : HS.findResolvedModulesForHeader(*File))
+      if (const Module *M = KH.getModule())
+        CollectIncludingMapsFromAncestors(M);
   }
 
   return ModuleMaps;

>From b03c34f99bd90398f0599b5703cbf82d8b881981 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 13:57:19 -0700
Subject: [PATCH 6/8] **Locally** included

---
 clang/include/clang/Lex/HeaderSearch.h        | 14 ++++--
 clang/lib/Lex/HeaderSearch.cpp                |  1 +
 clang/lib/Serialization/ASTWriter.cpp         |  2 +-
 ...e-non-affecting-module-map-files-textual.c | 46 +++++++++++++++----
 4 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index c5f90ef4cb3682..5ac63dddd4d4ea 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -56,6 +56,12 @@ class TargetInfo;
 /// The preprocessor keeps track of this information for each
 /// file that is \#included.
 struct HeaderFileInfo {
+  // TODO: Whether the file was included is not a property of the file itself.
+  // It's a preprocessor state, move it there.
+  /// True if this file has been included (or imported) **locally**.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsLocallyIncluded : 1;
+
   // TODO: Whether the file was imported is not a property of the file itself.
   // It's a preprocessor state, move it there.
   /// True if this is a \#import'd file.
@@ -135,10 +141,10 @@ struct HeaderFileInfo {
   StringRef Framework;
 
   HeaderFileInfo()
-      : isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
-        External(false), isModuleHeader(false), isTextualModuleHeader(false),
-        isCompilingModuleHeader(false), Resolved(false),
-        IndexHeaderMapHeader(false), IsValid(false) {}
+      : IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
+        DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
+        isTextualModuleHeader(false), isCompilingModuleHeader(false),
+        Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
 
   /// Retrieve the controlling macro for this header file, if
   /// any.
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..574723b33866af 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1574,6 +1574,7 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
     }
   }
 
+  FileInfo.IsLocallyIncluded = true;
   IsFirstIncludeOfFile = PP.markIncluded(File);
   return true;
 }
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4265ab42eda8e0..50413fc44bff24 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -241,7 +241,7 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
 
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
     if (!HFI || (!HFI->isCompilingModuleHeader &&
-                 (HFI->isModuleHeader || !PP.alreadyIncluded(*File))))
+                 (HFI->isModuleHeader || !HFI->IsLocallyIncluded)))
       continue;
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File))
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
index 485e360d72bec1..ae3f3fe85e6517 100644
--- a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -1,20 +1,46 @@
 // This test checks that a module map with a textual header can be marked as
-// non-affecting if its header did not get included.
+// non-affecting.
 
 // RUN: rm -rf %t && mkdir %t
 // RUN: split-file %s %t
 
+//--- X.modulemap
+module X { textual header "X.h" }
+//--- X.h
+typedef int X_int;
+
+//--- Y.modulemap
+module Y { textual header "Y.h" }
+//--- Y.h
+typedef int Y_int;
+
 //--- A.modulemap
-module A { textual header "A.h" }
-//--- B.modulemap
-module B { header "B.h" }
+module A { header "A.h" export * }
 //--- A.h
-//--- B.h
+#include "X.h"
 
-// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
-// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A0.pcm \
+// RUN:   -fmodule-map-file=%t/X.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A0 --implicit-check-not=Y.modulemap
+// A0: Input file: {{.*}}/X.modulemap
 
-// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
-// RUN:                                    -fmodule-map-file=%t/B.modulemap
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A1.pcm \
+// RUN:   -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A1 \
+// RUN:   --implicit-check-not=Y.modulemap
+// A1: Input file: {{.*}}/X.modulemap
+
+// RUN: diff %t/A0.pcm %t/A1.pcm
+
+//--- B.modulemap
+module B { header "B.h" export * }
+//--- B.h
+#include "A.h"
+typedef X_int B_int;
 
-// RUN: diff %t/B0.pcm %t/B1.pcm
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B.pcm \
+// RUN:   -fmodule-file=A=%t/A0.pcm \
+// RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/B.pcm | FileCheck %s --check-prefix=B \
+// RUN:   --implicit-check-not=X.modulemap --implicit-check-not=Y.modulemap
+// B: Input file: {{.*}}/B.modulemap

>From 1228f5c1ce904deec649f92c3d858bd3983c37b9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 24 Apr 2024 08:31:58 -0700
Subject: [PATCH 7/8] Fix test on Windows

---
 .../Modules/prune-non-affecting-module-map-files-textual.c  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
index ae3f3fe85e6517..fce325d4774c27 100644
--- a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -22,13 +22,13 @@ module A { header "A.h" export * }
 // RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A0.pcm \
 // RUN:   -fmodule-map-file=%t/X.modulemap
 // RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A0 --implicit-check-not=Y.modulemap
-// A0: Input file: {{.*}}/X.modulemap
+// A0: Input file: {{.*}}X.modulemap
 
 // RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A1.pcm \
 // RUN:   -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
 // RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A1 \
 // RUN:   --implicit-check-not=Y.modulemap
-// A1: Input file: {{.*}}/X.modulemap
+// A1: Input file: {{.*}}X.modulemap
 
 // RUN: diff %t/A0.pcm %t/A1.pcm
 
@@ -43,4 +43,4 @@ typedef X_int B_int;
 // RUN:   -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
 // RUN: %clang_cc1 -fsyntax-only -module-file-info %t/B.pcm | FileCheck %s --check-prefix=B \
 // RUN:   --implicit-check-not=X.modulemap --implicit-check-not=Y.modulemap
-// B: Input file: {{.*}}/B.modulemap
+// B: Input file: {{.*}}B.modulemap

>From 14ac904a7c9e99dd8d8cf4c48816d2f461025f1d Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 24 Apr 2024 09:04:49 -0700
Subject: [PATCH 8/8] Split out HFI conditions

---
 clang/lib/Serialization/ASTWriter.cpp | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 50413fc44bff24..63712b848018b3 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -240,9 +240,12 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
       continue;
 
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (!HFI->isCompilingModuleHeader &&
-                 (HFI->isModuleHeader || !HFI->IsLocallyIncluded)))
-      continue;
+    if (!HFI)
+      continue; // We have no information on this being a header file.
+    if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
+      continue; // Modular header, handled in the above module-based loop.
+    if (!HFI->isCompilingModuleHeader && !HFI->IsLocallyIncluded)
+      continue; // Non-modular header not included locally is not affecting.
 
     for (const auto &KH : HS.findResolvedModulesForHeader(*File))
       if (const Module *M = KH.getModule())
@@ -2061,17 +2064,13 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
     if (!File)
       continue;
 
-    // Get the file info. Skip emitting this file if we have no information on
-    // it as a header file (in which case HFI will be null) or if it hasn't
-    // changed since it was loaded. Also skip it if it's for a modular header
-    // from a different module; in that case, we rely on the module(s)
-    // containing the header to provide this information. Also skip it if it's
-    // for any header not from this module that has not been included; in that
-    // case, we don't need the information at all.
     const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
-    if (!HFI || (!HFI->isCompilingModuleHeader &&
-                 (HFI->isModuleHeader || !PP->alreadyIncluded(*File))))
-      continue;
+    if (!HFI)
+      continue; // We have no information on this being a header file.
+    if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
+      continue; // Header file info is tracked by the owning module file.
+    if (!HFI->isCompilingModuleHeader && !PP->alreadyIncluded(*File))
+      continue; // Non-modular header not included is not needed.
 
     // Massage the file path into an appropriate form.
     StringRef Filename = File->getName();



More information about the cfe-commits mailing list