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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
