jdemeule updated this revision to Diff 136944.
jdemeule added a comment.
Update patch after reviewer comments.
https://reviews.llvm.org/D43764
Files:
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-apply-replacements/tool/CMakeLists.txt
clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
test/clang-apply-replacements/Inputs/basic/basic.h
test/clang-apply-replacements/Inputs/basic/file1.yaml
test/clang-apply-replacements/Inputs/basic/file2.yaml
test/clang-apply-replacements/Inputs/conflict/expected.txt
unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
unittests/clang-apply-replacements/CMakeLists.txt
unittests/clang-apply-replacements/ReformattingTest.cpp
Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===================================================================
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp ----------------------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector<clang::tooling::Replacement> ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
- unsigned ReplacementLength,
- llvm::StringRef FilePath = "") {
- return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
- unsigned ReplacementLength,
- llvm::StringRef FilePath = "~") {
- ReplacementsVec Replaces;
- Replaces.push_back(
- makeReplacement(Offset, Length, ReplacementLength, FilePath));
- return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream &operator<<(std::ostream &os, const Range &R) {
- return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
- RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
- EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
- ReplacementsVec R;
- R.push_back(makeReplacement(2, 0, 3));
- R.push_back(makeReplacement(5, 2, 4));
- RangeVector Changes = calculateChangedRanges(R);
-
- Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
- EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===================================================================
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
add_extra_unittest(ClangApplyReplacementsTests
ApplyReplacementsTest.cpp
- ReformattingTest.cpp
)
target_link_libraries(ClangApplyReplacementsTests
PRIVATE
clangApplyReplacements
clangBasic
clangToolingCore
+ clangToolingRefactor
)
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===================================================================
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
//===----------------------------------------------------------------------===//
#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
#include "gtest/gtest.h"
using namespace clang::replace;
@@ -41,11 +42,12 @@
IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()), DiagOpts.get());
FileManager Files((FileSystemOptions()));
SourceManager SM(Diagnostics, Files);
+ TUReplacements TURs;
TUDiagnostics TUs =
makeTUDiagnostics("path/to/source.cpp", "diagnostic", {}, {}, "path/to");
- FileToReplacementsMap ReplacementsMap;
+ FileToChangesMap ReplacementsMap;
- EXPECT_TRUE(mergeAndDeduplicate(TUs, ReplacementsMap, SM));
+ EXPECT_TRUE(mergeAndDeduplicate(TURs, TUs, ReplacementsMap, SM));
EXPECT_TRUE(ReplacementsMap.empty());
}
Index: test/clang-apply-replacements/Inputs/conflict/expected.txt
===================================================================
--- test/clang-apply-replacements/Inputs/conflict/expected.txt
+++ test/clang-apply-replacements/Inputs/conflict/expected.txt
@@ -3,9 +3,11 @@
Replace 8:8-8:33 with "auto & i : ints"
Replace 8:8-8:33 with "int & elem : ints"
The following changes conflict:
- Replace 9:5-9:11 with "elem"
Replace 9:5-9:11 with "i"
+ Replace 9:5-9:11 with "elem"
The following changes conflict:
Remove 12:3-12:14
- Insert at 12:12 (int*)
Replace 12:12-12:12 with "nullptr"
+The following changes conflict:
+ Remove 12:3-12:14
+ Insert at 12:12 (int*)
Index: test/clang-apply-replacements/Inputs/basic/file2.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/basic/file2.yaml
+++ test/clang-apply-replacements/Inputs/basic/file2.yaml
@@ -10,4 +10,8 @@
Offset: 148
Length: 0
ReplacementText: 'override '
+ - FilePath: $(path)/../basic/basic.h
+ Offset: 298
+ Length: 1
+ ReplacementText: elem
...
Index: test/clang-apply-replacements/Inputs/basic/file1.yaml
===================================================================
--- test/clang-apply-replacements/Inputs/basic/file1.yaml
+++ test/clang-apply-replacements/Inputs/basic/file1.yaml
@@ -21,5 +21,5 @@
- FilePath: $(path)/../basic/basic.h
Offset: 148
Length: 0
- ReplacementText: 'override '
+ ReplacementText: 'noexcept '
...
Index: test/clang-apply-replacements/Inputs/basic/basic.h
===================================================================
--- test/clang-apply-replacements/Inputs/basic/basic.h
+++ test/clang-apply-replacements/Inputs/basic/basic.h
@@ -10,7 +10,7 @@
class Derived : public Parent {
public:
virtual void func() {}
- // CHECK: virtual void func() override {}
+ // CHECK: virtual void func() noexcept override {}
};
extern void ext(int (&)[5], const Parent &);
Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
===================================================================
--- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -1,4 +1,4 @@
-//===-- ClangApplyReplacementsMain.cpp - Main file for the tool -----------===//
+ //===-- ClangApplyReplacementsMain.cpp - Main file for the tool -----------===//
//
// The LLVM Compiler Infrastructure
//
@@ -86,116 +86,6 @@
OS << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n";
}
-/// \brief Convenience function to get rewritten content for \c Filename from
-/// \c Rewrites.
-///
-/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath().
-/// \post Replacements.empty() -> Result.empty()
-///
-/// \param[in] Replacements Replacements to apply
-/// \param[in] Rewrites Rewriter to use to apply replacements.
-/// \param[out] Result Contents of the file after applying replacements if
-/// replacements were provided.
-///
-/// \returns \parblock
-/// \li true if all replacements were applied successfully.
-/// \li false if at least one replacement failed to apply.
-static bool
-getRewrittenData(const std::vector<tooling::Replacement> &Replacements,
- Rewriter &Rewrites, std::string &Result) {
- if (Replacements.empty())
- return true;
-
- if (!applyAllReplacements(Replacements, Rewrites))
- return false;
-
- SourceManager &SM = Rewrites.getSourceMgr();
- FileManager &Files = SM.getFileManager();
-
- StringRef FileName = Replacements.begin()->getFilePath();
- const clang::FileEntry *Entry = Files.getFile(FileName);
- assert(Entry && "Expected an existing file");
- FileID ID = SM.translateFile(Entry);
- assert(ID.isValid() && "Expected a valid FileID");
- const RewriteBuffer *Buffer = Rewrites.getRewriteBufferFor(ID);
- Result = std::string(Buffer->begin(), Buffer->end());
-
- return true;
-}
-
-/// \brief Apply \c Replacements and return the new file contents.
-///
-/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath().
-/// \post Replacements.empty() -> Result.empty()
-///
-/// \param[in] Replacements Replacements to apply.
-/// \param[out] Result Contents of the file after applying replacements if
-/// replacements were provided.
-/// \param[in] Diagnostics For diagnostic output.
-///
-/// \returns \parblock
-/// \li true if all replacements applied successfully.
-/// \li false if at least one replacement failed to apply.
-static bool
-applyReplacements(const std::vector<tooling::Replacement> &Replacements,
- std::string &Result, DiagnosticsEngine &Diagnostics) {
- FileManager Files((FileSystemOptions()));
- SourceManager SM(Diagnostics, Files);
- Rewriter Rewrites(SM, LangOptions());
-
- return getRewrittenData(Replacements, Rewrites, Result);
-}
-
-/// \brief Apply code formatting to all places where replacements were made.
-///
-/// \pre !Replacements.empty().
-/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath().
-/// \pre Replacements[i].getOffset() <= Replacements[i+1].getOffset().
-///
-/// \param[in] Replacements Replacements that were made to the file. Provided
-/// to indicate where changes were made.
-/// \param[in] FileData The contents of the file \b after \c Replacements have
-/// been applied.
-/// \param[out] FormattedFileData The contents of the file after reformatting.
-/// \param[in] FormatStyle Style to apply.
-/// \param[in] Diagnostics For diagnostic output.
-///
-/// \returns \parblock
-/// \li true if reformatting replacements were all successfully
-/// applied.
-/// \li false if at least one reformatting replacement failed to apply.
-static bool
-applyFormatting(const std::vector<tooling::Replacement> &Replacements,
- const StringRef FileData, std::string &FormattedFileData,
- const format::FormatStyle &FormatStyle,
- DiagnosticsEngine &Diagnostics) {
- assert(!Replacements.empty() && "Need at least one replacement");
-
- RangeVector Ranges = calculateChangedRanges(Replacements);
-
- StringRef FileName = Replacements.begin()->getFilePath();
- tooling::Replacements R =
- format::reformat(FormatStyle, FileData, Ranges, FileName);
-
- // FIXME: Remove this copy when tooling::Replacements is implemented as a
- // vector instead of a set.
- std::vector<tooling::Replacement> FormattingReplacements;
- std::copy(R.begin(), R.end(), back_inserter(FormattingReplacements));
-
- if (FormattingReplacements.empty()) {
- FormattedFileData = FileData;
- return true;
- }
-
- FileManager Files((FileSystemOptions()));
- SourceManager SM(Diagnostics, Files);
- SM.overrideFileContents(Files.getFile(FileName),
- llvm::MemoryBuffer::getMemBufferCopy(FileData));
- Rewriter Rewrites(SM, LangOptions());
-
- return getRewrittenData(FormattingReplacements, Rewrites, FormattedFileData);
-}
-
int main(int argc, char **argv) {
cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories));
@@ -244,46 +134,19 @@
FileManager Files((FileSystemOptions()));
SourceManager SM(Diagnostics, Files);
- FileToReplacementsMap GroupedReplacements;
- if (!mergeAndDeduplicate(TURs, GroupedReplacements, SM))
- return 1;
- if (!mergeAndDeduplicate(TUDs, GroupedReplacements, SM))
+ FileToChangesMap Changes;
+ if (!mergeAndDeduplicate(TURs, TUDs, Changes, SM))
return 1;
- Rewriter ReplacementsRewriter(SM, LangOptions());
-
- for (const auto &FileAndReplacements : GroupedReplacements) {
- // This shouldn't happen but if a file somehow has no replacements skip to
- // next file.
- if (FileAndReplacements.second.empty())
- continue;
-
- std::string NewFileData;
- StringRef FileName = FileAndReplacements.first->getName();
- if (!applyReplacements(FileAndReplacements.second, NewFileData,
- Diagnostics)) {
- errs() << "Failed to apply replacements to " << FileName << "\n";
- continue;
- }
-
- // Apply formatting if requested.
- if (DoFormat &&
- !applyFormatting(FileAndReplacements.second, NewFileData, NewFileData,
- FormatStyle, Diagnostics)) {
- errs() << "Failed to apply reformatting replacements for " << FileName
- << "\n";
- continue;
- }
-
- // Write new file to disk
- std::error_code EC;
- llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_None);
- if (EC) {
- llvm::errs() << "Could not open " << FileName << " for writing\n";
- continue;
- }
+ tooling::ApplyChangesSpec Spec;
+ Spec.Cleanup = true;
+ Spec.Style = FormatStyle;
+ Spec.Format = DoFormat ? tooling::ApplyChangesSpec::kAll
+ : tooling::ApplyChangesSpec::kNone;
- FileStream << NewFileData;
+ for (const auto &FileChange : Changes) {
+ const FileEntry* Entry = FileChange.first;
+ applyChanges(Entry->getName(), FileChange.second, SM, Spec);
}
return 0;
Index: clang-apply-replacements/tool/CMakeLists.txt
===================================================================
--- clang-apply-replacements/tool/CMakeLists.txt
+++ clang-apply-replacements/tool/CMakeLists.txt
@@ -12,6 +12,7 @@
clangFormat
clangRewrite
clangToolingCore
+ clangToolingRefactor
)
install(TARGETS clang-apply-replacements
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -80,10 +80,9 @@
return ErrorCode;
}
-std::error_code
-collectReplacementsFromDirectory(const llvm::StringRef Directory,
- TUDiagnostics &TUs, TUReplacementFiles &TUFiles,
- clang::DiagnosticsEngine &Diagnostics) {
+std::error_code collectReplacementsFromDirectory(
+ const llvm::StringRef Directory, TUDiagnostics &TUs,
+ TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) {
using namespace llvm::sys::fs;
using namespace llvm::sys::path;
@@ -125,23 +124,21 @@
return ErrorCode;
}
-/// \brief Dumps information for a sequence of conflicting Replacements.
+/// \brief Dumps information for a conflicting Replacement.
///
-/// \param[in] File FileEntry for the file the conflicting Replacements are
+/// \param[in] File FileEntry for the file the conflicting Replacement are
/// for.
-/// \param[in] ConflictingReplacements List of conflicting Replacements.
+/// \param[in] Conflict A conflicting Replacement.
/// \param[in] SM SourceManager used for reporting.
-static void reportConflict(
- const FileEntry *File,
- const llvm::ArrayRef<clang::tooling::Replacement> ConflictingReplacements,
- SourceManager &SM) {
+static void reportConflict(const FileEntry *File,
+ const tooling::ReplacementError &Conflict,
+ SourceManager &SM) {
FileID FID = SM.translateFile(File);
if (FID.isInvalid())
FID = SM.createFileID(File, SourceLocation(), SrcMgr::C_User);
-
// FIXME: Output something a little more user-friendly (e.g. unified diff?)
- errs() << "The following changes conflict:\n";
- for (const tooling::Replacement &R : ConflictingReplacements) {
+
+ auto PrintConflit = [&](const tooling::Replacement &R) {
if (R.getLength() == 0) {
errs() << " Insert at " << SM.getLineNumber(FID, R.getOffset()) << ":"
<< SM.getColumnNumber(FID, R.getOffset()) << " "
@@ -162,221 +159,244 @@
else
errs() << " with \"" << R.getReplacementText() << "\"\n";
}
- }
-}
-
-// 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 (auto 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));
+ errs() << "The following changes conflict:\n";
+ if (auto R = Conflict.getExistingReplacement())
+ PrintConflit(*R);
+ if (auto R = Conflict.getNewReplacement())
+ PrintConflit(*R);
}
-/// \brief Deduplicates and tests for conflicts among the replacements for each
-/// file in \c Replacements. Any conflicts found are reported.
-///
-/// \post Replacements[i].getOffset() <= Replacements[i+1].getOffset().
-///
-/// \param[in,out] Replacements Container of all replacements grouped by file
-/// to be deduplicated and checked for conflicts.
-/// \param[in] SM SourceManager required for conflict reporting.
+/// \brief Dumps information for all conflicting replacements per file.
///
-/// \returns \parblock
-/// \li true if conflicts were detected
-/// \li false if no conflicts were detected
-static bool deduplicateAndDetectConflicts(FileToReplacementsMap &Replacements,
- SourceManager &SM) {
- bool conflictsFound = false;
-
- for (auto &FileAndReplacements : Replacements) {
- const FileEntry *Entry = FileAndReplacements.first;
- auto &Replacements = FileAndReplacements.second;
- assert(Entry != nullptr && "No file entry!");
-
- std::vector<tooling::Range> Conflicts;
- deduplicate(FileAndReplacements.second, Conflicts);
-
- if (Conflicts.empty())
- continue;
-
- conflictsFound = true;
-
+/// \param[in] Conflicts Map where FileEntry is the file the conflicting
+/// Replacement are for.
+/// \param[in] SM SourceManager used for reporting.
+static void reportConflicts(
+ const llvm::DenseMap<const FileEntry *,
+ std::vector<tooling::ReplacementError>> &Conflicts,
+ SourceManager &SM) {
+ for (const auto &FileConflicts : Conflicts) {
+ const FileEntry *Entry = FileConflicts.first;
errs() << "There are conflicting changes to " << Entry->getName() << ":\n";
-
- for (const tooling::Range &Conflict : Conflicts) {
- auto ConflictingReplacements = llvm::makeArrayRef(
- &Replacements[Conflict.getOffset()], Conflict.getLength());
- reportConflict(Entry, ConflictingReplacements, SM);
- }
+ for (const auto &Conflict : FileConflicts.second)
+ reportConflict(Entry, Conflict, SM);
}
-
- return conflictsFound;
}
-bool mergeAndDeduplicate(const TUReplacements &TUs,
- FileToReplacementsMap &GroupedReplacements,
- clang::SourceManager &SM) {
-
- // Group all replacements by target file.
+static llvm::DenseMap<const FileEntry *, std::vector<tooling::Replacement>>
+groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
+ FileToChangesMap &FileChanges, clang::SourceManager &SM) {
std::set<StringRef> Warned;
+ llvm::DenseMap<const FileEntry *, std::vector<tooling::Replacement>>
+ GroupedReplacements;
+
for (const auto &TU : TUs) {
for (const tooling::Replacement &R : TU.Replacements) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
- if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) {
+ if (const FileEntry *Entry =
+ SM.getFileManager().getFile(R.getFilePath())) {
GroupedReplacements[Entry].push_back(R);
} else if (Warned.insert(R.getFilePath()).second) {
- errs() << "Described file '" << R.getFilePath()
- << "' doesn't exist. Ignoring...\n";
+ errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
}
}
}
- // Ask clang to deduplicate and report conflicts.
- return !deduplicateAndDetectConflicts(GroupedReplacements, SM);
-}
-
-bool mergeAndDeduplicate(const TUDiagnostics &TUs,
- FileToReplacementsMap &GroupedReplacements,
- clang::SourceManager &SM) {
-
- // Group all replacements by target file.
- std::set<StringRef> Warned;
- for (const auto &TU : TUs) {
+ for (const auto &TU : TUDs) {
for (const auto &D : TU.Diagnostics) {
for (const auto &Fix : D.Fix) {
for (const tooling::Replacement &R : Fix.second) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
- if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) {
+ if (const FileEntry *Entry =
+ SM.getFileManager().getFile(R.getFilePath())) {
GroupedReplacements[Entry].push_back(R);
} else if (Warned.insert(R.getFilePath()).second) {
- errs() << "Described file '" << R.getFilePath()
- << "' doesn't exist. Ignoring...\n";
+ errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
}
}
}
}
}
-
- // Ask clang to deduplicate and report conflicts.
- return !deduplicateAndDetectConflicts(GroupedReplacements, SM);
+ return GroupedReplacements;
}
-bool applyReplacements(const FileToReplacementsMap &GroupedReplacements,
- clang::Rewriter &Rewrites) {
+namespace {
- // Apply all changes
- //
- // FIXME: No longer certain GroupedReplacements is really the best kind of
- // data structure for applying replacements. Rewriter certainly doesn't care.
- // However, until we nail down the design of ReplacementGroups, might as well
- // leave this as is.
- for (const auto &FileAndReplacements : GroupedReplacements) {
- if (!applyAllReplacements(FileAndReplacements.second, Rewrites))
- return false;
+// Carries extra error information when converting Replacements to AtomicChange.
+class ConflictError : public llvm::ErrorInfo<ConflictError> {
+public:
+ ConflictError(const FileEntry *Entry,
+ std::vector<tooling::ReplacementError> Conflicts)
+ : Entry(Entry), Conflicts(Conflicts) {}
+
+ void log(raw_ostream &OS) const override {}
+
+ // Corresponding FileEntry.
+ const FileEntry *getEntry() const { return Entry; }
+
+ // All the conflict in ReplacementError form.
+ const std::vector<tooling::ReplacementError> &getConflicts() const {
+ return Conflicts;
}
- return true;
-}
+ static char ID;
-RangeVector calculateChangedRanges(
- const std::vector<clang::tooling::Replacement> &Replaces) {
- RangeVector ChangedRanges;
-
- // Generate the new ranges from the replacements.
- int Shift = 0;
- for (const tooling::Replacement &R : Replaces) {
- unsigned Offset = R.getOffset() + Shift;
- unsigned Length = R.getReplacementText().size();
- Shift += Length - R.getLength();
- ChangedRanges.push_back(tooling::Range(Offset, Length));
+private:
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
}
- return ChangedRanges;
-}
+ const FileEntry *Entry;
+ std::vector<tooling::ReplacementError> Conflicts;
+};
+
+char ConflictError::ID = 0;
+
+// Store replacements for a given file and deduplicate them using one unique
+// AtomicChange. Final changes will be retrive as a set of AtomicChange or an
+// error containing all conflicts.
+class ReplacementsToAtomicChanges {
+public:
+ explicit ReplacementsToAtomicChanges(clang::SourceManager &SM,
+ const FileEntry *Entry)
+ : SM(&SM), Entry(Entry),
+ BeginLoc(SM.getLocForStartOfFile(
+ SM.getOrCreateFileID(Entry, SrcMgr::C_User))),
+ AllChanges(Entry->getName(), Entry->getName()) {}
+
+ // Store a replacement. Deduplication will be handle at this level.
+ // A replacement with a length of 0 is considered as an insertion.
+ void add(const tooling::Replacement &R) {
+ if (R.getLength() == 0)
+ addInsertion(R);
+ else
+ addReplacement(R);
+ }
+
+ // Retrive all the stored replacement as set of AtomicChange.
+ // Is a conflict has been occured, Expected will contain the resulting error
+ // in form of ConflictError.
+ llvm::Expected<std::vector<tooling::AtomicChange>> getChanges() const {
+ if (!Conflicts.empty())
+ return llvm::make_error<ConflictError>(Entry, Conflicts);
+
+ std::vector<tooling::AtomicChange> Changes;
+ for (const auto &R : AllChanges.getReplacements()) {
+ tooling::AtomicChange Change(Entry->getName(), Entry->getName());
+ if (llvm::Error Err =
+ Change.replace(*SM, BeginLoc.getLocWithOffset(R.getOffset()),
+ R.getLength(), R.getReplacementText()))
+ llvm_unreachable(
+ "Failed to add replacement when converting to AtomicChange.");
+ Changes.push_back(Change);
+ }
+ return Changes;
+ }
+
+private:
+ void addReplacement(const tooling::Replacement &R) {
+ if (llvm::Error Error =
+ AllChanges.replace(*SM, BeginLoc.getLocWithOffset(R.getOffset()),
+ R.getLength(), R.getReplacementText())) {
+ // Store conflicting replacement for errors reporting per file.
+ handleAllErrors(std::move(Error),
+ [&](const tooling::ReplacementError &Err) {
+ Conflicts.push_back(Err);
+ });
+ }
+ }
+
+ void addInsertion(const tooling::Replacement &R) {
+ if (llvm::Error Error =
+ AllChanges.insert(*SM, BeginLoc.getLocWithOffset(R.getOffset()),
+ R.getReplacementText())) {
+ // Store conflicting replacement for errors reporting per file.
+ handleAllErrors(std::move(Error),
+ [&](const tooling::ReplacementError &Err) {
+ Conflicts.push_back(Err);
+ });
+ }
+ }
+
+ clang::SourceManager *SM;
+ const FileEntry *Entry;
+ const SourceLocation BeginLoc;
+ tooling::AtomicChange AllChanges;
+ std::vector<tooling::ReplacementError> Conflicts;
+};
+
+} // namespace
-bool writeFiles(const clang::Rewriter &Rewrites) {
+bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs,
+ FileToChangesMap &FileChanges,
+ clang::SourceManager &SM) {
+
+ llvm::DenseMap<const FileEntry *, std::vector<tooling::Replacement>>
+ GroupedReplacements = groupReplacements(TUs, TUDs, FileChanges, SM);
- for (auto BufferI = Rewrites.buffer_begin(), BufferE = Rewrites.buffer_end();
- BufferI != BufferE; ++BufferI) {
- StringRef FileName =
- Rewrites.getSourceMgr().getFileEntryForID(BufferI->first)->getName();
+ llvm::DenseMap<const FileEntry *, std::vector<tooling::ReplacementError>>
+ Conflicts;
- std::error_code EC;
- llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_Text);
- if (EC) {
- errs() << "Warning: Could not write to " << EC.message() << "\n";
+ // To keep current clang-apply-replacement behavior, report conflicts per file
+ // and skip file containing conflicts, all replacements are store into 1 big
+ // AtomicChange.
+ // Once done and if no conflicts have been occured, a set of AtomicChange is
+ // generated else conflicts are stored for reporting.
+ for (const auto &FileAndReplacements : GroupedReplacements) {
+ const FileEntry *Entry = FileAndReplacements.first;
+ ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+ for (const auto &R : FileAndReplacements.second)
+ DeduplicatedChanges.add(R);
+
+ llvm::Expected<std::vector<tooling::AtomicChange>> Changes =
+ DeduplicatedChanges.getChanges();
+ if (!Changes) {
+ handleAllErrors(Changes.takeError(), [&](const ConflictError &Error) {
+ Conflicts.insert(
+ std::make_pair(Error.getEntry(), Error.getConflicts()));
+ });
continue;
}
- BufferI->second.write(FileStream);
+ FileChanges.insert(std::make_pair(Entry, *Changes));
}
+ reportConflicts(Conflicts, SM);
+ return Conflicts.empty();
+}
+bool applyChanges(StringRef File,
+ const std::vector<tooling::AtomicChange> &Changes,
+ const SourceManager &SM,
+ const tooling::ApplyChangesSpec &Spec) {
+
+ llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
+ SM.getFileManager().getBufferForFile(File);
+ if (!Buffer) {
+ errs() << "Can't get buffer for file " << File << ": "
+ << Buffer.getError().message() << "\n";
+ return false;
+ }
+
+ StringRef Code = Buffer.get()->getBuffer();
+ llvm::Expected<std::string> NewCode =
+ tooling::applyAtomicChanges(File, Code, Changes, Spec);
+ if (!NewCode) {
+ errs() << llvm::toString(NewCode.takeError()) << "\n";
+ return false;
+ }
+ // Write new file to disk
+ std::error_code EC;
+ llvm::raw_fd_ostream FileStream(File, EC, llvm::sys::fs::F_None);
+ if (EC) {
+ errs() << "Could not open " << File << " for writing\n";
+ return false;
+ }
+ FileStream << *NewCode;
return true;
}
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
@@ -18,6 +18,7 @@
#include "clang/Tooling/Core/Diagnostic.h"
#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include <string>
@@ -29,15 +30,8 @@
class DiagnosticsEngine;
class Rewriter;
-namespace format {
-struct FormatStyle;
-} // end namespace format
-
namespace replace {
-/// \brief Collection of source ranges.
-typedef std::vector<clang::tooling::Range> RangeVector;
-
/// \brief Collection of TranslationUnitReplacements.
typedef std::vector<clang::tooling::TranslationUnitReplacements> TUReplacements;
@@ -47,10 +41,10 @@
/// \brief Collection of TranslationUniDiagnostics.
typedef std::vector<clang::tooling::TranslationUnitDiagnostics> TUDiagnostics;
-/// \brief Map mapping file name to Replacements targeting that file.
+/// \brief Map mapping file name to a set of AtomicChange targeting that file.
typedef llvm::DenseMap<const clang::FileEntry *,
- std::vector<clang::tooling::Replacement>>
- FileToReplacementsMap;
+ std::vector<tooling::AtomicChange>>
+ FileToChangesMap;
/// \brief Recursively descends through a directory structure rooted at \p
/// Directory and attempts to deserialize *.yaml files as
@@ -77,65 +71,36 @@
const llvm::StringRef Directory, TUDiagnostics &TUs,
TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics);
-/// \brief Deduplicate, check for conflicts, and apply all Replacements stored
-/// in \c TUs. If conflicts occur, no Replacements are applied.
-///
-/// \post For all (key,value) in GroupedReplacements, value[i].getOffset() <=
-/// value[i+1].getOffset().
+/// \brief Deduplicate, check for conflicts, and convert all Replacements stored
+/// in \c TUs to AtomicChange. If conflicts occur, no Replacements are applied.
///
/// \param[in] TUs Collection of TranslationUnitReplacements or
-/// TranslationUnitDiagnostics to merge,
-/// deduplicate, and test for conflicts.
-/// \param[out] GroupedReplacements Container grouping all Replacements by the
+/// TranslationUnitDiagnostics to merge, deduplicate, and test for conflicts.
+/// \param[out] FileChanges Container grouping all changes by the
/// file they target.
/// \param[in] SM SourceManager required for conflict reporting.
///
/// \returns \parblock
-/// \li true If all changes were applied successfully.
+/// \li true If all changes were converted successfully.
/// \li false If there were conflicts.
-bool mergeAndDeduplicate(const TUReplacements &TUs,
- FileToReplacementsMap &GroupedReplacements,
+bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs,
+ FileToChangesMap &FileChanges,
clang::SourceManager &SM);
-bool mergeAndDeduplicate(const TUDiagnostics &TUs,
- 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.
+/// \brief Apply \c AtomicChange on File and rewrite it.
///
-/// \param[in] GroupedReplacements Deduplicated and conflict free Replacements
-/// to apply.
-/// \param[out] Rewrites The results of applying replacements will be applied
-/// to this Rewriter.
+/// \param[in] File Path of the file where to apply AtomicChange.
+/// \param[in] Changes to apply.
+/// \param[in] SM Source Manager where to read the file.
+/// \param[in] Spec For code cleanup and formatting.
///
/// \returns \parblock
-/// \li true If all changes were applied successfully.
-/// \li false If a replacement failed to apply.
-bool applyReplacements(const FileToReplacementsMap &GroupedReplacements,
- clang::Rewriter &Rewrites);
-
-/// \brief Given a collection of Replacements for a single file, produces a list
-/// of source ranges that enclose those Replacements.
-///
-/// \pre Replacements[i].getOffset() <= Replacements[i+1].getOffset().
-///
-/// \param[in] Replacements Replacements from a single file.
-///
-/// \returns Collection of source ranges that enclose all given Replacements.
-/// One range is created for each replacement.
-RangeVector calculateChangedRanges(
- const std::vector<clang::tooling::Replacement> &Replacements);
-
-/// \brief Write the contents of \c FileContents to disk. Keys of the map are
-/// filenames and values are the new contents for those files.
-///
-/// \param[in] Rewrites Rewriter containing written files to write to disk.
-bool writeFiles(const clang::Rewriter &Rewrites);
+/// \li true if all changes applied successfully.
+/// \li false if something wrong occurs.
+bool applyChanges(StringRef File,
+ const std::vector<tooling::AtomicChange> &Changes,
+ const SourceManager &SM,
+ const tooling::ApplyChangesSpec &Spec);
/// \brief Delete the replacement files.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
