[clang-tools-extra] r278101 - Fix clang-tidy crash when a single fix is applied on multiple files.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 9 00:54:50 PDT 2016
Author: ioeric
Date: Tue Aug 9 02:54:49 2016
New Revision: 278101
URL: http://llvm.org/viewvc/llvm-project?rev=278101&view=rev
Log:
Fix clang-tidy crash when a single fix is applied on multiple files.
Summary:
tooling::Replacements only holds replacements for a single file, so
this patch makes Fix a map from file paths to tooling::Replacements so that it
can be applied on multiple files.
Reviewers: hokein, alexfh
Subscribers: Prazek, cfe-commits
Differential Revision: https://reviews.llvm.org/D23257
Added:
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=278101&r1=278100&r2=278101&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Tue Aug 9 02:54:49 2016
@@ -126,27 +126,32 @@ public:
}
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));
+ }
}
}
}
@@ -511,9 +516,12 @@ void handleErrors(const std::vector<Clan
void exportReplacements(const std::vector<ClangTidyError> &Errors,
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 ClangTidyError &Error : Errors) {
+ for (const auto &FileAndFixes : Error.Fix)
+ TUR.Replacements.insert(TUR.Replacements.end(),
+ FileAndFixes.second.begin(),
+ FileAndFixes.second.end());
+ }
yaml::Output YAML(OS);
YAML << TUR;
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=278101&r1=278100&r2=278101&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Tue Aug 9 02:54:49 2016
@@ -77,8 +77,8 @@ protected:
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);
+ llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling.
if (Err) {
llvm::errs() << "Fix conflicts with existing fix! "
@@ -495,25 +495,29 @@ void ClangTidyDiagnosticConsumer::remove
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 (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]));
+ }
}
}
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=278101&r1=278100&r2=278101&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Tue Aug 9 02:54:49 2016
@@ -62,7 +62,8 @@ struct ClangTidyError {
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.
Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h?rev=278101&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h (added)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-pass-by-value/header-with-fix.h Tue Aug 9 02:54:49 2016
@@ -0,0 +1,8 @@
+struct S {
+ S(S&&);
+ S(const S&);
+};
+struct Foo {
+ Foo(const S &s);
+ S s;
+};
Added: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp?rev=278101&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value-multi-fixes.cpp Tue Aug 9 02:54:49 2016
@@ -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(S s);
+Foo::Foo(const S &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(S s) : s(std::move(s)) {}
Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=278101&r1=278100&r2=278101&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Tue Aug 9 02:54:49 2016
@@ -118,15 +118,18 @@ runCheckOnCode(StringRef Code, std::vect
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 ClangTidyError &Error : Context.getErrors()) {
+ 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);
More information about the cfe-commits
mailing list