[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf978ea498309: [clang][clang-scan-deps] Aggregate the full 
dependency information. (authored by Bigcheese).

Changed prior to commit:
  https://reviews.llvm.org/D70268?vs=233233&id=233455#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -129,6 +130,11 @@
 llvm::cl::init(ScanningOutputFormat::Make),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt FullCommandLine(
+"full-command-line",
+llvm::cl::desc("Include the full command lines to use to build modules"),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -189,9 +195,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string &Input,
-   llvm::Expected &MaybeFile,
-   SharedStream &OS, SharedStream &Errs) {
+static bool
+handleMakeDependencyToolResult(const std::string &Input,
+   llvm::Expected &MaybeFile,
+   SharedStream &OS, SharedStream &Errs) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [&Input, &Errs](llvm::StringError &Err) {
@@ -206,6 +213,184 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> &Set) {
+  std::vector Strings;
+  for (auto &&I : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+static llvm::json::Array toJSONSorted(std::vector V) {
+  std::sort(V.begin(), V.end(),
+[](const ClangModuleDep &A, const ClangModuleDep &B) {
+  return std::tie(A.ModuleName, A.ContextHash) <
+ std::tie(B.ModuleName, B.ContextHash);
+});
+
+  llvm::json::Array Ret;
+  for (const ClangModuleDep &CMD : V)
+Ret.push_back(llvm::json::Object(
+{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+  return Ret;
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ size_t InputIndex) {
+const FullDependencies &FD = FDR.FullDeps;
+
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.FileDeps);
+ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+
+std::unique_lock ul(Lock);
+for (const ModuleDeps &MD : FDR.DiscoveredModules) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+if (FullCommandLine)
+  ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
+  [&](ClangModuleDep CMD) -> const ModuleDeps & {
+return lookupModuleDeps(CMD);
+  });
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream &OS) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto &&M : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair &A, const ContextModulePair &B) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps &A, const InputDeps &B) {
+

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

Looks like the test for -full-command-line got dropped. I'll add that when I 
commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60692 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 233233.
Bigcheese added a comment.

Refactored `FullDependencies::getAdditionalCommandLine` and 
`ModuleDeps::getFullCommandLine` to share code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -129,6 +130,11 @@
 llvm::cl::init(ScanningOutputFormat::Make),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt FullCommandLine(
+"full-command-line",
+llvm::cl::desc("Include the full command lines to use to build modules"),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -189,9 +195,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string &Input,
-   llvm::Expected &MaybeFile,
-   SharedStream &OS, SharedStream &Errs) {
+static bool
+handleMakeDependencyToolResult(const std::string &Input,
+   llvm::Expected &MaybeFile,
+   SharedStream &OS, SharedStream &Errs) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [&Input, &Errs](llvm::StringError &Err) {
@@ -206,6 +213,184 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> &Set) {
+  std::vector Strings;
+  for (auto &&I : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+static llvm::json::Array toJSONSorted(std::vector V) {
+  std::sort(V.begin(), V.end(),
+[](const ClangModuleDep &A, const ClangModuleDep &B) {
+  return std::tie(A.ModuleName, A.ContextHash) <
+ std::tie(B.ModuleName, B.ContextHash);
+});
+
+  llvm::json::Array Ret;
+  for (const ClangModuleDep &CMD : V)
+Ret.push_back(llvm::json::Object(
+{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+  return Ret;
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ size_t InputIndex) {
+const FullDependencies &FD = FDR.FullDeps;
+
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.FileDeps);
+ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+
+std::unique_lock ul(Lock);
+for (const ModuleDeps &MD : FDR.DiscoveredModules) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+if (FullCommandLine)
+  ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
+  [&](ClangModuleDep CMD) -> const ModuleDeps & {
+return lookupModuleDeps(CMD);
+  });
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream &OS) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto &&M : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair &A, const ContextModulePair &B) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps &A, const InputDeps &B) {
+return A.FileName < B.FileName;
+  });
+
+using namespace llvm::json;
+
+Ar

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:29
+
+  Ret.push_back("-fno-implicit-modules");
+  Ret.push_back("-fno-implicit-module-maps");

@Bigcheese It looks like there's some duplication in argument logic between 
this function and `getAdditionalCommandLine`. Can it be factored out into one 
function?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:97
+  if (OutputPaths.empty())
+OutputPaths = Opts.Targets;
   Dependencies.push_back(File);

arphaman wrote:
> Bigcheese wrote:
> > arphaman wrote:
> > > What if `Opts.Targets` is empty?
> > If I recall correctly, `Opts.Targets` can never be empty. I gets set to 
> > `.o` if it would be empty.
> What I mean is, what if the client didn't request a dependency file in the 
> original compilation command? ScanDeps worker currently has a fake 
> `"clang-scan-deps dependency"` target that it adds if the target is empty, 
> but I do't think that should be reported as an output file.
Ah, I see now. This is only needed for the compilation database case (as that's 
the only way to map back to a compile command), it's not needed if you're 
calling `getFullDependencies` directly. I can remove it for now, but I'm not 
really sure what to replace it with.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 232483.
Bigcheese marked 2 inline comments as done.
Bigcheese added a comment.

- Removed `OutputPaths`.
- Add documentation for `AlreadySeen`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -129,6 +130,11 @@
 llvm::cl::init(ScanningOutputFormat::Make),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt FullCommandLine(
+"full-command-line",
+llvm::cl::desc("Include the full command lines to use to build modules"),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -189,9 +195,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string &Input,
-   llvm::Expected &MaybeFile,
-   SharedStream &OS, SharedStream &Errs) {
+static bool
+handleMakeDependencyToolResult(const std::string &Input,
+   llvm::Expected &MaybeFile,
+   SharedStream &OS, SharedStream &Errs) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [&Input, &Errs](llvm::StringError &Err) {
@@ -206,6 +213,184 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> &Set) {
+  std::vector Strings;
+  for (auto &&I : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+static llvm::json::Array toJSONSorted(std::vector V) {
+  std::sort(V.begin(), V.end(),
+[](const ClangModuleDep &A, const ClangModuleDep &B) {
+  return std::tie(A.ModuleName, A.ContextHash) <
+ std::tie(B.ModuleName, B.ContextHash);
+});
+
+  llvm::json::Array Ret;
+  for (const ClangModuleDep &CMD : V)
+Ret.push_back(llvm::json::Object(
+{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+  return Ret;
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ size_t InputIndex) {
+const FullDependencies &FD = FDR.FullDeps;
+
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.FileDeps);
+ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+
+std::unique_lock ul(Lock);
+for (const ModuleDeps &MD : FDR.DiscoveredModules) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+if (FullCommandLine)
+  ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
+  [&](ClangModuleDep CMD) -> const ModuleDeps & {
+return lookupModuleDeps(CMD);
+  });
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream &OS) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto &&M : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair &A, const ContextModulePair &B) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps &A, const InputDeps &B) {
+return A.FileName < B.FileName;
+  });
+
+using namespace llvm::json;
+
+Arr

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:97
+  if (OutputPaths.empty())
+OutputPaths = Opts.Targets;
   Dependencies.push_back(File);

Bigcheese wrote:
> arphaman wrote:
> > What if `Opts.Targets` is empty?
> If I recall correctly, `Opts.Targets` can never be empty. I gets set to 
> `.o` if it would be empty.
What I mean is, what if the client didn't request a dependency file in the 
original compilation command? ScanDeps worker currently has a fake 
`"clang-scan-deps dependency"` target that it adds if the target is empty, but 
I do't think that should be reported as an output file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese marked an inline comment as done.
Bigcheese added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:97
+  if (OutputPaths.empty())
+OutputPaths = Opts.Targets;
   Dependencies.push_back(File);

arphaman wrote:
> What if `Opts.Targets` is empty?
If I recall correctly, `Opts.Targets` can never be empty. I gets set to 
`.o` if it would be empty.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:97
+  if (OutputPaths.empty())
+OutputPaths = Opts.Targets;
   Dependencies.push_back(File);

What if `Opts.Targets` is empty?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese marked 2 inline comments as done.
Bigcheese added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:31
+  /// Modules that the input directly imports.
+  std::vector DirectModuleDependencies;
+  /// The Clang modules this input transitively depends on that have not 
already

arphaman wrote:
> Are the strings supposed to represent the module names here? For C++20 
> modules, will a single string be enough to represent both the module's name 
> and its partition name?
If it's a partition the module name will just contain a `:`.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:63
+  llvm::Expected
+  getFullDependencies(const tooling::CompilationDatabase &Compilations,
+  StringRef CWD, const llvm::StringSet<> &AlreadySeen);

arphaman wrote:
> Can you add a comment on how `AlreadySeen` is supposed to be used. Are 
> clients expected to update it once they get more dependencies?
Will do. I'm still working out exactly how `AlreadySeen` should work. Ideally 
it would be shared across all workers of the same `DependencyScanningService`, 
but that needs thread synchronization and could have rather high overhead. The 
current model is that it's per worker, and you should expect to need to 
deduplicate modules across workers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:31
+  /// Modules that the input directly imports.
+  std::vector DirectModuleDependencies;
+  /// The Clang modules this input transitively depends on that have not 
already

Are the strings supposed to represent the module names here? For C++20 modules, 
will a single string be enough to represent both the module's name and its 
partition name?



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:63
+  llvm::Expected
+  getFullDependencies(const tooling::CompilationDatabase &Compilations,
+  StringRef CWD, const llvm::StringSet<> &AlreadySeen);

Can you add a comment on how `AlreadySeen` is supposed to be used. Are clients 
expected to update it once they get more dependencies?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70268/new/

https://reviews.llvm.org/D70268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Herald added subscribers: cfe-commits, tschuett, dexonsmith, mgrang.
Herald added a project: clang.
Bigcheese added reviewers: arphaman, kousikk.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -13,6 +13,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Options.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
@@ -130,9 +131,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string &Input,
-   llvm::Expected &MaybeFile,
-   SharedStream &OS, SharedStream &Errs) {
+static bool
+handleMakeDependencyToolResult(const std::string &Input,
+   llvm::Expected &MaybeFile,
+   SharedStream &OS, SharedStream &Errs) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [&Input, &Errs](llvm::StringError &Err) {
@@ -147,6 +149,141 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> &Set) {
+  std::vector Strings;
+  for (auto &&I : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependencies FD, size_t InputIndex) {
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.DirectFileDependencies);
+ID.ModuleDeps = std::move(FD.DirectModuleDependencies);
+ID.OutputPaths = std::move(FD.OutputPaths);
+
+std::unique_lock ul(Lock);
+for (ModuleDeps &MD : FD.ClangModuleDeps) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream &OS) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto &&M : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair &A, const ContextModulePair &B) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps &A, const InputDeps &B) {
+return std::tie(A.FileName, A.OutputPaths) <
+   std::tie(B.FileName, B.OutputPaths);
+  });
+
+using namespace llvm::json;
+
+Array OutModules;
+for (auto &&ModName : ModuleNames) {
+  auto &MD = Modules[ModName];
+  Object O{
+  {"name", MD.ModuleName},
+  {"context-hash", MD.ContextHash},
+  {"file-deps", toJSONSorted(MD.FileDeps)},
+  {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
+  {"clang-modulemap-file", MD.ClangModuleMapFile},
+  };
+  OutModules.push_back(std::move(O));
+}
+
+Array TUs;
+for (auto &&I : Inputs) {
+  Object O{
+  {"input-file", I.FileName},
+  {"clang-context-hash", I.ContextHash},
+  {"file-deps", I.FileDeps},
+  {"clang-module-deps", I.ModuleDeps},
+  {"output-files", I.OutputPaths},
+  };
+  TUs.push_back(std::move(O));
+}
+
+Object Output{
+{"modules", std::move(OutModules)},
+{"translation-units", std::move(TUs)},
+};
+
+OS << llvm::formatv("{0:2}\n", Value(std::move(Output)));
+  }
+
+private:
+  struct ContextModulePair {
+std::string ContextHash;
+std::string ModuleName;
+mutable size_t InputIndex;
+
+bool operator==(const ContextModulePair &Other) const {
+  return ContextHash == Other.ContextHash && ModuleName