Indeed, ideally there is no bool return value and we just write return make_error<Failure>("foo")<br><div class="gmail_quote"><div dir="ltr">On Mon, May 15, 2017 at 5:58 AM Pavel Labath via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br>
<br>
I think this is a step in the right direction. Besides reducing boilerplate, this will also help us ensure correctness, as we get a constant trickle of bug reports for commands which forgot to set the result status.<br>
<br>
The name is not ideal, but it's probably the best we can get. (The ideal solution for me would be to get rid of the duality in the DoExecute function -- e.g. remove the bool return and let the execution state be fully signalled by the result object -- but that's way off from what you were originally trying to do).<br>
<br>
That said, I'm not sure I should be the person approving this. :)<br>
<br>
<br>
<br>
================<br>
Comment at: lldb/source/Commands/CommandObjectPlatform.cpp:611<br>
+    if (!platform_sp->CloseFile(fd, error))<br>
+      return result.AppendError(eReturnStatusFailed, error.AsCString());<br>
+<br>
----------------<br>
I believe this should be `"{0}",  error` to avoid hitting problems in case the error contains '{'. (Plus it's 5 chars shorter :) )<br>
<br>
<br>
================<br>
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1181<br>
+                                "Failed to send signal {0}: {1}\n", signo,<br>
+                                ST.AsCString());<br>
+<br>
----------------<br>
AsCString unnecessary<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33167" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33167</a><br>
<br>
<br>
<br>
</blockquote></div>