jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, Bigcheese.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This patch changes the PCM serialization logic to refer to input files by their
"requested" name. This fixes a bug where the dependency scanner reports the
"final" file paths, which can result in failed explicit compiles due to the
`module.modulemap` file not being surrounded by the expected framework
directory structure.
Depends on D135634 <https://reviews.llvm.org/D135634>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135636
Files:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-file-name-as-requested.m
Index: clang/test/ClangScanDeps/modules-file-name-as-requested.m
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-file-name-as-requested.m
@@ -0,0 +1,63 @@
+// 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
+
+//--- real/module.modulemap
+framework module FW { header "Header.h" }
+//--- real/Header.h
+//--- overlay.json.template
+{
+ "case-sensitive": "false",
+ "version": "0",
+ "roots": [
+ {
+ "contents": [
+ {
+ "external-contents" : "DIR/real/Header.h",
+ "name" : "Header.h",
+ "type" : "file"
+ }
+ ],
+ "name": "DIR/frameworks/FW.framework/Headers",
+ "type": "directory"
+ },
+ {
+ "contents": [
+ {
+ "external-contents": "DIR/real/module.modulemap",
+ "name": "module.modulemap",
+ "type": "file"
+ }
+ ],
+ "name": "DIR/frameworks/FW.framework/Modules",
+ "type": "directory"
+ }
+ ]
+}
+
+//--- modules/module.modulemap
+module Importer { header "header.h" }
+//--- modules/header.h
+#include <FW/Header.h>
+
+//--- cdb.json.template
+[{
+ "file": "DIR/tu.m",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -Werror=non-modular-include-in-module -ivfsoverlay DIR/overlay.json -F DIR/frameworks -I DIR/modules -c DIR/tu.m -o DIR/tu.o"
+}]
+
+//--- tu.m
+@import Importer;
+
+// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.template > %t/overlay.json
+// 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: %deps-to-rsp %t/result.json --module-name=FW > %t/FW.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=Importer > %t/Importer.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp
+// RUN: %clang @%t/FW.cc1.rsp
+// RUN: %clang @%t/Importer.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
@@ -440,10 +440,12 @@
addAllAffectingModules(M, MD, SeenDeps);
MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
- *MF, [&](const FileEntry *FE) {
- if (FE->getName().endswith("__inferred_module.map"))
+ *MF, [&](FileEntryRef FE) {
+ if (FE.getNameAsRequested().endswith("__inferred_module.map"))
return;
- MD.ModuleMapFileDeps.emplace_back(FE->getName());
+ SmallString<128> CanonicalPath = FE.getNameAsRequested();
+ ModMapInfo.canonicalizeModuleMapPath(CanonicalPath);
+ MD.ModuleMapFileDeps.emplace_back(CanonicalPath);
});
CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1496,12 +1496,14 @@
/// An input file.
struct InputFileEntry {
- const FileEntry *File;
+ FileEntryRef File;
bool IsSystemFile;
bool IsTransient;
bool BufferOverridden;
bool IsTopLevelModuleMap;
uint32_t ContentHash[2];
+
+ InputFileEntry(FileEntryRef File) : File(File) {}
};
} // namespace
@@ -1558,8 +1560,7 @@
continue;
}
- InputFileEntry Entry;
- Entry.File = Cache->OrigEntry;
+ InputFileEntry Entry(*Cache->OrigEntry);
Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
Entry.IsTransient = Cache->IsTransient;
Entry.BufferOverridden = Cache->BufferOverridden;
@@ -1574,9 +1575,8 @@
if (MemBuff)
ContentHash = hash_value(MemBuff->getBuffer());
else
- // FIXME: The path should be taken from the FileEntryRef.
PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
- << Entry.File->getName();
+ << Entry.File.getNameAsRequested();
}
auto CH = llvm::APInt(64, ContentHash);
Entry.ContentHash[0] =
@@ -1616,14 +1616,13 @@
RecordData::value_type Record[] = {
INPUT_FILE,
InputFileOffsets.size(),
- (uint64_t)Entry.File->getSize(),
+ (uint64_t)Entry.File.getSize(),
(uint64_t)getTimestampForOutput(Entry.File),
Entry.BufferOverridden,
Entry.IsTransient,
Entry.IsTopLevelModuleMap};
- // FIXME: The path should be taken from the FileEntryRef.
- EmitRecordWithPath(IFAbbrevCode, Record, Entry.File->getName());
+ EmitRecordWithPath(IFAbbrevCode, Record, Entry.File.getNameAsRequested());
}
// Emit content hash for this file.
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9180,14 +9180,14 @@
void ASTReader::visitTopLevelModuleMaps(
serialization::ModuleFile &MF,
- llvm::function_ref<void(const FileEntry *FE)> Visitor) {
+ llvm::function_ref<void(FileEntryRef FE)> Visitor) {
unsigned NumInputs = MF.InputFilesLoaded.size();
for (unsigned I = 0; I < NumInputs; ++I) {
InputFileInfo IFI = readInputFileInfo(MF, I + 1);
if (IFI.TopLevelModuleMap)
// FIXME: This unnecessarily re-reads the InputFileInfo.
if (auto FE = getInputFile(MF, I + 1).getFile())
- Visitor(FE);
+ Visitor(*FE);
}
}
Index: clang/lib/Frontend/FrontendAction.cpp
===================================================================
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -638,11 +638,10 @@
if (&MF != &PrimaryModule)
CI.getFrontendOpts().ModuleFiles.push_back(MF.FileName);
- ASTReader->visitTopLevelModuleMaps(
- PrimaryModule, [&](const FileEntry *FE) {
- CI.getFrontendOpts().ModuleMapFiles.push_back(
- std::string(FE->getName()));
- });
+ ASTReader->visitTopLevelModuleMaps(PrimaryModule, [&](FileEntryRef FE) {
+ CI.getFrontendOpts().ModuleMapFiles.push_back(
+ std::string(FE.getNameAsRequested()));
+ });
}
// Set up the input file for replay purposes.
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -2319,8 +2319,7 @@
/// Visit all the top-level module maps loaded when building the given module
/// file.
void visitTopLevelModuleMaps(serialization::ModuleFile &MF,
- llvm::function_ref<
- void(const FileEntry *)> Visitor);
+ llvm::function_ref<void(FileEntryRef)> Visitor);
bool isProcessingUpdateRecords() { return ProcessingUpdateRecords; }
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
