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

Daniel Jasper djasper at google.com
Sun Jan 12 09:33:12 PST 2014



================
Comment at: clang-tidy/ClangTidy.h:79
@@ +78,3 @@
+  /// \brief Add a diagnostic with the check's name.
+  DiagnosticBuilder diag(SourceLocation Loc, StringRef Message);
+
----------------
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.

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

================
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);
 
----------------
Why not StringRef?

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:77
@@ -74,2 +76,3 @@
   /// FIXME: Figure out a way to manage ID spaces.
-  DiagnosticBuilder Diag(SourceLocation Loc, StringRef Message);
+  DiagnosticBuilder diag(StringRef CheckName, SourceLocation Loc,
+                         StringRef Message);
----------------
While I agree that this is correct by the coding standards, I wonder if we should really deviate from "Diag()". Has not been done once within Clang's codebase AFAICT..

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


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



More information about the cfe-commits mailing list