[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