diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index c5f90ef4cb368..5ac63dddd4d4e 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 0632882b29614..574723b33866a 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 8a4b36207c473..63712b848018b 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -171,34 +171,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { .ModulesPruneNonAffectingModuleMaps) return std::nullopt; - SmallVector ModulesToProcess{RootModule}; - const HeaderSearch &HS = PP.getHeaderSearchInfo(); - - SmallVector 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->isModuleHeader && !HFI->isCompilingModuleHeader)) - 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 ModuleMaps; auto CollectIncludingModuleMaps = [&](FileID FID, FileEntryRef F) { @@ -233,12 +208,48 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { } }; - for (const Module *CurrentModule : ModulesToProcess) { + // Handle all the affecting modules referenced from the root module. + + std::queue 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 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) + 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()) + CollectIncludingMapsFromAncestors(M); } return ModuleMaps; @@ -2053,14 +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. const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File); - if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) - 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(); 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 0000000000000..fce325d4774c2 --- /dev/null +++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c @@ -0,0 +1,46 @@ +// 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 + +//--- 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 { header "A.h" export * } +//--- A.h +#include "X.h" + +// 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/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: %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