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; 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 Fix; SmallVector 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 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> 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 Apply(Errors.size(), true); for (auto &FileAndEvents : FileEvents) { 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,8 @@ +struct S { + S(S&&); + S(const S&); +}; +struct Foo { + Foo(const S &s); + S s; +}; 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(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 +// CHECK-FIXES: Foo::Foo(S s) : s(std::move(s)) {} 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);