ioeric updated this revision to Diff 67164.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Addressed reviewers' comments.
https://reviews.llvm.org/D23257
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -119,14 +119,15 @@
DiagConsumer.finish();
tooling::Replacements Fixes;
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) {
+ 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 "";
+ }
}
- }
if (Errors)
*Errors = Context.getErrors();
auto Result = tooling::applyAllReplacements(Code, Fixes);
Index: test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
@@ -0,0 +1,12 @@
+// RUN: cat %S/Inputs/modernize-pass-by-value/header-with-fix.h > %T/pass-by-value-header-with-fix.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %T | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:"
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIXES
+// RUN: FileCheck -input-file=%T/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES
+
+#include "pass-by-value-header-with-fix.h"
+// CHECK-HEADER-FIXES: Foo(std::string s);
+Foo::Foo(const std::string &s) : s(s) {}
+// CHECK-MESSAGES: :9:10: warning: pass by value and use std::move [modernize-pass-by-value]
+// CHECK-FIXES: #include <utility>
+// CHECK-FIXES: Foo::Foo(std::string s) : s(std::move(s)) {}
Index: test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
===================================================================
--- /dev/null
+++ test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
@@ -0,0 +1,6 @@
+#include <string>
+
+struct Foo {
+ Foo(const std::string &s);
+ std::string s;
+};
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -62,7 +62,8 @@
std::string CheckName;
ClangTidyMessage Message;
- tooling::Replacements Fix;
+ // Fixes grouped by file path.
+ llvm::StringMap<tooling::Replacements> Fix;
SmallVector<ClangTidyMessage, 1> Notes;
// A build directory of the diagnostic source file.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -77,8 +77,8 @@
assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() &&
"Only file locations supported in fix-it hints.");
- auto Err =
- Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert));
+ tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
+ auto Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling.
if (Err) {
llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,27 +495,28 @@
std::vector<int> Sizes;
for (const auto &Error : Errors) {
int Size = 0;
- for (const auto &Replace : Error.Fix)
- Size += Replace.getLength();
+ for (const auto &FileAndReplaces : Error.Fix)
+ for (const auto &Replace : FileAndReplaces.second)
+ Size += Replace.getLength();
Sizes.push_back(Size);
}
// Build events from error intervals.
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)
+ for (const auto &Replace : FileAndReplace.second) {
+ 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]));
+ }
std::vector<bool> Apply(Errors.size(), true);
for (auto &FileAndEvents : FileEvents) {
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -126,27 +126,32 @@
}
auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
<< Message.Message << Name;
- for (const tooling::Replacement &Fix : Error.Fix) {
- // Retrieve the source range for applicable fixes. Macro definitions
- // on the command line have locations in a virtual buffer and don't
- // have valid file paths and are therefore not applicable.
- SourceRange Range;
- SourceLocation FixLoc;
- if (Fix.isApplicable()) {
- SmallString<128> FixAbsoluteFilePath = Fix.getFilePath();
- Files.makeAbsolutePath(FixAbsoluteFilePath);
- FixLoc = getLocation(FixAbsoluteFilePath, Fix.getOffset());
- SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength());
- Range = SourceRange(FixLoc, FixEndLoc);
- Diag << FixItHint::CreateReplacement(Range, Fix.getReplacementText());
- }
-
- ++TotalFixes;
- if (ApplyFixes) {
- bool Success = Fix.isApplicable() && Fix.apply(Rewrite);
- if (Success)
- ++AppliedFixes;
- FixLocations.push_back(std::make_pair(FixLoc, Success));
+ for (const auto &FileAndReplacements : Error.Fix) {
+ for (const auto &Replacement : FileAndReplacements.second) {
+ // Retrieve the source range for applicable fixes. Macro definitions
+ // on the command line have locations in a virtual buffer and don't
+ // have valid file paths and are therefore not applicable.
+ SourceRange Range;
+ SourceLocation FixLoc;
+ if (Replacement.isApplicable()) {
+ SmallString<128> FixAbsoluteFilePath = Replacement.getFilePath();
+ Files.makeAbsolutePath(FixAbsoluteFilePath);
+ FixLoc = getLocation(FixAbsoluteFilePath, Replacement.getOffset());
+ SourceLocation FixEndLoc =
+ FixLoc.getLocWithOffset(Replacement.getLength());
+ Range = SourceRange(FixLoc, FixEndLoc);
+ Diag << FixItHint::CreateReplacement(
+ Range, Replacement.getReplacementText());
+ }
+
+ ++TotalFixes;
+ if (ApplyFixes) {
+ bool Success =
+ Replacement.isApplicable() && Replacement.apply(Rewrite);
+ if (Success)
+ ++AppliedFixes;
+ FixLocations.push_back(std::make_pair(FixLoc, Success));
+ }
}
}
}
@@ -512,8 +517,10 @@
raw_ostream &OS) {
tooling::TranslationUnitReplacements TUR;
for (const ClangTidyError &Error : Errors)
- TUR.Replacements.insert(TUR.Replacements.end(), Error.Fix.begin(),
- Error.Fix.end());
+ for (const auto &FileAndFixes : Error.Fix)
+ TUR.Replacements.insert(TUR.Replacements.end(),
+ FileAndFixes.second.begin(),
+ FileAndFixes.second.end());
yaml::Output YAML(OS);
YAML << TUR;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
