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

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 24 06:29:31 PDT 2016

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?


More information about the cfe-commits mailing list