Index: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h =================================================================== --- clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h +++ clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h @@ -91,6 +91,11 @@ FileToReplacementsMap &GroupedReplacements, clang::SourceManager &SM); +// FIXME: Remove this function after changing clang-apply-replacements to use +// Replacements class. +bool applyAllReplacements(const std::vector &Replaces, + Rewriter &Rewrite); + /// \brief Apply all replacements in \c GroupedReplacements. /// /// \param[in] GroupedReplacements Deduplicated and conflict free Replacements Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp =================================================================== --- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -122,6 +122,81 @@ } } +// FIXME: Remove this function after changing clang-apply-replacements to use +// Replacements class. +bool applyAllReplacements(const std::vector &Replaces, + Rewriter &Rewrite) { + bool Result = true; + for (std::vector::const_iterator I = Replaces.begin(), + E = Replaces.end(); + I != E; ++I) { + if (I->isApplicable()) { + Result = I->apply(Rewrite) && Result; + } else { + Result = false; + } + } + return Result; +} + + +// FIXME: moved from libToolingCore. remove this when std::vector +// is replaced with tooling::Replacements class. +static void deduplicate(std::vector &Replaces, + std::vector &Conflicts) { + if (Replaces.empty()) + return; + + auto LessNoPath = [](const tooling::Replacement &LHS, + const tooling::Replacement &RHS) { + if (LHS.getOffset() != RHS.getOffset()) + return LHS.getOffset() < RHS.getOffset(); + if (LHS.getLength() != RHS.getLength()) + return LHS.getLength() < RHS.getLength(); + return LHS.getReplacementText() < RHS.getReplacementText(); + }; + + auto EqualNoPath = [](const tooling::Replacement &LHS, + const tooling::Replacement &RHS) { + return LHS.getOffset() == RHS.getOffset() && + LHS.getLength() == RHS.getLength() && + LHS.getReplacementText() == RHS.getReplacementText(); + }; + + // Deduplicate. We don't want to deduplicate based on the path as we assume + // that all replacements refer to the same file (or are symlinks). + std::sort(Replaces.begin(), Replaces.end(), LessNoPath); + Replaces.erase(std::unique(Replaces.begin(), Replaces.end(), EqualNoPath), + Replaces.end()); + + // Detect conflicts + tooling::Range ConflictRange(Replaces.front().getOffset(), + Replaces.front().getLength()); + unsigned ConflictStart = 0; + unsigned ConflictLength = 1; + for (unsigned i = 1; i < Replaces.size(); ++i) { + tooling::Range Current(Replaces[i].getOffset(), Replaces[i].getLength()); + if (ConflictRange.overlapsWith(Current)) { + // Extend conflicted range + ConflictRange = + tooling::Range(ConflictRange.getOffset(), + std::max(ConflictRange.getLength(), + Current.getOffset() + Current.getLength() - + ConflictRange.getOffset())); + ++ConflictLength; + } else { + if (ConflictLength > 1) + Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength)); + ConflictRange = Current; + ConflictStart = i; + ConflictLength = 1; + } + } + + if (ConflictLength > 1) + Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength)); +} + /// \brief Deduplicates and tests for conflicts among the replacements for each /// file in \c Replacements. Any conflicts found are reported. /// @@ -144,7 +219,7 @@ assert(Entry != nullptr && "No file entry!"); std::vector Conflicts; - tooling::deduplicate(FileAndReplacements.second, Conflicts); + deduplicate(FileAndReplacements.second, Conflicts); if (Conflicts.empty()) continue; @@ -197,7 +272,7 @@ // However, until we nail down the design of ReplacementGroups, might as well // leave this as is. for (const auto &FileAndReplacements : GroupedReplacements) { - if (!tooling::applyAllReplacements(FileAndReplacements.second, Rewrites)) + if (!applyAllReplacements(FileAndReplacements.second, Rewrites)) return false; } Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp =================================================================== --- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -108,7 +108,7 @@ Rewriter &Rewrites, std::string &Result) { if (Replacements.empty()) return true; - if (!tooling::applyAllReplacements(Replacements, Rewrites)) + if (!applyAllReplacements(Replacements, Rewrites)) return false; SourceManager &SM = Rewrites.getSourceMgr(); Index: clang-rename/RenamingAction.h =================================================================== --- clang-rename/RenamingAction.h +++ clang-rename/RenamingAction.h @@ -27,17 +27,17 @@ public: RenamingAction(const std::string &NewName, const std::string &PrevName, const std::vector &USRs, - tooling::Replacements &Replaces, bool PrintLocations = false) - : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces), - PrintLocations(PrintLocations) { - } + std::map &FileToReplaces, + bool PrintLocations = false) + : NewName(NewName), PrevName(PrevName), USRs(USRs), + FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {} std::unique_ptr newASTConsumer(); private: const std::string &NewName, &PrevName; const std::vector &USRs; - tooling::Replacements &Replaces; + std::map &FileToReplaces; bool PrintLocations; }; Index: clang-rename/RenamingAction.cpp =================================================================== --- clang-rename/RenamingAction.cpp +++ clang-rename/RenamingAction.cpp @@ -34,14 +34,13 @@ class RenamingASTConsumer : public ASTConsumer { public: - RenamingASTConsumer(const std::string &NewName, - const std::string &PrevName, - const std::vector &USRs, - tooling::Replacements &Replaces, - bool PrintLocations) - : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces), - PrintLocations(PrintLocations) { - } + RenamingASTConsumer( + const std::string &NewName, const std::string &PrevName, + const std::vector &USRs, + std::map &FileToReplaces, + bool PrintLocations) + : NewName(NewName), PrevName(PrevName), USRs(USRs), + FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {} void HandleTranslationUnit(ASTContext &Context) override { const auto &SourceMgr = Context.getSourceManager(); @@ -49,7 +48,8 @@ std::vector NewCandidates; for (const auto &USR : USRs) { - NewCandidates = getLocationsOfUSR(USR, PrevName, Context.getTranslationUnitDecl()); + NewCandidates = + getLocationsOfUSR(USR, PrevName, Context.getTranslationUnitDecl()); RenamingCandidates.insert(RenamingCandidates.end(), NewCandidates.begin(), NewCandidates.end()); NewCandidates.clear(); @@ -62,25 +62,35 @@ errs() << "clang-rename: renamed at: " << SourceMgr.getFilename(Loc) << ":" << FullLoc.getSpellingLineNumber() << ":" << FullLoc.getSpellingColumnNumber() << "\n"; - Replaces.insert(tooling::Replacement(SourceMgr, Loc, PrevNameLen, - NewName)); + // FIXME: better error handling. + auto Replace = + tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName); + auto Err = FileToReplaces[Replace.getFilePath()].add(Replace); + if (Err) + llvm::errs() << "Renaming failed in " << Replace.getFilePath() << "! " + << llvm::toString(std::move(Err)) << "\n"; } else - for (const auto &Loc : RenamingCandidates) - Replaces.insert(tooling::Replacement(SourceMgr, Loc, PrevNameLen, - NewName)); + for (const auto &Loc : RenamingCandidates) { + auto Replace = + tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName); + auto Err = FileToReplaces[Replace.getFilePath()].add(Replace); + if (Err) + llvm::errs() << "Renaming failed in " << Replace.getFilePath() << "! " + << llvm::toString(std::move(Err)) << "\n"; + } } private: const std::string &NewName, &PrevName; const std::vector &USRs; - tooling::Replacements &Replaces; + std::map &FileToReplaces; bool PrintLocations; }; std::unique_ptr RenamingAction::newASTConsumer() { return llvm::make_unique(NewName, PrevName, USRs, - Replaces, PrintLocations); + FileToReplaces, PrintLocations); } } // namespace rename Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -77,7 +77,14 @@ assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() && "Only file locations supported in fix-it hints."); - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + auto Err = + Error.Fix.add(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + // FIXME: better error handling. + if (Err) { + llvm::errs() << "Fix conflicts with existing fix! " + << llvm::toString(std::move(Err)) << "\n"; + } + assert(!Err && "Fix conflicts with existing fix!"); } } Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -314,8 +314,11 @@ return tooling::Replacements(); std::string IncludeName = "#include " + Header.str() + "\n"; // Create replacements for the new header. - clang::tooling::Replacements Insertions = { - tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)}; + clang::tooling::Replacements Insertions; + // FIXME: remove this error comsuption when createInsertHeaderReplacements + // returns Error. + llvm::consumeError( + Insertions.add(tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName))); return formatReplacements( Code, cleanupAroundReplacements(Code, Insertions, Style), Style); Index: tool-template/ToolTemplate.cpp =================================================================== --- tool-template/ToolTemplate.cpp +++ tool-template/ToolTemplate.cpp @@ -53,18 +53,20 @@ namespace { class ToolTemplateCallback : public MatchFinder::MatchCallback { - public: - ToolTemplateCallback(Replacements *Replace) : Replace(Replace) {} +public: + ToolTemplateCallback(std::map *Replace) + : Replace(Replace) {} void run(const MatchFinder::MatchResult &Result) override { - // TODO: This routine will get called for each thing that the matchers find. + // TODO: This routine will get called for each thing that the matchers + // find. // At this point, you can examine the match, and do whatever you want, // including replacing the matched text with other text (void)Replace; // This to prevent an "unused member variable" warning; } - private: - Replacements *Replace; +private: + std::map *Replace; }; } // end anonymous namespace Index: unittests/clang-tidy/ClangTidyTest.h =================================================================== --- unittests/clang-tidy/ClangTidyTest.h +++ unittests/clang-tidy/ClangTidyTest.h @@ -118,7 +118,14 @@ DiagConsumer.finish(); tooling::Replacements Fixes; for (const ClangTidyError &Error : Context.getErrors()) - Fixes.insert(Error.Fix.begin(), Error.Fix.end()); + for (const auto &Fix : Error.Fix) { + auto Err = Fixes.add(Fix); + // FIXME: better error handling. + if (Err) { + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + return ""; + } + } if (Errors) *Errors = Context.getErrors(); return tooling::applyAllReplacements(Code, Fixes);