[PATCH] D17778: TypedError for recoverable error handling

[Lang Hames via llvm-commits]

lhames marked an inline comment as done.
lhames added a comment.

Updated patch should be in shortly.


================
Comment at: include/llvm/Support/Error.h:492
@@ +491,3 @@
+                    OS << "\n";
+                    return Error::success();
+                  });
----------------
dblaikie wrote:
> Drop this?
Yep.

================
Comment at: include/llvm/Support/Error.h:505
@@ +504,3 @@
+  handleAllErrors(std::move(Err),
+                  [](const ErrorInfoBase &) { return Error::success(); });
+}
----------------
dblaikie wrote:
> Drop the return here?
Also yep.

================
Comment at: unittests/Support/ErrorTest.cpp:290
@@ +289,3 @@
+                      CustomErrorExtraInfo = SE.getExtraInfo();
+                      return Error();
+                    },
----------------
dblaikie wrote:
> Drop this?
Yep.

================
Comment at: unittests/Support/ErrorTest.cpp:301
@@ +300,3 @@
+                      CustomErrorInfo1 = CE.getInfo();
+                      return Error();
+                    });
----------------
dblaikie wrote:
> And this?
Yep.

================
Comment at: unittests/Support/ErrorTest.cpp:406
@@ +405,3 @@
+                     "when an error exists!\"\\)")
+        << "Incorrect Expected error value";
+    consumeError(A.takeError());
----------------
joker.eph wrote:
> How does it work with assertions disabled? (alternatively: where is this identified as "requires: assert"?)
> 
> To avoid the issue reported by David, you might roll your own "assert" so that you control the exact printed message?
I'm updating the Error class to disable the 'Checked' bit in Release mode, so I'll wrap all these tests with #ifndef NDEBUG. 


http://reviews.llvm.org/D17778