alexfh added inline comments. ================ Comment at: clang-tidy/ClangTidy.cpp:115 @@ +114,3 @@ + // + // Specify the build directory of the source file as current working + // directroy to find source file correctly. ---------------- Maybe `Change the directory to the one used during the analysis.`?
================ Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:120 @@ +119,3 @@ + bool IsWarningAsError, + const std::string &BuildDirectory) + : BuildDirectory(std::move(BuildDirectory)), CheckName(CheckName), ---------------- This should be a StringRef for consistency. ================ Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:66 @@ +65,3 @@ + // It's an absolute path which is `directory` field of the source file in + // compilation database (compile_commands.json). If users don't specify + // compilation database directory, it is the current directory where ---------------- `compile_commands.json` is one format of a compilation database, there are others. So it might be not very useful to name it here. ================ Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:70 @@ +69,3 @@ + // + // It can be empty in some cases, e.g. the source file is not exist. + std::string BuildDirectory; ---------------- **does** not exist ================ Comment at: test/clang-tidy/clang-tidy-run-with-database.cpp:1 @@ +1,2 @@ +// RUN: DATABASEDIR=$S/Inputs/compilation-database +// RUN: sed 's|test_dir|%S|g' %S/Inputs/compilation-database/template.json > %S/Inputs/compilation-database/compile_commands.json ---------------- I think, this test now needs `// REQUIRES: shell` ================ Comment at: test/clang-tidy/clang-tidy-run-with-database.cpp:2 @@ +1,3 @@ +// RUN: DATABASEDIR=$S/Inputs/compilation-database +// RUN: sed 's|test_dir|%S|g' %S/Inputs/compilation-database/template.json > %S/Inputs/compilation-database/compile_commands.json +// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %S/Inputs/compilation-database %s -header-filter=.* ---------------- The test shouldn't change anything in the source directory. It can create any setup it needs in the temporary directory though (the %t placeholder would be useful for this). http://reviews.llvm.org/D17335 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
