Index: clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h =================================================================== --- clang-tools-extra/trunk/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h +++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp =================================================================== --- clang-tools-extra/trunk/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp =================================================================== --- clang-tools-extra/trunk/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-rename/RenamingAction.h =================================================================== --- clang-tools-extra/trunk/clang-rename/RenamingAction.h +++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-rename/RenamingAction.cpp =================================================================== --- clang-tools-extra/trunk/clang-rename/RenamingAction.cpp +++ clang-tools-extra/trunk/clang-rename/RenamingAction.cpp @@ -34,11 +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(); @@ -58,21 +60,25 @@ << ":" << 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"; } } 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-tools-extra/trunk/clang-rename/tool/ClangRename.cpp =================================================================== --- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp +++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp @@ -146,9 +146,10 @@ // Export replacements. tooling::TranslationUnitReplacements TUR; - const tooling::Replacements &Replacements = Tool.getReplacements(); - TUR.Replacements.insert(TUR.Replacements.end(), Replacements.begin(), - Replacements.end()); + const auto &FileToReplacements = Tool.getReplacements(); + for (const auto &Entry : FileToReplacements) + TUR.Replacements.insert(TUR.Replacements.end(), Entry.second.begin(), + Entry.second.end()); yaml::Output YAML(OS); YAML << TUR; Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp =================================================================== --- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp @@ -352,23 +352,36 @@ std::string IncludeName = "#include " + Context.getHeaderInfos().front().Header + "\n"; // Create replacements for the new header. - clang::tooling::Replacements Insertions = { - tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)}; + clang::tooling::Replacements Insertions; + auto Err = + Insertions.add(tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)); + if (Err) + return std::move(Err); auto CleanReplaces = cleanupAroundReplacements(Code, Insertions, Style); if (!CleanReplaces) return CleanReplaces; + auto Replaces = std::move(*CleanReplaces); if (AddQualifiers) { for (const auto &Info : Context.getQuerySymbolInfos()) { // Ignore the empty range. - if (Info.Range.getLength() > 0) - CleanReplaces->insert({FilePath, Info.Range.getOffset(), - Info.Range.getLength(), - Context.getHeaderInfos().front().QualifiedName}); + if (Info.Range.getLength() > 0) { + auto R = tooling::Replacement( + {FilePath, Info.Range.getOffset(), Info.Range.getLength(), + Context.getHeaderInfos().front().QualifiedName}); + auto Err = Replaces.add(R); + if (Err) { + llvm::consumeError(std::move(Err)); + R = tooling::Replacement( + R.getFilePath(), Replaces.getShiftedCodePosition(R.getOffset()), + R.getLength(), R.getReplacementText()); + Replaces = Replaces.merge(tooling::Replacements(R)); + } + } } } - return formatReplacements(Code, *CleanReplaces, Style); + return formatReplacements(Code, Replaces, Style); } } // namespace include_fixer Index: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp =================================================================== --- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp +++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp @@ -350,8 +350,8 @@ } if (!Quiet) - errs() << "Added #include " << Context.getHeaderInfos().front().Header - << '\n'; + llvm::errs() << "Added #include " << Context.getHeaderInfos().front().Header + << "\n"; // Set up a new source manager for applying the resulting replacements. IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions); Index: clang-tools-extra/trunk/tool-template/ToolTemplate.cpp =================================================================== --- clang-tools-extra/trunk/tool-template/ToolTemplate.cpp +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h =================================================================== --- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h +++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h @@ -119,7 +119,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. 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);