[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

Wed Jun 12 14:38:49 PDT 2019

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

================
Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"
----------------
aganea wrote:
> arphaman wrote:
> > aganea wrote:
> > > Is `-MD -MF` required for clang-scan-deps to work? Can't we append those arguments automatically, so that only include paths are needed in the CDB?
> > > 
> > > Would it possible for the tool to produce a collated file? A real-world usage would otherwise produce tens of thousands of files. Which then would be opened/parsed by the calling application.
> > I agree, the tool shouldn't rely on those options being there. `-MD -MF` in this test right now a crutch to get the dependencies printed somewhere.
> > 
> > I think the tool by default should print out the collated dependencies to STDOUT instead of writing them to files that were specified for the build. The `-MD -MF` options will not be required as well. This implementation requires some changes in the dependency collector, which I am planning to do as part of clang-scan-deps in the follow-up patches. Will it be ok to keep this patch as it is right now, and transition to printing out the collated dependencies without requiring the options as a follow-up?
> > 
> > 
> Sounds good! Please cc me on the upcoming reviews, we have a good use-case for clang-scan-deps in [[ http://www.fastbuild.org/docs/home.html | Fastbuild ]]!
Sure, I will CC you on the upcoming patches.

================
Comment at: clang/tools/clang-scan-deps/CMakeLists.txt:3
+    Core
+    Support
+)
----------------
aganea wrote:
> Two space indent.
Oops, I missed this comment. Fixed in r363205.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60233