maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Make tooling::applyAllReplacements return llvm::Expected<string> instead of empty string to indicate potential error. 

Summary:
return llvm::Expected<> to carry error status and error information.
This is the first step towards introducing "Error" into tooling::Replacements.

Reviewers: djasper, klimek

Subscribers: ioeric, klimek, cfe-commits

Differential Revision: http://reviews.llvm.org/D21601

llvm-svn: 275062
This commit is contained in:
Eric Liu 2016-07-11 13:53:12 +00:00
parent 4d85003964
commit 4f8d99433d
16 changed files with 131 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
clang/include/clang/Format/Format.h
View File

@ -770,16 +770,18 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
unsigned *Cursor = nullptr);
/// \brief Returns the replacements corresponding to applying and formatting
/// \p Replaces.
tooling::Replacements formatReplacements(StringRef Code,
const tooling::Replacements &Replaces,
const FormatStyle &Style);
/// \p Replaces on success; otheriwse, return an llvm::Error carrying
/// llvm::StringError.
llvm::Expected<tooling::Replacements>
formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style);
/// \brief Returns the replacements corresponding to applying \p Replaces and
/// cleaning up the code after that.
/// cleaning up the code after that on success; otherwise, return an llvm::Error
/// carrying llvm::StringError.
/// This also inserts a C++ #include directive into the correct block if the
/// replacement corresponding to the header insertion has offset UINT_MAX.
tooling::Replacements
llvm::Expected<tooling::Replacements>
cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style);

34
clang/include/clang/Tooling/Core/Replacement.h
View File

@ -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 @@ bool applyAllReplacements(const std::vector<Replacement> &Replaces,
/// \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 code with
/// replacements applied; 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 @@ struct TranslationUnitReplacements {
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.
///

22
clang/lib/Format/Format.cpp
View File

@ -1393,27 +1393,29 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
}
template <typename T>
static tooling::Replacements
static llvm::Expected<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)
return NewCode.takeError();
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);
}
tooling::Replacements formatReplacements(StringRef Code,
const tooling::Replacements &Replaces,
const FormatStyle &Style) {
llvm::Expected<tooling::Replacements>
formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style) {
// We need to use lambda function here since there are two versions of
// `sortIncludes`.
auto SortIncludes = [](const FormatStyle &Style, StringRef Code,
@ -1421,8 +1423,10 @@ tooling::Replacements formatReplacements(StringRef Code,
StringRef FileName) -> tooling::Replacements {
return sortIncludes(Style, Code, Ranges, FileName);
};
tooling::Replacements SortedReplaces =
auto SortedReplaces =
processReplacements(SortIncludes, Code, Replaces, Style);
if (!SortedReplaces)
return SortedReplaces.takeError();
// We need to use lambda function here since there are two versions of
// `reformat`.
@ -1431,7 +1435,7 @@ tooling::Replacements formatReplacements(StringRef Code,
StringRef FileName) -> tooling::Replacements {
return reformat(Style, Code, Ranges, FileName);
};
return processReplacements(Reformat, Code, SortedReplaces, Style);
return processReplacements(Reformat, Code, *SortedReplaces, Style);
}
namespace {
@ -1591,7 +1595,7 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
} // anonymous namespace
tooling::Replacements
llvm::Expected<tooling::Replacements>
cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
const FormatStyle &Style) {
// We need to use lambda function here since there are two versions of

10
clang/lib/Tooling/Core/Replacement.cpp
View File

@ -249,8 +249,10 @@ bool applyAllReplacements(const std::vector<Replacement> &Replaces,
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 @@ std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
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);

8
clang/lib/Tooling/Refactoring.cpp
View File

@ -79,9 +79,13 @@ bool formatAndApplyAllReplacements(const Replacements &Replaces,
StringRef Code = SM.getBufferData(ID);
format::FormatStyle CurStyle = format::getStyle(Style, FilePath, "LLVM");
Replacements NewReplacements =
auto NewReplacements =
format::formatReplacements(Code, CurReplaces, CurStyle);
Result = applyAllReplacements(NewReplacements, Rewrite) && Result;
if (!NewReplacements) {
llvm::errs() << llvm::toString(NewReplacements.takeError()) << "\n";
return false;
}
Result = applyAllReplacements(*NewReplacements, Rewrite) && Result;
}
return Result;
}

9
clang/tools/clang-format/ClangFormat.cpp
View File

