[PATCH] D17335: [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:120
@@ +119,3 @@
+                               bool IsWarningAsError,
+                               const std::string &BuildDirectory)
+    : BuildDirectory(std::move(BuildDirectory)), CheckName(CheckName),
----------------
hokein wrote:
> We might not use `StringRef` here since 
> `SourceManager.getFileManager().getVirtualFileSystem().getCurrentWorkingDirectory()` returns `std::string`.
As discussed offline, it makes sense to avoid calling `SourceManager.getFileManager().getVirtualFileSystem().getCurrentWorkingDirectory()` for each error, since it already has a cost of at least a string allocation/copy, but might also be an actual I/O operation. We can assume the working directory doesn't change while parsing a file, so we can cache it in the `ClangTidyContext` together with the current file name. Once we do this, we can get a `StringRef` to the working directory stored in `ClangTidyContext` and pass it here (but still store a `std::string` to avoid dependencies on a lifetime of an external string).

================
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. If users don't specify compilation database
+  // directory, it is the current directory where clang-tidy runs.
----------------
hokein wrote:
> However, I see the `compile_commands.json` is hardcoded in source. http://clang.llvm.org/doxygen/JSONCompilationDatabase_8cpp_source.html#l00111
Yes, but this is just one implementation of a compilation database. There are other implementations as well.

================
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. If users don't specify compilation database
+  // directory, it is the current directory where clang-tidy runs.
----------------
alexfh wrote:
> hokein wrote:
> > However, I see the `compile_commands.json` is hardcoded in source. http://clang.llvm.org/doxygen/JSONCompilationDatabase_8cpp_source.html#l00111
> Yes, but this is just one implementation of a compilation database. There are other implementations as well.
nit: "the compilation database"


http://reviews.llvm.org/D17335