[PATCH] Add the check name to the clang-tidy diagnostic output.

Alexander Kornienko alexfh at google.com
Mon Jan 13 01:59:42 PST 2014



================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:50
@@ -48,3 +49,3 @@
 struct ClangTidyError {
-  ClangTidyError(const ClangTidyMessage &Message);
+  ClangTidyError(const std::string &CheckName, const ClangTidyMessage &Message);
 
----------------
Daniel Jasper wrote:
> Why not StringRef?
Don't know why. Changed to StringRef.

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:91
@@ -87,1 +90,3 @@
 
+  /// \brief Returns the name of clang-tidy check, which produced this
+  /// diagnostic ID.
----------------
Daniel Jasper wrote:
> .. name of the clang-tidy check which .. (add "the", remove comma).
Done.

================
Comment at: clang-tidy/ClangTidy.h:86
@@ -78,3 +85,3 @@
 protected:
   ClangTidyContext *Context;
 
----------------
Daniel Jasper wrote:
> Do the individual check implementation still need access to the Context? If not, making it private might be a good step towards better layering.
Done.

================
Comment at: clang-tidy/ClangTidy.h:79
@@ +78,3 @@
+  /// \brief Add a diagnostic with the check's name.
+  DiagnosticBuilder diag(SourceLocation Loc, StringRef Message);
+
----------------
Daniel Jasper wrote:
> All the other methods in Clang are call "Diag". Consistency with that might trump local consistency here (or consistency with the Coding Standards), but I am not sure.
I think, having a coding style-compliant name is better (as it's also local style-compliant), as being compliant with the naming style of similar methods in completely different classes. Plus, creating more style violations will make fixing them harder, when/if someone decides to do this ;)


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



More information about the cfe-commits mailing list