@ -257,13 +257,16 @@ static bool format(StringRef FileName) {
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) {

28
clang/unittests/Format/CleanupTest.cpp
View File

@ -25,9 +25,9 @@ protected:
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(static_cast<bool>(Result));
return *Result;
}
};
@ -254,16 +254,26 @@ protected:
inline std::string apply(StringRef Code,
const tooling::Replacements Replaces) {
return applyAllReplacements(
Code, cleanupAroundReplacements(Code, Replaces, Style));
auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
EXPECT_TRUE(static_cast<bool>(CleanReplaces))
<< llvm::toString(CleanReplaces.takeError()) << "\n";
auto Result = applyAllReplacements(Code, *CleanReplaces);
EXPECT_TRUE(static_cast<bool>(Result));
return *Result;
}
inline std::string formatAndApply(StringRef Code,
const tooling::Replacements Replaces) {
return applyAllReplacements(
Code,
formatReplacements(
Code, cleanupAroundReplacements(Code, Replaces, Style), Style));
auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
EXPECT_TRUE(static_cast<bool>(CleanReplaces))
<< llvm::toString(CleanReplaces.takeError()) << "\n";
auto FormattedReplaces = formatReplacements(Code, *CleanReplaces, Style);
EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
<< llvm::toString(FormattedReplaces.takeError()) << "\n";
auto Result = applyAllReplacements(Code, *FormattedReplaces);
EXPECT_TRUE(static_cast<bool>(Result));
return *Result;
}
int getOffset(StringRef Code, int Line, int Column) {

24
clang/unittests/Format/FormatTest.cpp
View File

@ -47,10 +47,10 @@ protected:
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(static_cast<bool>(Result));
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
return *Result;
}
FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
@ -11553,8 +11553,12 @@ TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
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 FormattedReplaces = formatReplacements(Code, Replaces, Style);
EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
<< llvm::toString(FormattedReplaces.takeError()) << "\n";
auto Result = applyAllReplacements(Code, *FormattedReplaces);
EXPECT_TRUE(static_cast<bool>(Result));
EXPECT_EQ(Expected, *Result);
}
TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
@ -11578,8 +11582,12 @@ TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
format::FormatStyle Style = format::getLLVMStyle();
Style.SortIncludes = true;
auto FinalReplaces = formatReplacements(Code, Replaces, Style);
EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
<< llvm::toString(FormattedReplaces.takeError()) << "\n";
auto Result = applyAllReplacements(Code, *FormattedReplaces);
EXPECT_TRUE(static_cast<bool>(Result));
EXPECT_EQ(Expected, *Result);
}
} // end namespace

8
clang/unittests/Format/FormatTestJS.cpp
View File

@ -28,10 +28,10 @@ protected:
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(static_cast<bool>(Result));
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
return *Result;
}
static std::string format(

8
clang/unittests/Format/FormatTestJava.cpp
View File

@ -25,10 +25,10 @@ protected:
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(static_cast<bool>(Result));
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
return *Result;
}
static std::string

8
clang/unittests/Format/FormatTestProto.cpp
View File

@ -25,10 +25,10 @@ protected:
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(static_cast<bool>(Result));
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
return *Result;
}
static std::string format(llvm::StringRef Code) {

8
clang/unittests/Format/FormatTestSelective.cpp
View File

@ -28,10 +28,10 @@ protected:
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(static_cast<bool>(Result));
DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
return *Result;
}
FormatStyle Style = getLLVMStyle();

9
clang/unittests/Format/SortImportsTestJS.cpp
View File

@ -25,10 +25,13 @@ protected:
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(static_cast<bool>(Sorted));
auto Formatted = applyAllReplacements(
*Sorted, reformat(Style, *Sorted, Ranges, FileName));
EXPECT_TRUE(static_cast<bool>(Formatted));
return *Formatted;
}
void verifySort(llvm::StringRef Expected, llvm::StringRef Code,

9
clang/unittests/Format/SortIncludesTest.cpp
View File

@ -26,10 +26,13 @@ protected:
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(static_cast<bool>(Sorted));
auto Result = applyAllReplacements(
*Sorted, reformat(Style, *Sorted, Ranges, FileName));
EXPECT_TRUE(static_cast<bool>(Result));
return *Result;
}
unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {

29
clang/unittests/Tooling/RefactoringTest.cpp
View File

@ -640,27 +640,32 @@ protected:
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(static_cast<bool>(AfterFirst));
auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second);
EXPECT_TRUE(static_cast<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(static_cast<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(static_cast<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(static_cast<bool>(MergedRewrite));
EXPECT_EQ(*InSequenceRewrite, *MergedRewrite);
if (*InSequenceRewrite != *MergedRewrite)
for (tooling::Replacement M : Merged)
llvm::errs() << M.getOffset() << " " << M.getLength() << " "
<< M.getReplacementText() << "\n";

5
clang/unittests/Tooling/RewriterTest.cpp
View File

@ -41,8 +41,9 @@ TEST(Rewriter, AdjacentInsertAndDelete) {
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(static_cast<bool>(Rewritten));
EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten);
}
} // end namespace