ioeric updated this revision to Diff 65685.
ioeric added a comment.
- Merged with origin/master.
https://reviews.llvm.org/D21749
Files:
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
clang-rename/RenamingAction.cpp
clang-rename/RenamingAction.h
clang-rename/tool/ClangRename.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
include-fixer/IncludeFixer.cpp
include-fixer/tool/ClangIncludeFixer.cpp
tool-template/ToolTemplate.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- unittests/clang-tidy/ClangTidyTest.h
+++ 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.
+ if (Err) {
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ return "";
+ }
+ }
if (Errors)
*Errors = Context.getErrors();
auto Result = tooling::applyAllReplacements(Code, Fixes);
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<std::string, Replacements> *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<std::string, Replacements> *Replace;
};
} // end anonymous namespace
Index: include-fixer/tool/ClangIncludeFixer.cpp
===================================================================
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -350,8 +350,7 @@
}
if (!Quiet)
- errs() << "Added #include " << Context.getHeaderInfos().front().Header
- << '\n';
+ llvm::errs() << "Added #include" << Context.getHeaderInfos().front().Header;
// Set up a new source manager for applying the resulting replacements.
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
Index: include-fixer/IncludeFixer.cpp
===================================================================
--- include-fixer/IncludeFixer.cpp
+++ 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;
+ // FIXME: remove this error comsuption when createInsertHeaderReplacements
+ // returns Error.
+ llvm::consumeError(
+ Insertions.add(tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)));
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-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: clang-rename/tool/ClangRename.cpp
===================================================================
--- clang-rename/tool/ClangRename.cpp
+++ 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-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<std::string> &USRs,
- tooling::Replacements &Replaces, bool PrintLocations = false)
- : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces),
- PrintLocations(PrintLocations) {
- }
+ std::map<std::string, tooling::Replacements> &FileToReplaces,
+ bool PrintLocations = false)
+ : NewName(NewName), PrevName(PrevName), USRs(USRs),
+ FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
std::unique_ptr<ASTConsumer> newASTConsumer();
private:
const std::string &NewName, &PrevName;
const std::vector<std::string> &USRs;
- tooling::Replacements &Replaces;
+ std::map<std::string, tooling::Replacements> &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<std::string> &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<std::string> &USRs,
+ std::map<std::string, tooling::Replacements> &FileToReplaces,
+ bool PrintLocations)
+ : NewName(NewName), PrevName(PrevName), USRs(USRs),
+ FileToReplaces(FileToReplaces), PrintLocations(PrintLocations) {}
void HandleTranslationUnit(ASTContext &Context) override {
const auto &SourceMgr = Context.getSourceManager();
@@ -64,21 +63,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<std::string> &USRs;
- tooling::Replacements &Replaces;
+ std::map<std::string, tooling::Replacements> &FileToReplaces;
bool PrintLocations;
};
std::unique_ptr<ASTConsumer> RenamingAction::newASTConsumer() {
return llvm::make_unique<RenamingASTConsumer>(NewName, PrevName, USRs,
- Replaces, PrintLocations);
+ FileToReplaces, PrintLocations);
}
} // namespace rename
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-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<tooling::Replacement> &Replaces,
+ Rewriter &Rewrite) {
+ bool Result = true;
+ for (std::vector<tooling::Replacement>::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<Replacement>
+// is replaced with tooling::Replacements class.
+static void deduplicate(std::vector<tooling::Replacement> &Replaces,
+ std::vector<tooling::Range> &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<tooling::Range> 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/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<tooling::Replacement> &Replaces,
+ Rewriter &Rewrite);
+
/// \brief Apply all replacements in \c GroupedReplacements.
///
/// \param[in] GroupedReplacements Deduplicated and conflict free Replacements
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
