[clang][deps] Stop sorting file dependencies #114079
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesThe dependency scanner collects file dependencies of modules into a Patch is 58.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114079.diff 36 Files Affected:
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 3de19d756da00d..7b71a7a8e648b2 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -122,7 +122,7 @@ struct ModuleDeps {
/// A collection of absolute paths to files that this module directly depends
/// on, not including transitive dependencies.
- llvm::StringSet<> FileDeps;
+ std::vector<std::string> FileDeps;
/// A collection of absolute paths to module map files that this module needs
/// to know about. The ordering is significant.
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 637416cd1fc621..f930dfaca3cc05 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -779,23 +779,23 @@ static StringRef makeAbsoluteAndPreferred(CompilerInstance &CI, StringRef Path,
void ModuleDepCollector::addFileDep(StringRef Path) {
if (IsStdModuleP1689Format) {
// Within P1689 format, we don't want all the paths to be absolute path
- // since it may violate the tranditional make style dependencies info.
- FileDeps.push_back(std::string(Path));
+ // since it may violate the traditional make style dependencies info.
+ FileDeps.emplace_back(Path);
return;
}
llvm::SmallString<256> Storage;
Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
- FileDeps.push_back(std::string(Path));
+ FileDeps.emplace_back(Path);
}
void ModuleDepCollector::addFileDep(ModuleDeps &MD, StringRef Path) {
if (IsStdModuleP1689Format) {
- MD.FileDeps.insert(Path);
+ MD.FileDeps.emplace_back(Path);
return;
}
llvm::SmallString<256> Storage;
Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
- MD.FileDeps.insert(Path);
+ MD.FileDeps.emplace_back(Path);
}
diff --git a/clang/test/ClangScanDeps/diagnostics.c b/clang/test/ClangScanDeps/diagnostics.c
index bc05594f7828dc..30751934634424 100644
--- a/clang/test/ClangScanDeps/diagnostics.c
+++ b/clang/test/ClangScanDeps/diagnostics.c
@@ -34,8 +34,8 @@ module mod { header "mod.h" }
// CHECK: ],
// CHECK-NEXT: "context-hash": "[[HASH_MOD:.*]]",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/mod.h"
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/mod.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "mod"
@@ -55,7 +55,7 @@ module mod { header "mod.h" }
// CHECK-NOT: "-fimplicit-module-maps"
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.c"
+// CHECK-DAG: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/header-search-pruning-transitive.c b/clang/test/ClangScanDeps/header-search-pruning-transitive.c
index 6c422650902689..8f6489515b60b4 100644
--- a/clang/test/ClangScanDeps/header-search-pruning-transitive.c
+++ b/clang/test/ClangScanDeps/header-search-pruning-transitive.c
@@ -72,8 +72,8 @@ module X { header "X.h" }
// CHECK: ],
// CHECK-NEXT: "context-hash": "[[HASH_X:.*]]",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/X.h",
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/X.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "X"
@@ -85,11 +85,11 @@ module X { header "X.h" }
// 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-DAG: "[[PREFIX]]/Y.h"
+// CHECK-DAG: "[[PREFIX]]/a/a.h"
+// CHECK-DAG: "[[PREFIX]]/begin/begin.h"
+// CHECK-DAG: "[[PREFIX]]/end/end.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "Y"
@@ -107,7 +107,7 @@ module X { header "X.h" }
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/test.c"
+// CHECK-DAG: "[[PREFIX]]/test.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
// CHECK-NEXT: }
@@ -128,8 +128,8 @@ module X { header "X.h" }
// 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-DAG: "[[PREFIX]]/X.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "X"
@@ -141,10 +141,10 @@ module X { header "X.h" }
// 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-DAG: "[[PREFIX]]/Y.h"
+// CHECK-DAG: "[[PREFIX]]/begin/begin.h"
+// CHECK-DAG: "[[PREFIX]]/end/end.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "Y"
@@ -162,7 +162,7 @@ module X { header "X.h" }
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/test.c"
+// CHECK-DAG: "[[PREFIX]]/test.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/test.c"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/line-directive.c b/clang/test/ClangScanDeps/line-directive.c
index 4bf532167a9352..85a683966b73f0 100644
--- a/clang/test/ClangScanDeps/line-directive.c
+++ b/clang/test/ClangScanDeps/line-directive.c
@@ -11,8 +11,8 @@
// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.c"
-// CHECK-NEXT: "[[PREFIX]]/header.h"
+// CHECK-DAG: "[[PREFIX]]/tu.c"
+// CHECK-DAG: "[[PREFIX]]/header.h"
// CHECK-NEXT: ]
//--- cdb.json.template
diff --git a/clang/test/ClangScanDeps/link-libraries.c b/clang/test/ClangScanDeps/link-libraries.c
index bc0b0c546ea032..7e8018b5f535d3 100644
--- a/clang/test/ClangScanDeps/link-libraries.c
+++ b/clang/test/ClangScanDeps/link-libraries.c
@@ -44,8 +44,8 @@ module transitive {
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/Inputs/frameworks/Framework.framework/Headers/Framework.h"
-// CHECK-NEXT: "[[PREFIX]]/Inputs/frameworks/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/Inputs/frameworks/Framework.framework/Headers/Framework.h"
+// CHECK-DAG: "[[PREFIX]]/Inputs/frameworks/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [
// CHECK-NEXT: {
@@ -67,8 +67,8 @@ module transitive {
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/direct.h"
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/direct.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "direct"
@@ -89,10 +89,10 @@ module transitive {
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/Inputs/frameworks/module.modulemap"
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
-// CHECK-NEXT: "[[PREFIX]]/root.h"
-// CHECK-NEXT: "[[PREFIX]]/root/textual.h"
+// CHECK-DAG: "[[PREFIX]]/Inputs/frameworks/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/root.h"
+// CHECK-DAG: "[[PREFIX]]/root/textual.h"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "root"
@@ -104,8 +104,8 @@ module transitive {
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
-// CHECK-NEXT: "[[PREFIX]]/transitive.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/transitive.h"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [
// CHECK-NEXT: {
diff --git a/clang/test/ClangScanDeps/modules-context-hash.c b/clang/test/ClangScanDeps/modules-context-hash.c
index 69fd8561a2b32a..1006c74dfcd0f3 100644
--- a/clang/test/ClangScanDeps/modules-context-hash.c
+++ b/clang/test/ClangScanDeps/modules-context-hash.c
@@ -34,9 +34,9 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "[[HASH_MOD_A:.*]]",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/a/dep.h",
-// CHECK-NEXT: "[[PREFIX]]/mod.h",
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/a/dep.h"
+// CHECK-DAG: "[[PREFIX]]/mod.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "mod"
@@ -54,7 +54,7 @@
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.c"
+// CHECK-DAG: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
@@ -72,9 +72,9 @@
// CHECK: ],
// CHECK-NOT: "context-hash": "[[HASH_MOD_A]]",
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/b/dep.h",
-// CHECK-NEXT: "[[PREFIX]]/mod.h",
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/b/dep.h"
+// CHECK-DAG: "[[PREFIX]]/mod.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "mod"
@@ -92,7 +92,7 @@
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.c"
+// CHECK-DAG: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/modules-dep-args.c b/clang/test/ClangScanDeps/modules-dep-args.c
index 14de2e8f4594a1..9d31844e31a005 100644
--- a/clang/test/ClangScanDeps/modules-dep-args.c
+++ b/clang/test/ClangScanDeps/modules-dep-args.c
@@ -58,8 +58,8 @@ module Direct { header "direct.h" }
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/direct.h",
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/direct.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "Direct"
@@ -71,8 +71,8 @@ module Direct { header "direct.h" }
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/transitive.h"
+// CHECK-DAG: "[[PREFIX]]/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/transitive.h"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "Transitive"
@@ -96,7 +96,7 @@ module Direct { header "direct.h" }
// CHECK_EAGER: "-fmodule-file=[[PREFIX]]/{{.*}}/Direct-{{.*}}.pcm"
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.c"
+// CHECK-DAG: "[[PREFIX]]/tu.c"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/modules-extern-submodule.c b/clang/test/ClangScanDeps/modules-extern-submodule.c
index 6e9082b0165fdf..aabc4090978a46 100644
--- a/clang/test/ClangScanDeps/modules-extern-submodule.c
+++ b/clang/test/ClangScanDeps/modules-extern-submodule.c
@@ -49,10 +49,10 @@ module third {}
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/first/first/first.h",
-// CHECK-NEXT: "[[PREFIX]]/first/first/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/second/second/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/second/second/sub.modulemap"
+// CHECK-DAG: "[[PREFIX]]/first/first/first.h"
+// CHECK-DAG: "[[PREFIX]]/first/first/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second/sub.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "first"
@@ -72,10 +72,10 @@ module third {}
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/second/second/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/second/second/sub.h",
-// CHECK-NEXT: "[[PREFIX]]/second/second/sub.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/third/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second/sub.h"
+// CHECK-DAG: "[[PREFIX]]/second/second/sub.modulemap"
+// CHECK-DAG: "[[PREFIX]]/third/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "second"
@@ -90,7 +90,7 @@ module third {}
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/third/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/third/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "third"
@@ -113,7 +113,7 @@ module third {}
// CHECK: "-fmodule-file=first=[[PREFIX]]/cache/{{.*}}/first-{{.*}}.pcm",
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.m"
+// CHECK-DAG: "[[PREFIX]]/tu.m"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/modules-extern-unrelated.m b/clang/test/ClangScanDeps/modules-extern-unrelated.m
index 724779c12a97b7..0a6b7dc91db1b9 100644
--- a/clang/test/ClangScanDeps/modules-extern-unrelated.m
+++ b/clang/test/ClangScanDeps/modules-extern-unrelated.m
@@ -38,7 +38,7 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/first/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "first"
@@ -50,8 +50,8 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/first/first_other.h",
-// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/first/first_other.h"
+// CHECK-DAG: "[[PREFIX]]/first/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "first_other"
@@ -69,9 +69,9 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/second/second.h",
-// CHECK-NEXT: "[[PREFIX]]/second/second.modulemap"
+// CHECK-DAG: "[[PREFIX]]/first/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second.h"
+// CHECK-DAG: "[[PREFIX]]/second/second.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "second"
@@ -93,10 +93,10 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/second/second.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/zeroth/module.modulemap",
-// CHECK-NEXT: "[[PREFIX]]/zeroth/zeroth.h"
+// CHECK-DAG: "[[PREFIX]]/first/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/second/second.modulemap"
+// CHECK-DAG: "[[PREFIX]]/zeroth/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/zeroth/zeroth.h"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "zeroth"
@@ -117,7 +117,7 @@
// CHECK: ],
// CHECK-NEXT: "executable": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/tu.m"
+// CHECK-DAG: "[[PREFIX]]/tu.m"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/modules-file-path-isolation.c b/clang/test/ClangScanDeps/modules-file-path-isolation.c
index d4fef2f4fdd28e..ddd26a70cb8ed5 100644
--- a/clang/test/ClangScanDeps/modules-file-path-isolation.c
+++ b/clang/test/ClangScanDeps/modules-file-path-isolation.c
@@ -17,8 +17,8 @@
// CHECK: "modules": [
// CHECK-NEXT: {
// CHECK: "file-deps": [
-// CHECK-NEXT: "{{.*}}A.h",
-// CHECK-NEXT: "{{.*}}module.modulemap"
+// CHECK-DAG: "{{.*}}A.h"
+// CHECK-DAG: "{{.*}}module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "A"
diff --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
index e4d5caff9236e7..c7e8c12ea950cc 100644
--- a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
+++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
@@ -25,8 +25,8 @@
// CHECK: ],
// CHECK-NEXT: "context-hash": "[[HASH_H2:[A-Z0-9]+]]",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/Inputs/header2.h",
-// CHECK-NEXT: "[[PREFIX]]/Inputs/module.modulemap"
+// CHECK-DAG: "[[PREFIX]]/Inputs/header2.h"
+// CHECK-DAG: "[[PREFIX]]/Inputs/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [],
// CHECK-NEXT: "name": "header2"
@@ -44,9 +44,9 @@
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK: "file-deps": [
-// CHECK-NEXT: "[[PREFIX]]/modules-fmodule-name-no-module-built.m"
-// CHECK-NEXT: "[[PREFIX]]/Inputs/header3.h"
-// CHECK-NEXT: "[[PREFIX]]/Inputs/header.h"
+// CHECK-DAG: "[[PREFIX]]/modules-fmodule-name-no-module-built.m"
+// CHECK-DAG: "[[PREFIX]]/Inputs/header3.h"
+// CHECK-DAG: "[[PREFIX]]/Inputs/header.h"
// CHECK-NEXT: ],
// CHECK-NEXT: "input-file": "[[PREFIX]]/modules-fmodule-name-no-module-built.m"
// CHECK-NEXT: }
diff --git a/clang/test/ClangScanDeps/modules-full-by-mod-name.c b/clang/test/ClangScanDeps/m...
[truncated]
|
benlangmuir
reviewed
Oct 29, 2024
| @@ -472,7 +465,8 @@ class FullDeps { | |||
| JOS.attributeArray("command-line", | |||
| toJSONStrings(JOS, MD.getBuildArguments())); | |||
| JOS.attribute("context-hash", StringRef(MD.ID.ContextHash)); | |||
| JOS.attributeArray("file-deps", toJSONSorted(JOS, MD.FileDeps)); | |||
| // TODO: Shuffle these to prevent tests from depending on the order. | |||
There was a problem hiding this comment.
If the order is deterministic, why wouldn't we want to depend on it in tests?
There was a problem hiding this comment.
Because we might want to change one arbitrary deterministic ordering to another arbitrary deterministic ordering and I'd like for the tests to be resilient against that so that they don't need to change every time we do that.
There was a problem hiding this comment.
It's not clear to me the value of being able to change the order (I don't think this would happen often) outweighs the benefits of better test coverage for deterministic output.
Regardless, I don't want to random shuffling the output of clang-scan-deps, because it breaks diffing the output. To me this TODO just seems like a bad idea.
There was a problem hiding this comment.
Hmm, I wanted to seed the random shuffle with something "stable" (like the repo revision number) to be able to diff in tests. I also wanted to add a clang-scan-deps flag to disable it entirely for development purposes. Is that something that you still consider a bad idea?
There was a problem hiding this comment.
It's better than random shuffle, but I would prefer we just keep the natural order. I'm not convinced we will change the order often enough for test churn to matter.
| @@ -34,8 +34,8 @@ module mod { header "mod.h" } | |||
| // CHECK: ], | |||
| // CHECK-NEXT: "context-hash": "[[HASH_MOD:.*]]", | |||
| // CHECK-NEXT: "file-deps": [ | |||
| // CHECK-NEXT: "[[PREFIX]]/mod.h" | |||
| // CHECK-NEXT: "[[PREFIX]]/module.modulemap" | |||
| // CHECK-DAG: "[[PREFIX]]/mod.h" | |||
There was a problem hiding this comment.
I'm concerned that making all the tests use CHECK-DAG for dependencies will hide non-deterministic output in the future.
There was a problem hiding this comment.
We already have a test that diffs two scan results (submodule-order.c), so I believe that would catch non-deterministic ordering. (We might need to tweak the test so that one of the modules contains multiple files.)
There was a problem hiding this comment.
This test has very few files and isn't making any attempt to be exhaustive about which kinds of input files it has. I don't think it provides very good coverage for checking the order is deterministic.
|
Ok, abandoning in favor of #114457. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The dependency scanner collects file dependencies of modules into a
llvm::StringSet. However, we don't need the deduplication it performs, since that's already guaranteed by the PCM we read the paths from. We also don't need to report the file dependencies in any particular order. This allows us to switch from a set to a vector to avoid the overhead of string hashing and establishing deterministic order. The order of file dependencies is no longer lexicographical, but it's still deterministic (inherited from the PCM file).