Index: cfe/trunk/include/clang/Format/Format.h =================================================================== --- cfe/trunk/include/clang/Format/Format.h +++ cfe/trunk/include/clang/Format/Format.h @@ -770,16 +770,18 @@ 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 +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 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style); Index: cfe/trunk/include/clang/Tooling/Core/Replacement.h =================================================================== --- cfe/trunk/include/clang/Tooling/Core/Replacement.h +++ cfe/trunk/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 #include #include @@ -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 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 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 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 &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. /// Index: cfe/trunk/lib/Format/Format.cpp =================================================================== --- cfe/trunk/lib/Format/Format.cpp +++ cfe/trunk/lib/Format/Format.cpp @@ -1393,27 +1393,29 @@ } template -static tooling::Replacements +static llvm::Expected 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 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 +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 @@ 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 @@ 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 @@ } // anonymous namespace -tooling::Replacements +llvm::Expected cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style) { // We need to use lambda function here since there are two versions of Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp =================================================================== --- cfe/trunk/lib/Tooling/Core/Replacement.cpp +++ cfe/trunk/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 applyAllReplacements(StringRef Code, + const Replacements &Replaces) { + if (Replaces.empty()) + return Code.str(); IntrusiveRefCntPtr InMemoryFileSystem( new vfs::InMemoryFileSystem); @@ -269,7 +271,9 @@ Replacement Replace("", I->getOffset(), I->getLength(), I->getReplacementText()); if (!Replace.apply(Rewrite)) - return ""; + return llvm::make_error( + "Failed to apply replacement: " + Replace.toString(), + llvm::inconvertibleErrorCode()); } std::string Result; llvm::raw_string_ostream OS(Result); Index: cfe/trunk/lib/Tooling/Refactoring.cpp =================================================================== --- cfe/trunk/lib/Tooling/Refactoring.cpp +++ cfe/trunk/lib/Tooling/Refactoring.cpp @@ -79,9 +79,13 @@ 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; } Index: cfe/trunk/tools/clang-format/ClangFormat.cpp =================================================================== --- cfe/trunk/tools/clang-format/ClangFormat.cpp +++ cfe/trunk/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: cfe/trunk/unittests/Format/CleanupTest.cpp =================================================================== --- cfe/trunk/unittests/Format/CleanupTest.cpp +++ cfe/trunk/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(static_cast(Result)); + return *Result; } }; @@ -254,16 +254,26 @@ 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(CleanReplaces)) + << llvm::toString(CleanReplaces.takeError()) << "\n"; + auto Result = applyAllReplacements(Code, *CleanReplaces); + EXPECT_TRUE(static_cast(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(CleanReplaces)) + << llvm::toString(CleanReplaces.takeError()) << "\n"; + auto FormattedReplaces = formatReplacements(Code, *CleanReplaces, Style); + EXPECT_TRUE(static_cast(FormattedReplaces)) + << llvm::toString(FormattedReplaces.takeError()) << "\n"; + auto Result = applyAllReplacements(Code, *FormattedReplaces); + EXPECT_TRUE(static_cast(Result)); + return *Result; } int getOffset(StringRef Code, int Line, int Column) { Index: cfe/trunk/unittests/Format/FormatTest.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/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(static_cast(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; } FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { @@ -11553,8 +11553,12 @@ 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(FormattedReplaces)) + << llvm::toString(FormattedReplaces.takeError()) << "\n"; + auto Result = applyAllReplacements(Code, *FormattedReplaces); + EXPECT_TRUE(static_cast(Result)); + EXPECT_EQ(Expected, *Result); } TEST_F(ReplacementTest, SortIncludesAfterReplacement) { @@ -11578,8 +11582,12 @@ 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(FormattedReplaces)) + << llvm::toString(FormattedReplaces.takeError()) << "\n"; + auto Result = applyAllReplacements(Code, *FormattedReplaces); + EXPECT_TRUE(static_cast(Result)); + EXPECT_EQ(Expected, *Result); } } // end namespace Index: cfe/trunk/unittests/Format/FormatTestJS.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -28,10 +28,10 @@ tooling::Replacements Replaces = reformat(Style, Code, Ranges, "", &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(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; } static std::string format( Index: cfe/trunk/unittests/Format/FormatTestJava.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestJava.cpp +++ cfe/trunk/unittests/Format/FormatTestJava.cpp @@ -25,10 +25,10 @@ DEBUG(llvm::errs() << Code << "\n\n"); std::vector 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(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; } static std::string Index: cfe/trunk/unittests/Format/FormatTestProto.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestProto.cpp +++ cfe/trunk/unittests/Format/FormatTestProto.cpp @@ -25,10 +25,10 @@ DEBUG(llvm::errs() << Code << "\n\n"); std::vector 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(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; } static std::string format(llvm::StringRef Code) { Index: cfe/trunk/unittests/Format/FormatTestSelective.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestSelective.cpp +++ cfe/trunk/unittests/Format/FormatTestSelective.cpp @@ -28,10 +28,10 @@ tooling::Replacements Replaces = reformat(Style, Code, Ranges, "", &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(Result)); + DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; } FormatStyle Style = getLLVMStyle(); Index: cfe/trunk/unittests/Format/SortImportsTestJS.cpp =================================================================== --- cfe/trunk/unittests/Format/SortImportsTestJS.cpp +++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp @@ -25,10 +25,13 @@ if (Length == 0U) Length = Code.size() - Offset; std::vector 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(Sorted)); + auto Formatted = applyAllReplacements( + *Sorted, reformat(Style, *Sorted, Ranges, FileName)); + EXPECT_TRUE(static_cast(Formatted)); + return *Formatted; } void verifySort(llvm::StringRef Expected, llvm::StringRef Code, Index: cfe/trunk/unittests/Format/SortIncludesTest.cpp =================================================================== --- cfe/trunk/unittests/Format/SortIncludesTest.cpp +++ cfe/trunk/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(static_cast(Sorted)); + auto Result = applyAllReplacements( + *Sorted, reformat(Style, *Sorted, Ranges, FileName)); + EXPECT_TRUE(static_cast(Result)); + return *Result; } unsigned newCursor(llvm::StringRef Code, unsigned Cursor) { Index: cfe/trunk/unittests/Tooling/RefactoringTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp +++ cfe/trunk/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(static_cast(AfterFirst)); + auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second); + EXPECT_TRUE(static_cast(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(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(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(MergedRewrite)); + EXPECT_EQ(*InSequenceRewrite, *MergedRewrite); + if (*InSequenceRewrite != *MergedRewrite) for (tooling::Replacement M : Merged) llvm::errs() << M.getOffset() << " " << M.getLength() << " " << M.getReplacementText() << "\n"; Index: cfe/trunk/unittests/Tooling/RewriterTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/RewriterTest.cpp +++ cfe/trunk/unittests/Tooling/RewriterTest.cpp @@ -41,8 +41,9 @@ Replacements Replaces; Replaces.insert(Replacement("", 6, 6, "")); Replaces.insert(Replacement("", 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(Rewritten)); + EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten); } } // end namespace