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_svob...@apple.com> Date: Fri, 19 Apr 2024 12:13:06 -0700 Subject: [PATCH 1/4] [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_svob...@apple.com> Date: Mon, 22 Apr 2024 09:28:26 -0700 Subject: [PATCH 2/4] [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_svob...@apple.com> Date: Tue, 23 Apr 2024 07:51:17 -0700 Subject: [PATCH 3/4] [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_svob...@apple.com> Date: Tue, 23 Apr 2024 12:47:22 -0700 Subject: [PATCH 4/4] 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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits