[PATCH] D26137: [clang-tidy] Add check name to YAML export

Alpha Abdoulaye via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 3 08:38:09 PST 2017


Alpha added a comment.

Thanks for the review!



================
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:106
   void reportDiagnostic(const ClangTidyError &Error) {
-    const ClangTidyMessage &Message = Error.Message;
+    const ClangTidyMessage Message = Error.Message;
     SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
----------------
alexfh wrote:
> alexfh wrote:
> > Why this change?
> Assuming, it was unintentional, changing back to reference.
It is indeed unintentional.


================
Comment at: tools/extra/clang-tidy/ClangTidy.h:245
 /// output stream.
-void exportReplacements(const std::vector<ClangTidyError> &Errors,
+void exportReplacements(const StringRef MainFilePath,
+                        const std::vector<ClangTidyError> &Errors,
----------------
alexfh wrote:
> Top-level const on function parameters in function declarations is useless (it's not a part of the function prototype and it tells nothing to the function users). It might make sense on a function definition, if used consistently (same way as on local variables). However, is not very common in LLVM/Clang.
Ok, this is good to know.


================
Comment at: tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h:36
 
-/// \brief A message from a clang-tidy check.
-///
-/// Note that this is independent of a \c SourceManager.
-struct ClangTidyMessage {
-  ClangTidyMessage(StringRef Message = "");
-  ClangTidyMessage(StringRef Message, const SourceManager &Sources,
-                   SourceLocation Loc);
-  std::string Message;
-  std::string FilePath;
-  unsigned FileOffset;
-};
+typedef clang::tooling::DiagnosticMessage ClangTidyMessage;
 
----------------
alexfh wrote:
> alexfh wrote:
> > Do we actually need this typedef? How much is the old type name used?
> Removed.
I was quite hesitant with this one, wondering if it would be better to keep clang-tidy specific naming in that case.


Repository:
  rL LLVM

https://reviews.llvm.org/D26137





More information about the cfe-commits mailing list