diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index a52d59c61c4ce..f8158a654c45a 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -184,14 +184,30 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { const SourceManager &SM = PP.getSourceManager(); const ModuleMap &MM = HS.getModuleMap(); - llvm::DenseSet ModuleMaps; - - llvm::DenseSet ProcessedModules; - auto CollectModuleMapsForHierarchy = [&](const Module *M) { + // Module maps used only by textual headers are special. Their FileID is + // non-affecting, but their FileEntry is (i.e. must be written as InputFile). + enum AffectedReason : bool { + AR_TextualHeader = 0, + AR_ImportOrTextualHeader = 1, + }; + auto AssignMostImportant = [](AffectedReason &LHS, AffectedReason RHS) { + LHS = std::max(LHS, RHS); + }; + llvm::DenseMap ModuleMaps; + llvm::DenseMap ProcessedModules; + auto CollectModuleMapsForHierarchy = [&](const Module *M, + AffectedReason Reason) { M = M->getTopLevelModule(); - if (!ProcessedModules.insert(M).second) + // We need to process the header either when it was not present or when we + // previously flagged module map as textual headers and now we found a + // proper import. + if (auto [It, Inserted] = ProcessedModules.insert({M, Reason}); + !Inserted && Reason <= It->second) { return; + } else { + It->second = Reason; + } std::queue Q; Q.push(M); @@ -202,12 +218,12 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { // The containing module map is affecting, because it's being pointed // into by Module::DefinitionLoc. if (auto F = MM.getContainingModuleMapFileID(Mod); F.isValid()) - ModuleMaps.insert(F); + AssignMostImportant(ModuleMaps[F], Reason); // For inferred modules, the module map that allowed inferring is not // related to the virtual containing module map file. It did affect the // compilation, though. if (auto UniqF = MM.getModuleMapFileIDForUniquing(Mod); UniqF.isValid()) - ModuleMaps.insert(UniqF); + AssignMostImportant(ModuleMaps[UniqF], Reason); for (auto *SubM : Mod->submodules()) Q.push(SubM); @@ -216,7 +232,7 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { // Handle all the affecting modules referenced from the root module. - CollectModuleMapsForHierarchy(RootModule); + CollectModuleMapsForHierarchy(RootModule, AR_ImportOrTextualHeader); std::queue Q; Q.push(RootModule); @@ -225,9 +241,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { Q.pop(); for (const Module *ImportedModule : CurrentModule->Imports) - CollectModuleMapsForHierarchy(ImportedModule); + CollectModuleMapsForHierarchy(ImportedModule, AR_ImportOrTextualHeader); for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses) - CollectModuleMapsForHierarchy(UndeclaredModule); + CollectModuleMapsForHierarchy(UndeclaredModule, AR_ImportOrTextualHeader); for (auto *M : CurrentModule->submodules()) Q.push(M); @@ -256,7 +272,7 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { for (const auto &KH : HS.findResolvedModulesForHeader(*File)) if (const Module *M = KH.getModule()) - CollectModuleMapsForHierarchy(M); + CollectModuleMapsForHierarchy(M, AR_TextualHeader); } // FIXME: This algorithm is not correct for module map hierarchies where @@ -278,13 +294,16 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) { // includes a module map defining a module that's not a submodule of X. llvm::DenseSet ModuleFileEntries; - for (FileID MM : ModuleMaps) { - if (auto *FE = SM.getFileEntryForID(MM)) + llvm::DenseSet ModuleFileIDs; + for (auto [FID, Reason] : ModuleMaps) { + if (Reason == AR_ImportOrTextualHeader) + ModuleFileIDs.insert(FID); + if (auto *FE = SM.getFileEntryForID(FID)) ModuleFileEntries.insert(FE); } AffectingModuleMaps R; - R.DefinitionFileIDs = std::move(ModuleMaps); + R.DefinitionFileIDs = std::move(ModuleFileIDs); R.DefinitionFiles = std::move(ModuleFileEntries); return std::move(R); } diff --git a/clang/test/Modules/prune-non-affecting-module-map-repeated-textual.cpp b/clang/test/Modules/prune-non-affecting-module-map-repeated-textual.cpp new file mode 100644 index 0000000000000..30914056c55fa --- /dev/null +++ b/clang/test/Modules/prune-non-affecting-module-map-repeated-textual.cpp @@ -0,0 +1,104 @@ +// Same as prune-non-affecting-module-map-repeated.cpp, but check that textual-only +// inclusions do not cause duplication of the module map files they are defined in. + +// RUN: rm -rf %t && mkdir %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \ +// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mixed.map\ +// RUN: -fmodule-map-file=%t/mod1.map \ +// RUN: -fmodule-name=mod1 -emit-module %t/mod1.map -o %t/mod1.pcm +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \ +// RUN: -fmodule-map-file=%t/mixed.map\ +// RUN: -fmodule-name=mixed -emit-module %t/mixed.map -o %t/mixed.pcm +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \ +// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod1.map -fmodule-map-file=%t/mod2.map \ +// RUN: -fmodule-file=%t/mod1.pcm -fmodule-file=%t/mixed.pcm \ +// RUN: -fmodule-name=mod2 -emit-module %t/mod2.map -o %t/mod2.pcm +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \ +// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod2.map -fmodule-map-file=%t/mod3.map \ +// RUN: -fmodule-file=%t/mod2.pcm -fmodule-file=%t/mixed.pcm \ +// RUN: -fmodule-name=mod3 -emit-module %t/mod3.map -o %t/mod3.pcm +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \ +// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod3.map -fmodule-map-file=%t/mod4.map \ +// RUN: -fmodule-file=%t/mod3.pcm \ +// RUN: -fmodule-name=mod4 -emit-module %t/mod4.map -o %t/mod4.pcm +// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod4.map -fmodule-file=%t/mod4.pcm -fsyntax-only -verify %t/check_slocs.cc + +//--- base.map +module base { textual header "vector.h" } +//--- mixed.map +module mixed { textual header "mixed_text.h" header "mixed_mod.h"} +//--- mod1.map +module mod1 { header "mod1.h" } +//--- mod2.map +module mod2 { header "mod2.h" } +//--- mod3.map +module mod3 { header "mod3.h" } +//--- mod4.map +module mod4 { header "mod4.h" } +//--- check_slocs.cc +#include "mod4.h" +#include "vector.h" +#pragma clang __debug sloc_usage // expected-remark {{source manager location address space usage}} +// expected-note@* {{% of available space}} + +// base.map must only be present once, despite being used in each module. +// Because its location in every module compile should be non-affecting. + +// expected-note@base.map:1 {{file entered 1 time}} + +// different modules use either only textual header from mixed.map or both textual and modular +// headers. Either combination must lead to only 1 use at the end, because the module is ultimately +// in the import chain and any textual uses should not change that. + +// expected-note@mixed.map:1 {{file entered 1 time}} + +// expected-note@* + {{file entered}} + + +//--- vector.h +#ifndef VECTOR_H +#define VECTOR_H +#endif + +//--- mixed_text.h +#ifndef MIXED_TEXT_H +#define MIXED_TEXT_H +#endif +//--- mixed_mod.h +#ifndef MIXED_MOD_H +#define MIXED_MOD_H +#endif + +//--- mod1.h +#ifndef MOD1 +#define MOD1 +#include "vector.h" +#include "mixed_text.h" +int mod1(); +#endif +//--- mod2.h +#ifndef MOD2 +#define MOD2 +#include "vector.h" +#include "mod1.h" +#include "mixed_mod.h" +int mod2(); +#endif +//--- mod3.h +#ifndef MOD3 +#define MOD3 +#include "vector.h" +#include "mod2.h" +#include "mixed_text.h" +#include "mixed_mod.h" +int mod3(); +#endif +//--- mod4.h +#ifndef MOD4 +#define MOD4 +#include "vector.h" +#include "mod3.h" +int mod4(); +#endif