aganea accepted this revision.
aganea added inline comments.
This revision is now accepted and ready to land.


================
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"
----------------
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 ]]!


================
Comment at: clang/tools/clang-scan-deps/CMakeLists.txt:3
+    Core
+    Support
+)
----------------
Two space indent.


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

https://reviews.llvm.org/D60233



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

Reply via email to