jansvoboda11 updated this revision to Diff 461432.
jansvoboda11 added a comment.
Fix failing test by undoing some API changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134224/new/
https://reviews.llvm.org/D134224
Files:
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
clang/test/ClangScanDeps/modules-excluded-header.m
Index: clang/test/ClangScanDeps/modules-excluded-header.m
===================================================================
--- /dev/null
+++ 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
+@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
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ 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<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
+ // different mechanism (lazy loading mode).
+ if (SeenModuleMaps.contains(FE))
+ return;
+ MD.ModuleMapFileDeps.emplace_back(FE->getName());
+ });
CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
MD, [&](CompilerInvocation &BuildInvocation) {
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1819,8 +1819,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<uint32_t>(Value);
}
};
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1901,8 +1901,8 @@
"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.
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ 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];
+ (void) Headers[Header.Entry].push_back(KH);
Mod->Headers[Module::HK_Excluded].push_back(std::move(Header));
}
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1523,13 +1523,14 @@
ModuleMap::KnownHeader
HeaderSearch::findModuleForHeader(const FileEntry *File,
- bool AllowTextual) const {
+ 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>
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ 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<Module *, 2, ModuleHeaderRole> Storage;
+ llvm::PointerIntPair<Module *, 3, ModuleHeaderRole> 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,
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ 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.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
