[PATCH] Improve clang-tidy diagnostic output and filtering.

[Alexander Kornienko]

================
Comment at: clang-tidy/ClangTidy.cpp:292
@@ +291,3 @@
+static SourceLocation getLocation(SourceManager &SourceMgr, StringRef FilePath,
+                           unsigned Offset) {
+  SourceLocation Loc;
----------------
Daniel Jasper wrote:
> Fix the indentation.
Done.

================
Comment at: clang-tidy/ClangTidy.cpp:295
@@ +294,3 @@
+  if (!FilePath.empty()) {
+    const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
+    FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
----------------
Daniel Jasper wrote:
> Maybe assert that file is != NULL?
If you mean to assert instead of "if (!FilePath.empty())", then it's a bad idea, as we need to handle errors without locations, e.g. related to command line arguments.

If you mean to just add an assertion, then it will be redundant, as one of the functions called by createFileID already assert that file is not NULL.

================
Comment at: clang-tidy/ClangTidy.cpp:296
@@ +295,3 @@
+    const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
+    FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
+    Loc = SourceMgr.getLocForStartOfFile(ID);
----------------
Daniel Jasper wrote:
> Use SourceManager::translateFile?
An attempt to use translateFile results in the following assertion:

  tools/clang/lib/Basic/SourceManager.cpp:841: clang::FileID clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && "Invalid SLocOffset or bad function choice"' failed.

I didn't investigate this, but I'm inclined to believe this was a "bad function choice".

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:132
@@ -130,2 +131,3 @@
   SmallVector<ClangTidyError, 8> Errors;
+  SmallVector<bool, 8> RelatesToUserCode;
 };
----------------
Daniel Jasper wrote:
> Why not store this inside the ClangTidyError?
> 
> Or even on step further, why do we store ClangTidyErrors that don't relate to user code in the first place? Then all we'd need is a bool variable that stores whether the last non-Note error was related to user code so that we can drop uninteresting Notes. Or am I missing something?
The alternative you described turns out to require a bit more states information, than just one boolean (at least, another one: is there a ClangTidyError we are expecting updates to?). We'd need to handle state transitions differently, when a new warning comes, a new note comes, and the finish() method is invoked. I had this alternative code in draft, and I didn't like it.

As for storing the flag inside the ClangTidyError, I was thinking about this, but considered this flag to be not useful enough outside of the diagnostic consumer.


http://llvm-reviews.chandlerc.com/D2714