[clang] d73daa9 - [clang][deps] Don't prune search paths used by dependencies
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 16 04:18:02 PDT 2022
Author: Jan Svoboda
Date: 2022-03-16T12:17:53+01:00
New Revision: d73daa9135463ad4d4a08a9f0a75e109f921ad54
URL: https://github.com/llvm/llvm-project/commit/d73daa9135463ad4d4a08a9f0a75e109f921ad54
DIFF: https://github.com/llvm/llvm-project/commit/d73daa9135463ad4d4a08a9f0a75e109f921ad54.diff
LOG: [clang][deps] Don't prune search paths used by dependencies
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.
Reviewed By: dexonsmith
Differential Revision: https://reviews.llvm.org/D121303
Added:
clang/test/ClangScanDeps/header-search-pruning-transitive.c
Modified:
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Removed:
################################################################################
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 4d70b0c80aad2..d3a84bf8a8e47 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -23,9 +23,21 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts,
// 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) {
+ 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(
diff --git a/clang/test/ClangScanDeps/header-search-pruning-transitive.c b/clang/test/ClangScanDeps/header-search-pruning-transitive.c
new file mode 100644
index 0000000000000..434f7e5d9fb22
--- /dev/null
+++ b/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
diff erent 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
diff erent version of Y)
+// also has a
diff erent 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: }
More information about the cfe-commits
mailing list