[clang] f35230a - [clang][modules][deps] Report modulemaps describing excluded headers
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 22 12:36:15 PDT 2022
Author: Jan Svoboda
Date: 2022-09-22T12:36:05-07:00
New Revision: f35230ae0a69bbdd74a4faa3ae0b69a46796dc12
URL: https://github.com/llvm/llvm-project/commit/f35230ae0a69bbdd74a4faa3ae0b69a46796dc12
DIFF: https://github.com/llvm/llvm-project/commit/f35230ae0a69bbdd74a4faa3ae0b69a46796dc12.diff
LOG: [clang][modules][deps] Report modulemaps describing excluded headers
Module map files describing excluded headers do affect compilation. Track them in the compiler, serialize them into the PCM file and report them in the scanner.
Depends on D134222.
Reviewed By: Bigcheese
Differential Revision: https://reviews.llvm.org/D134224
Added:
clang/test/ClangScanDeps/modules-excluded-header.m
Modified:
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Lex/ModuleMap.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index e438385f25508..d4b2306096972 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -648,7 +648,8 @@ class HeaderSearch {
/// \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
index 20da9eb18a3e2..8fbe4a927c77b 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -136,9 +136,11 @@ class ModuleMap {
/// 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 @@ class ModuleMap {
/// A header that is known to reside within a given module,
/// whether it was included or excluded.
class KnownHeader {
- llvm::PointerIntPair<Module *, 2, ModuleHeaderRole> Storage;
+ llvm::PointerIntPair<Module *, 3, ModuleHeaderRole> Storage;
public:
KnownHeader() : Storage(nullptr, NormalHeader) {}
@@ -434,7 +436,8 @@ class ModuleMap {
/// 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
index 20696b1377d4d..029be1ed09b50 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1522,14 +1522,14 @@ bool HeaderSearch::hasModuleMap(StringRef FileName,
}
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<ModuleMap::KnownHeader>
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 87a90bc3e704f..174d639ac1f90 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -567,9 +567,11 @@ static bool isBetterKnownHeader(const ModuleMap::KnownHeader &New,
}
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 @@ ModuleMap::isHeaderUnavailableInModule(const FileEntry *Header,
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::addHeader(Module *Mod, Module::Header Header,
}
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
index ad6db3ce1993d..5521a9b41fb23 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1901,8 +1901,8 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
"Wrong data length in HeaderFileInfo deserialization");
while (d != End) {
uint32_t LocalSMID = endian::readNext<uint32_t, little, unaligned>(d);
- auto HeaderRole = static_cast<ModuleMap::ModuleHeaderRole>(LocalSMID & 3);
- LocalSMID >>= 2;
+ auto HeaderRole = static_cast<ModuleMap::ModuleHeaderRole>(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
index 76a997c864e5f..05e957c26ced0 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1820,8 +1820,8 @@ namespace {
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<uint32_t>(Value);
}
};
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 52062cdfb6502..905b45d07a37f 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -397,52 +397,34 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
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<const Module *> SeenDeps;
+ addAllSubmodulePrebuiltDeps(M, MD, SeenDeps);
+ addAllSubmoduleDeps(M, MD, SeenDeps);
+ addAllAffectingModules(M, MD, SeenDeps);
+
+ llvm::DenseSet<const FileEntry *> 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<const Module *> SeenModules;
- addAllSubmodulePrebuiltDeps(M, MD, SeenModules);
- llvm::DenseSet<const Module *> AddedModules;
- addAllSubmoduleDeps(M, MD, AddedModules);
- llvm::DenseSet<const Module *> 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
+ //
diff erent 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
index 0000000000000..030c92247a689
--- /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 <X/Excluded.h>
+
+//--- tu.m
+ at 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
More information about the cfe-commits
mailing list