ioeric created this revision.
ioeric added reviewers: klimek, djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
return llvm::Expected<> to carry error status and error information.
This is the first step towards introducing "Error" into tooling::Replacements.
http://reviews.llvm.org/D21601
Files:
include/clang/Tooling/Core/Replacement.h
lib/Format/Format.cpp
lib/Tooling/Core/Replacement.cpp
tools/clang-format/ClangFormat.cpp
unittests/Format/CleanupTest.cpp
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestJS.cpp
unittests/Format/FormatTestJava.cpp
unittests/Format/FormatTestProto.cpp
unittests/Format/FormatTestSelective.cpp
unittests/Format/SortImportsTestJS.cpp
unittests/Format/SortIncludesTest.cpp
unittests/Tooling/RefactoringTest.cpp
unittests/Tooling/RewriterTest.cpp
Index: unittests/Tooling/RewriterTest.cpp
===================================================================
--- unittests/Tooling/RewriterTest.cpp
+++ unittests/Tooling/RewriterTest.cpp
@@ -41,8 +41,9 @@
Replacements Replaces;
Replaces.insert(Replacement("<file>", 6, 6, ""));
Replaces.insert(Replacement("<file>", 6, 0, "replaced\n"));
- EXPECT_EQ("line1\nreplaced\nline3\nline4",
- applyAllReplacements("line1\nline2\nline3\nline4", Replaces));
+ auto Rewritten = applyAllReplacements("line1\nline2\nline3\nline4", Replaces);
+ EXPECT_TRUE((bool)Rewritten);
+ EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten);
}
} // end namespace
Index: unittests/Tooling/RefactoringTest.cpp
===================================================================
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -640,27 +640,32 @@
StringRef Result, const Replacements &First,
const Replacements &Second) {
// These are mainly to verify the test itself and make it easier to read.
- std::string AfterFirst = applyAllReplacements(Code, First);
- std::string InSequenceRewrite = applyAllReplacements(AfterFirst, Second);
- EXPECT_EQ(Intermediate, AfterFirst);
- EXPECT_EQ(Result, InSequenceRewrite);
+ auto AfterFirst = applyAllReplacements(Code, First);
+ EXPECT_TRUE((bool)AfterFirst);
+ auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second);
+ EXPECT_TRUE((bool)InSequenceRewrite);
+ EXPECT_EQ(Intermediate, *AfterFirst);
+ EXPECT_EQ(Result, *InSequenceRewrite);
tooling::Replacements Merged = mergeReplacements(First, Second);
- std::string MergedRewrite = applyAllReplacements(Code, Merged);
- EXPECT_EQ(InSequenceRewrite, MergedRewrite);
- if (InSequenceRewrite != MergedRewrite)
+ auto MergedRewrite = applyAllReplacements(Code, Merged);
+ EXPECT_TRUE((bool)MergedRewrite);
+ EXPECT_EQ(*InSequenceRewrite, *MergedRewrite);
+ if (*InSequenceRewrite != *MergedRewrite)
for (tooling::Replacement M : Merged)
llvm::errs() << M.getOffset() << " " << M.getLength() << " "
<< M.getReplacementText() << "\n";
}
void mergeAndTestRewrite(StringRef Code, const Replacements &First,
const Replacements &Second) {
- std::string InSequenceRewrite =
- applyAllReplacements(applyAllReplacements(Code, First), Second);
+ auto AfterFirst = applyAllReplacements(Code, First);
+ EXPECT_TRUE((bool)AfterFirst);
+ auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second);
tooling::Replacements Merged = mergeReplacements(First, Second);
- std::string MergedRewrite = applyAllReplacements(Code, Merged);
- EXPECT_EQ(InSequenceRewrite, MergedRewrite);
- if (InSequenceRewrite != MergedRewrite)
+ auto MergedRewrite = applyAllReplacements(Code, Merged);
+ EXPECT_TRUE((bool)MergedRewrite);
+ EXPECT_EQ(*InSequenceRewrite, *MergedRewrite);
+ if (*InSequenceRewrite != *MergedRewrite)
for (tooling::Replacement M : Merged)
llvm::errs() << M.getOffset() << " " << M.getLength() << " "
<< M.getReplacementText() << "\n";
Index: unittests/Format/SortIncludesTest.cpp
===================================================================
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -26,10 +26,13 @@
std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
auto Ranges = GetCodeRange(Code);
- std::string Sorted =
+ auto Sorted =
applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
- return applyAllReplacements(Sorted,
- reformat(Style, Sorted, Ranges, FileName));
+ EXPECT_TRUE((bool)Sorted);
+ auto Result = applyAllReplacements(
+ *Sorted, reformat(Style, *Sorted, Ranges, FileName));
+ EXPECT_TRUE((bool)Result);
+ return *Result;
}
unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {
Index: unittests/Format/SortImportsTestJS.cpp
===================================================================
--- unittests/Format/SortImportsTestJS.cpp
+++ unittests/Format/SortImportsTestJS.cpp
@@ -25,10 +25,13 @@
if (Length == 0U)
Length = Code.size() - Offset;
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
- std::string Sorted =
+ auto Sorted =
applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
- return applyAllReplacements(Sorted,
- reformat(Style, Sorted, Ranges, FileName));
+ EXPECT_TRUE((bool)Sorted);
+ auto Formatted = applyAllReplacements(
+ *Sorted, reformat(Style, *Sorted, Ranges, FileName));
+ EXPECT_TRUE((bool)Formatted);
+ return *Formatted;
}
void verifySort(llvm::StringRef Expected, llvm::StringRef Code,
Index: unittests/Format/FormatTestSelective.cpp
===================================================================
--- unittests/Format/FormatTestSelective.cpp
+++ unittests/Format/FormatTestSelective.cpp
@@ -28,10 +28,10 @@
tooling::Replacements Replaces =
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- DEBUG(llvm::errs() << "\n" << Result << "\n\n");
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
}
FormatStyle Style = getLLVMStyle();
Index: unittests/Format/FormatTestProto.cpp
===================================================================
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -25,10 +25,10 @@
DEBUG(llvm::errs() << Code << "\n\n");
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
tooling::Replacements Replaces = reformat(Style, Code, Ranges);
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- DEBUG(llvm::errs() << "\n" << Result << "\n\n");
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
}
static std::string format(llvm::StringRef Code) {
Index: unittests/Format/FormatTestJava.cpp
===================================================================
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -25,10 +25,10 @@
DEBUG(llvm::errs() << Code << "\n\n");
std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
tooling::Replacements Replaces = reformat(Style, Code, Ranges);
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- DEBUG(llvm::errs() << "\n" << Result << "\n\n");
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
}
static std::string
Index: unittests/Format/FormatTestJS.cpp
===================================================================
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -28,10 +28,10 @@
tooling::Replacements Replaces =
reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
EXPECT_FALSE(IncompleteFormat);
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- DEBUG(llvm::errs() << "\n" << Result << "\n\n");
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
}
static std::string format(
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -47,10 +47,10 @@
EXPECT_EQ(ExpectedIncompleteFormat, IncompleteFormat) << Code << "\n\n";
}
ReplacementCount = Replaces.size();
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- DEBUG(llvm::errs() << "\n" << Result << "\n\n");
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+ return *Result;
}
FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
@@ -11553,8 +11553,10 @@
format::FormatStyle Style = format::getLLVMStyle();
Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
- EXPECT_EQ(Expected, applyAllReplacements(
- Code, formatReplacements(Code, Replaces, Style)));
+ auto Result =
+ applyAllReplacements(Code, formatReplacements(Code, Replaces, Style));
+ EXPECT_TRUE((bool)Result);
+ EXPECT_EQ(Expected, *Result);
}
TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
@@ -11578,8 +11580,10 @@
format::FormatStyle Style = format::getLLVMStyle();
Style.SortIncludes = true;
- auto FinalReplaces = formatReplacements(Code, Replaces, Style);
- EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
+ auto Result =
+ applyAllReplacements(Code, formatReplacements(Code, Replaces, Style));
+ EXPECT_TRUE((bool)Result);
+ EXPECT_EQ(Expected, *Result);
}
} // end namespace
Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -25,9 +25,9 @@
const FormatStyle &Style = getLLVMStyle()) {
tooling::Replacements Replaces = format::cleanup(Style, Code, Ranges);
- std::string Result = applyAllReplacements(Code, Replaces);
- EXPECT_NE("", Result);
- return Result;
+ auto Result = applyAllReplacements(Code, Replaces);
+ EXPECT_TRUE((bool)Result);
+ return *Result;
}
};
@@ -254,16 +254,20 @@
inline std::string apply(StringRef Code,
const tooling::Replacements Replaces) {
- return applyAllReplacements(
+ auto Result = applyAllReplacements(
Code, cleanupAroundReplacements(Code, Replaces, Style));
+ EXPECT_TRUE((bool)Result);
+ return *Result;
}
inline std::string formatAndApply(StringRef Code,
const tooling::Replacements Replaces) {
- return applyAllReplacements(
+ auto Result = applyAllReplacements(
Code,
formatReplacements(
Code, cleanupAroundReplacements(Code, Replaces, Style), Style));
+ EXPECT_TRUE((bool)Result);
+ return *Result;
}
int getOffset(StringRef Code, int Line, int Column) {
Index: tools/clang-format/ClangFormat.cpp
===================================================================
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -257,13 +257,16 @@
unsigned CursorPosition = Cursor;
Replacements Replaces = sortIncludes(FormatStyle, Code->getBuffer(), Ranges,
AssumedFileName, &CursorPosition);
- std::string ChangedCode =
- tooling::applyAllReplacements(Code->getBuffer(), Replaces);
+ auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
+ if (!ChangedCode) {
+ llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
+ return true;
+ }
for (const auto &R : Replaces)
Ranges.push_back({R.getOffset(), R.getLength()});
bool IncompleteFormat = false;
- Replacements FormatChanges = reformat(FormatStyle, ChangedCode, Ranges,
+ Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges,
AssumedFileName, &IncompleteFormat);
Replaces = tooling::mergeReplacements(Replaces, FormatChanges);
if (OutputXML) {
Index: lib/Tooling/Core/Replacement.cpp
===================================================================
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -249,8 +249,10 @@
return Result;
}
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
- if (Replaces.empty()) return Code;
+llvm::Expected<std::string> applyAllReplacements(StringRef Code,
+ const Replacements &Replaces) {
+ if (Replaces.empty())
+ return Code.str();
IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
new vfs::InMemoryFileSystem);
@@ -269,7 +271,9 @@
Replacement Replace("<stdin>", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
- return "";
+ return llvm::make_error<llvm::StringError>(
+ "Failed to apply replacement: " + Replace.toString(),
+ llvm::inconvertibleErrorCode());
}
std::string Result;
llvm::raw_string_ostream OS(Result);
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1392,21 +1392,26 @@
return Replaces;
}
+// If any replacements in \p Replaces fails to apply, this returns \p Replaces.
template <typename T>
static tooling::Replacements
processReplacements(T ProcessFunc, StringRef Code,
const tooling::Replacements &Replaces,
const FormatStyle &Style) {
if (Replaces.empty())
return tooling::Replacements();
- std::string NewCode = applyAllReplacements(Code, Replaces);
+ auto NewCode = applyAllReplacements(Code, Replaces);
+ if (!NewCode) {
+ llvm::errs() << llvm::toString(NewCode.takeError()) << "\n";
+ return Replaces;
+ }
std::vector<tooling::Range> ChangedRanges =
tooling::calculateChangedRanges(Replaces);
StringRef FileName = Replaces.begin()->getFilePath();
tooling::Replacements FormatReplaces =
- ProcessFunc(Style, NewCode, ChangedRanges, FileName);
+ ProcessFunc(Style, *NewCode, ChangedRanges, FileName);
return mergeReplacements(Replaces, FormatReplaces);
}
Index: include/clang/Tooling/Core/Replacement.h
===================================================================
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -22,6 +22,7 @@
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
#include <map>
#include <set>
#include <string>
@@ -165,9 +166,13 @@
/// \brief Applies all replacements in \p Replaces to \p Code.
///
-/// This completely ignores the path stored in each replacement. If one or more
-/// replacements cannot be applied, this returns an empty \c string.
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
+/// This completely ignores the path stored in each replacement. If all
+/// replacements are applied successfully, this returns the result code;
+/// otherwise, an llvm::Error carrying llvm::StringError is returned (the Error
+/// message can be converted to string using `llvm::toString()` and
+/// 'std::error_code` in the `Error` should be ignored).
+llvm::Expected<std::string> applyAllReplacements(StringRef Code,
+ const Replacements &Replaces);
/// \brief Calculates how a code \p Position is shifted when \p Replaces are
/// applied.
@@ -203,29 +208,6 @@
std::vector<Replacement> Replacements;
};
-/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite.
-///
-/// Replacement applications happen independently of the success of
-/// other applications.
-///
-/// \returns true if all replacements apply. false otherwise.
-bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite);
-
-/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite.
-///
-/// Replacement applications happen independently of the success of
-/// other applications.
-///
-/// \returns true if all replacements apply. false otherwise.
-bool applyAllReplacements(const std::vector<Replacement> &Replaces,
- Rewriter &Rewrite);
-
-/// \brief Applies all replacements in \p Replaces to \p Code.
-///
-/// This completely ignores the path stored in each replacement. If one or more
-/// replacements cannot be applied, this returns an empty \c string.
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
-
/// \brief Calculates the ranges in a single file that are affected by the
/// Replacements. Overlapping ranges will be merged.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
