diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -648,7 +648,8 @@ /// \param File The header that we wish to map to a module. /// \param AllowTextual Whether we want to find textual headers too. ModuleMap::KnownHeader findModuleForHeader(const FileEntry *File, - bool AllowTextual = false) const; + bool AllowTextual = false, + bool AllowExcluded = false) const; /// Retrieve all the modules corresponding to the given file. /// diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -136,9 +136,11 @@ /// should be textually included. TextualHeader = 0x2, + /// This header is explicitly excluded from the module. + ExcludedHeader = 0x4, + // Caution: Adding an enumerator needs other changes. // Adjust the number of bits for KnownHeader::Storage. - // Adjust the bitfield HeaderFileInfo::HeaderRole size. // Adjust the HeaderFileInfoTrait::ReadData streaming. // Adjust the HeaderFileInfoTrait::EmitData streaming. // Adjust ModuleMap::addHeader. @@ -153,7 +155,7 @@ /// A header that is known to reside within a given module, /// whether it was included or excluded. class KnownHeader { - llvm::PointerIntPair Storage; + llvm::PointerIntPair Storage; public: KnownHeader() : Storage(nullptr, NormalHeader) {} @@ -434,7 +436,8 @@ /// given header file. The KnownHeader is default constructed to indicate /// that no module owns this header file. KnownHeader findModuleForHeader(const FileEntry *File, - bool AllowTextual = false); + bool AllowTextual = false, + bool AllowExcluded = false); /// Retrieve all the modules that contain the given header file. Note that /// this does not implicitly load module maps, except for builtin headers, diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -1522,14 +1522,14 @@ } ModuleMap::KnownHeader -HeaderSearch::findModuleForHeader(const FileEntry *File, - bool AllowTextual) const { +HeaderSearch::findModuleForHeader(const FileEntry *File, bool AllowTextual, + bool AllowExcluded) const { if (ExternalSource) { // Make sure the external source has handled header info about this file, // which includes whether the file is part of a module. (void)getExistingFileInfo(File); } - return ModMap.findModuleForHeader(File, AllowTextual); + return ModMap.findModuleForHeader(File, AllowTextual, AllowExcluded); } ArrayRef diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -567,9 +567,11 @@ } ModuleMap::KnownHeader ModuleMap::findModuleForHeader(const FileEntry *File, - bool AllowTextual) { + bool AllowTextual, + bool AllowExcluded) { auto MakeResult = [&](ModuleMap::KnownHeader R) -> ModuleMap::KnownHeader { - if (!AllowTextual && R.getRole() & ModuleMap::TextualHeader) + if ((!AllowTextual && R.getRole() & ModuleMap::TextualHeader) || + (!AllowExcluded && R.getRole() & ModuleMap::ExcludedHeader)) return {}; return R; }; @@ -700,6 +702,9 @@ E = Known->second.end(); I != E; ++I) { + if (I->getRole() == ModuleMap::ExcludedHeader) + continue; + if (I->isAvailable() && (!RequestingModule || I->getModule()->isSubModuleOf(RequestingModule))) { @@ -1261,11 +1266,13 @@ } void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) { + KnownHeader KH(Mod, ModuleHeaderRole::ExcludedHeader); + // Add this as a known header so we won't implicitly add it to any // umbrella directory module. // FIXME: Should we only exclude it from umbrella modules within the // specified module? - (void) Headers[Header.Entry]; + Headers[Header.Entry].push_back(KH); Mod->Headers[Module::HK_Excluded].push_back(std::move(Header)); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -1901,8 +1901,8 @@ "Wrong data length in HeaderFileInfo deserialization"); while (d != End) { uint32_t LocalSMID = endian::readNext(d); - auto HeaderRole = static_cast(LocalSMID & 3); - LocalSMID >>= 2; + auto HeaderRole = static_cast(LocalSMID & 7); + LocalSMID >>= 3; // This header is part of a module. Associate it with the module to enable // implicit module import. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1820,8 +1820,8 @@ auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) { if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) { - uint32_t Value = (ModID << 2) | (unsigned)Role; - assert((Value >> 2) == ModID && "overflow in header module info"); + uint32_t Value = (ModID << 3) | (unsigned)Role; + assert((Value >> 3) == ModID && "overflow in header module info"); LE.write(Value); } }; diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -397,52 +397,34 @@ MDC.addFileDep(MD, IF.getFile()->getName()); }); - // We usually don't need to list the module map files of our dependencies when - // building a module explicitly: their semantics will be deserialized from PCM - // files. - // - // However, some module maps loaded implicitly during the dependency scan can - // describe anti-dependencies. That happens when this module, let's call it - // M1, is marked as '[no_undeclared_includes]' and tries to access a header - // "M2/M2.h" from another module, M2, but doesn't have a 'use M2;' - // declaration. The explicit build needs the module map for M2 so that it - // knows that textually including "M2/M2.h" is not allowed. - // E.g., '__has_include("M2/M2.h")' should return false, but without M2's - // module map the explicit build would return true. - // - // An alternative approach would be to tell the explicit build what its - // textual dependencies are, instead of having it re-discover its - // anti-dependencies. For example, we could create and use an `-ivfs-overlay` - // with `fall-through: false` that explicitly listed the dependencies. - // However, that's more complicated to implement and harder to reason about. - if (M->NoUndeclaredIncludes) { - // We don't have a good way to determine which module map described the - // anti-dependency (let alone what's the corresponding top-level module - // map). We simply specify all the module maps in the order they were loaded - // during the implicit build during scan. - // TODO: Resolve this by serializing and only using Module::UndeclaredUses. - MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps( - *MF, [&](const FileEntry *FE) { - if (FE->getName().endswith("__inferred_module.map")) - return; - // The top-level modulemap of this module will be the input file. We - // don't need to specify it as a module map. - if (FE == ModuleMap) - return; - MD.ModuleMapFileDeps.push_back(FE->getName().str()); - }); - } + llvm::DenseSet SeenDeps; + addAllSubmodulePrebuiltDeps(M, MD, SeenDeps); + addAllSubmoduleDeps(M, MD, SeenDeps); + addAllAffectingModules(M, MD, SeenDeps); + + llvm::DenseSet SeenModuleMaps; + for (const Module *SM : SeenDeps) + if (const FileEntry *SMM = MDC.ScanInstance.getPreprocessor() + .getHeaderSearchInfo() + .getModuleMap() + .getModuleMapFileForUniquing(SM)) + SeenModuleMaps.insert(SMM); - // Add direct prebuilt module dependencies now, so that we can use them when - // creating a CompilerInvocation and computing context hash for this - // ModuleDeps instance. - // TODO: Squash these. - llvm::DenseSet SeenModules; - addAllSubmodulePrebuiltDeps(M, MD, SeenModules); - llvm::DenseSet AddedModules; - addAllSubmoduleDeps(M, MD, AddedModules); - llvm::DenseSet ProcessedModules; - addAllAffectingModules(M, MD, ProcessedModules); + MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps( + *MF, [&](const FileEntry *FE) { + if (FE->getName().endswith("__inferred_module.map")) + return; + // The top-level modulemap of this module will be the input file. We + // don't need to specify it via "-fmodule-map-file=". + if (FE == ModuleMap) + return; + // The top-level modulemap of dependencies will be serialized in the PCM + // file (eager loading mode) or passed on the command-line through a + // different mechanism (lazy loading mode). + if (SeenModuleMaps.contains(FE)) + return; + MD.ModuleMapFileDeps.emplace_back(FE->getName()); + }); CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs( MD, [&](CompilerInvocation &BuildInvocation) { diff --git a/clang/test/ClangScanDeps/modules-excluded-header.m b/clang/test/ClangScanDeps/modules-excluded-header.m new file mode 100644 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-excluded-header.m @@ -0,0 +1,54 @@ +// This test checks that we're reporting module maps affecting the compilation by describing excluded headers. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- frameworks/X.framework/Modules/module.modulemap +framework module X { + umbrella header "X.h" + exclude header "Excluded.h" +} +//--- frameworks/X.framework/Headers/X.h +//--- frameworks/X.framework/Headers/Excluded.h + +//--- mod/module.modulemap +module Mod { header "Mod.h" } +//--- mod/Mod.h +#include + +//--- tu.m +@import Mod; + +//--- cdb.json.template +[{ + "file": "DIR/tu.m", + "directory": "DIR", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR/mod -F DIR/frameworks -Werror=non-modular-include-in-module -c DIR/tu.m -o DIR/tu.m" +}] + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t + +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/mod/module.modulemap", +// CHECK-NEXT: "command-line": [ +// CHECK: "-fmodule-map-file=[[PREFIX]]/frameworks/X.framework/Modules/module.modulemap" +// CHECK-NOT: "-fmodule-file={{.*}}" +// CHECK: ], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK: ], +// CHECK-NEXT: "name": "Mod" +// CHECK-NEXT: } +// CHECK-NEXT: ] +// CHECK: } + +// RUN: %deps-to-rsp %t/result.json --module-name=Mod > %t/Mod.cc1.rsp +// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp + +// RUN: %clang @%t/Mod.cc1.rsp +// RUN: %clang @%t/tu.rsp