jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When pruning header search paths (to reduce the number of modules we need to
build explicitly), we can't prune the search paths used in (transitive)
dependencies of a module. Otherwise, we could end up with either of the
following dependency graphs:
X:<hash1> -> Y:<hash2>
X:<hash1> -> Y:<hash3>
depending on the search paths of the translation unit we discovered `X` and `Y`
from.
This patch fixes that.
Depends on D121295 <https://reviews.llvm.org/D121295>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D121303
Files:
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/header-search-pruning-transitive.c
Index: clang/test/ClangScanDeps/header-search-pruning-transitive.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/header-search-pruning-transitive.c
@@ -0,0 +1,169 @@
+// This test checks that pruning of header search paths produces consistent dependency graphs.
+//
+// When pruning header search paths for a module, we can't remove any paths its dependencies use.
+// Otherwise, we could get either of the following dependency graphs depending on the search path
+// configuration of the particular TU that first discovered the module:
+// X:<hash1> -> Y:<hash2>
+// X:<hash1> -> Y:<hash3>
+// We can't have the same version of module X depend on multiple different versions of Y based on
+// the TU configuration.
+//
+// Keeping all header search paths (transitive) dependencies use will ensure we get consistent
+// dependency graphs:
+// X:<hash1> -> Y:<hash2>
+// X:<hash4> -> Y:<hash3>
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- a/a.h
+//--- b/b.h
+//--- begin/begin.h
+//--- end/end.h
+//--- Y.h
+#include "begin.h"
+#if __has_include("a.h")
+#include "a.h"
+#endif
+#include "end.h"
+
+//--- X.h
+#include "Y.h"
+
+//--- module.modulemap
+module Y { header "Y.h" }
+module X { header "X.h" }
+
+//--- test.c
+#include "X.h"
+
+//--- cdb_with_a.json.template
+[{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang -c test.c -o DIR/test.o -fmodules -fimplicit-modules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Ibegin -Ia -Ib -Iend"
+}]
+
+//--- cdb_without_a.json.template
+[{
+ "file": "DIR/test.c",
+ "directory": "DIR",
+ "command": "clang -c test.c -o DIR/test.o -fmodules -fimplicit-modules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Ibegin -Ib -Iend"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_with_a.json.template > %t/cdb_with_a.json
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_without_a.json.template > %t/cdb_without_a.json
+
+// RUN: echo -%t > %t/results.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_with_a.json -format experimental-full -optimize-args >> %t/results.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_without_a.json -format experimental-full -optimize-args >> %t/results.json
+// RUN: cat %t/results.json | sed 's/\\/\//g' | FileCheck %s
+
+// CHECK: -[[PREFIX:.*]]
+// CHECK-NEXT: {
+// CHECK-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "[[HASH_Y_WITH_A:.*]]",
+// CHECK-NEXT: "module-name": "Y"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "[[HASH_X:.*]]",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/./X.h",
+// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "X"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "[[HASH_Y_WITH_A]]",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/./Y.h",
+// CHECK-NEXT: "[[PREFIX]]/./a/a.h",
+// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
+// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
+// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "Y"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-context-hash": "{{.*}}",
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "[[HASH_X]]",
+// CHECK-NEXT: "module-name": "X"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/test.c"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
+// CHECK-NEXT: {
+// CHECK-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "[[HASH_Y_WITHOUT_A:.*]]",
+// CHECK-NEXT: "module-name": "Y"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// Here is the actual check that this module X (which imports different version of Y)
+// also has a different context hash from the first version of module X.
+// CHECK-NOT: "context-hash": "[[HASH_X]]",
+// CHECK: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/./X.h",
+// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "X"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "[[HASH_Y_WITHOUT_A]]",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/./Y.h",
+// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
+// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
+// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "name": "Y"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-context-hash": "{{.*}}",
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "X"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/test.c"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
+// CHECK-NEXT: }
+// CHECK-NEXT: ]
+// CHECK-NEXT: }
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -23,9 +23,21 @@
// Only preserve search paths that were used during the dependency scan.
std::vector<HeaderSearchOptions::Entry> Entries = Opts.UserEntries;
Opts.UserEntries.clear();
- for (unsigned I = 0; I < Entries.size(); ++I)
- if (MF.SearchPathUsage[I])
- Opts.UserEntries.push_back(Entries[I]);
+
+ llvm::BitVector SearchPathUsage(Entries.size());
+ llvm::DenseSet<const serialization::ModuleFile *> Visited;
+ std::function<void(const serialization::ModuleFile *)> VisitMF =
+ [&](const serialization::ModuleFile *MF) -> void {
+ SearchPathUsage |= MF->SearchPathUsage;
+ Visited.insert(MF);
+ for (const serialization::ModuleFile *Import : MF->Imports)
+ if (!Visited.contains(Import))
+ VisitMF(Import);
+ };
+ VisitMF(&MF);
+
+ for (auto Idx : SearchPathUsage.set_bits())
+ Opts.UserEntries.push_back(Entries[Idx]);
}
CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
