alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for resurrecting this patch! A few comments inline.
================
Comment at: include/clang/Tooling/Core/Diagnostic.h:35
+ DiagnosticMessage(llvm::StringRef Message, const SourceManager &Sources,
+ SourceLocation Loc);
+ std::string Message;
----------------
What are the constraints on the location? Should it be a file location or macro
locations are fine too? Please add a (doxygen-style) comment.
================
Comment at: include/clang/Tooling/Core/Diagnostic.h:57
+
+ std::string CheckName;
+ DiagnosticMessage Message;
----------------
`CheckName` is somewhat clang-tidy specific. We need something more generic
here, e.g. `DiagnosticName`, `Category` or somesuch.
Please add a (doxygen-style) comment to this and other fields here.
================
Comment at: include/clang/Tooling/Core/Diagnostic.h:68-71
+ /// A freeform chunk of text to describe the context of the replacements.
+ /// Will be printed, for example, when detecting conflicts during replacement
+ /// deduplication.
+ std::string Context;
----------------
That's too vague. Are you intending to use it only for reporting problems with
replacement deduplication? Should it be in this structure at all?
================
Comment at: include/clang/Tooling/DiagnosticsYaml.h:82
+ }
+ }
+ Io.mapRequired("Diagnostics", Diagnostics);
----------------
nit: Formatting is off.
================
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:578
raw_ostream &OS) {
- TranslationUnitReplacements TUR;
- for (const ClangTidyError &Error : Errors) {
- for (const auto &FileAndFixes : Error.Fix)
- TUR.Replacements.insert(TUR.Replacements.end(),
- FileAndFixes.second.begin(),
- FileAndFixes.second.end());
- }
-
+ TranslationUnitDiagnostics TUD;
+ TUD.Diagnostics.insert(TUD.Diagnostics.end(), Errors.begin(), Errors.end());
----------------
This function neither fills `TUD.MainSourceFile` nor `TUD.Context` (which I'm
not sure even belongs to this structure).
================
Comment at: tools/extra/clang-tidy/ClangTidy.cpp:579
+ TranslationUnitDiagnostics TUD;
+ TUD.Diagnostics.insert(TUD.Diagnostics.end(), Errors.begin(), Errors.end());
yaml::Output YAML(OS);
----------------
I somewhat dislike the implicit object slicing that happens here. I'd prefer to
make this a loop with explicit conversion.
================
Comment at: tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:116
+ bool IsWarningAsError, StringRef BuildDirectory)
+ : clang::tooling::Diagnostic(CheckName, DiagLevel),
+ BuildDirectory(BuildDirectory), IsWarningAsError(IsWarningAsError) {}
----------------
No need for `clang::` here.
Repository:
rL LLVM
https://reviews.llvm.org/D26137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
