alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
A bunch of nits. Otherwise looks good.
Thank you for the fix!
================
Comment at: clang-tidy/ClangTidy.cpp:519
@@ -513,2 +518,3 @@
tooling::TranslationUnitReplacements TUR;
for (const ClangTidyError &Error : Errors)
+ for (const auto &FileAndFixes : Error.Fix)
----------------
Please add braces for the outer loop.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:81
@@ -82,1 +80,3 @@
+ tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling.
----------------
I was kind of intrigued by what `auto` stands for here. I think, `llvm::Error`
is a less mysterious way of storing an error here ;)
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:498
@@ -497,3 +497,3 @@
int Size = 0;
- for (const auto &Replace : Error.Fix)
- Size += Replace.getLength();
+ for (const auto &FileAndReplaces : Error.Fix)
+ for (const auto &Replace : FileAndReplaces.second)
----------------
Please add braces around the outer loop.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:506
@@ -504,15 +505,3 @@
std::map<std::string, std::vector<Event>> FileEvents;
- for (unsigned I = 0; I < Errors.size(); ++I) {
- for (const auto &Replace : Errors[I].Fix) {
- unsigned Begin = Replace.getOffset();
- unsigned End = Begin + Replace.getLength();
- const std::string &FilePath = Replace.getFilePath();
- // FIXME: Handle empty intervals, such as those from insertions.
- if (Begin == End)
- continue;
- FileEvents[FilePath].push_back(
- Event(Begin, End, Event::ET_Begin, I, Sizes[I]));
- FileEvents[FilePath].push_back(
- Event(Begin, End, Event::ET_End, I, Sizes[I]));
- }
- }
+ for (unsigned I = 0; I < Errors.size(); ++I)
+ for (const auto &FileAndReplace : Errors[I].Fix)
----------------
Please add braces for both outer loops.
================
Comment at: unittests/clang-tidy/ClangTidyTest.h:122
@@ -121,7 +121,3 @@
for (const ClangTidyError &Error : Context.getErrors())
- for (const auto &Fix : Error.Fix) {
- auto Err = Fixes.add(Fix);
- // FIXME: better error handling. Keep the behavior for now.
- if (Err) {
- llvm::errs() << llvm::toString(std::move(Err)) << "\n";
- return "";
+ for (const auto &FileAndFixes : Error.Fix)
+ for (const auto &Fix : FileAndFixes.second) {
----------------
Please add braces here as well.
https://reviews.llvm.org/D23257
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
