diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 3f42dbfa25bc..1ff305d030aa 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -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 +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); diff --git a/clang/include/clang/Tooling/Core/Replacement.h b/clang/include/clang/Tooling/Core/Replacement.h index 909ee12ade99..4815302ee455 100644 --- a/clang/include/clang/Tooling/Core/Replacement.h +++ b/clang/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 @@ bool applyAllReplacements(const std::vector &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 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 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. /// diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 6f7ee274e994..32d6bb855ad6 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1393,27 +1393,29 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, } 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 @@ 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 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style) { // We need to use lambda function here since there are two versions of diff --git a/clang/lib/Tooling/Core/Replacement.cpp b/clang/lib/Tooling/Core/Replacement.cpp index d52d0b089853..4f130709ac16 100644 --- a/clang/lib/Tooling/Core/Replacement.cpp +++ b/clang/lib/Tooling/Core/Replacement.cpp @@ -249,8 +249,10 @@ bool applyAllReplacements(const std::vector &Replaces, 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 @@ std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) { 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); diff --git a/clang/lib/Tooling/Refactoring.cpp b/clang/lib/Tooling/Refactoring.cpp index 51f0635c1ecb..28d535aeb45f 100644 --- a/clang/lib/Tooling/Refactoring.cpp +++ b/clang/lib/Tooling/Refactoring.cpp @@ -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; } diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp index a9b077e0b1ad..27577a5a336a 100644 --- a/clang/tools/clang-format/ClangFormat.cpp +++ b/clang/tools/clang-format/ClangFormat.cpp @@ -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) { diff --git a/clang/unittests/Format/CleanupTest.cpp b/clang/unittests/Format/CleanupTest.cpp index f51e3acf9aca..5f85c53b780a 100644 --- a/clang/unittests/Format/CleanupTest.cpp +++ b/clang/unittests/Format/CleanupTest.cpp @@ -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(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(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) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 218243380c3d..8d46ba6efcfe 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -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(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(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 @@ 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(FormattedReplaces)) + << llvm::toString(FormattedReplaces.takeError()) << "\n"; + auto Result = applyAllReplacements(Code, *FormattedReplaces); + EXPECT_TRUE(static_cast(Result)); + EXPECT_EQ(Expected, *Result); } } // end namespace diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp index a5f0b19577a4..5fda89d45d61 100644 --- a/clang/unittests/Format/FormatTestJS.cpp +++ b/clang/unittests/Format/FormatTestJS.cpp @@ -28,10 +28,10 @@ protected: 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( diff --git a/clang/unittests/Format/FormatTestJava.cpp b/clang/unittests/Format/FormatTestJava.cpp index 30d1bc8baf82..dfc3debc46e2 100644 --- a/clang/unittests/Format/FormatTestJava.cpp +++ b/clang/unittests/Format/FormatTestJava.cpp @@ -25,10 +25,10 @@ protected: 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 diff --git a/clang/unittests/Format/FormatTestProto.cpp b/clang/unittests/Format/FormatTestProto.cpp index d9fb1409139d..6881af4361cf 100644 --- a/clang/unittests/Format/FormatTestProto.cpp +++ b/clang/unittests/Format/FormatTestProto.cpp @@ -25,10 +25,10 @@ protected: 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) { diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp index c4286d4297dc..2bc60fd1e0d3 100644 --- a/clang/unittests/Format/FormatTestSelective.cpp +++ b/clang/unittests/Format/FormatTestSelective.cpp @@ -28,10 +28,10 @@ protected: 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(); diff --git a/clang/unittests/Format/SortImportsTestJS.cpp b/clang/unittests/Format/SortImportsTestJS.cpp index e6b5273f7b10..77c37e337ddf 100644 --- a/clang/unittests/Format/SortImportsTestJS.cpp +++ b/clang/unittests/Format/SortImportsTestJS.cpp @@ -25,10 +25,13 @@ protected: 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, diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp index c8a43fc6240c..13a1b9adeeec 100644 --- a/clang/unittests/Format/SortIncludesTest.cpp +++ b/clang/unittests/Format/SortIncludesTest.cpp @@ -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(Sorted)); + auto Result = applyAllReplacements( + *Sorted, reformat(Style, *Sorted, Ranges, FileName)); + EXPECT_TRUE(static_cast(Result)); + return *Result; } unsigned newCursor(llvm::StringRef Code, unsigned Cursor) { diff --git a/clang/unittests/Tooling/RefactoringTest.cpp b/clang/unittests/Tooling/RefactoringTest.cpp index fbf54134a7cc..df96bb159dea 100644 --- a/clang/unittests/Tooling/RefactoringTest.cpp +++ b/clang/unittests/Tooling/RefactoringTest.cpp @@ -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(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"; diff --git a/clang/unittests/Tooling/RewriterTest.cpp b/clang/unittests/Tooling/RewriterTest.cpp index 93f69eb9fa0c..e8afedb01174 100644 --- a/clang/unittests/Tooling/RewriterTest.cpp +++ b/clang/unittests/Tooling/RewriterTest.cpp @@ -41,8 +41,9 @@ TEST(Rewriter, AdjacentInsertAndDelete) { 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