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

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: include/clang/Format/Format.h:780
@@ -778,2 +779,3 @@
 /// \brief Returns the replacements corresponding to applying \p Replaces and
-/// cleaning up the code after that.
+/// cleaning up the code after that on success; otheriwse, return an llvm::Error
+/// carrying llvm::StringError.
----------------
typo: otheriwse

================
Comment at: lib/Format/Format.cpp:1395-1396
@@ -1394,2 +1394,4 @@
 
+// If any replacements in \p Replaces fails to apply, this returns \p Replaces.
+// FIXME: make this propagate the error from applyAllReplacements.
 template <typename T>
----------------
Comment doesn't apply any more, right?

================
Comment at: unittests/Format/CleanupTest.cpp:258
@@ +257,3 @@
+    auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+    EXPECT_TRUE((bool)CleanReplaces)
+        << llvm::toString(CleanReplaces.takeError()) << "\n";
----------------
The explicit cast is unfortunate. Does this not have an .Ok() method or something?


http://reviews.llvm.org/D21601