Indeed, ideally there is no bool return value and we just write return
make_error<Failure>("foo")
On Mon, May 15, 2017 at 5:58 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> 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.
>
> 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).
>
> That said, I'm not sure I should be the person approving this. :)
>
>
>
> ================
> Comment at: lldb/source/Commands/CommandObjectPlatform.cpp:611
> + if (!platform_sp->CloseFile(fd, error))
> + return result.AppendError(eReturnStatusFailed, error.AsCString());
> +
> ----------------
> I believe this should be `"{0}", error` to avoid hitting problems in case
> the error contains '{'. (Plus it's 5 chars shorter :) )
>
>
> ================
> Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1181
> + "Failed to send signal {0}: {1}\n", signo,
> + ST.AsCString());
> +
> ----------------
> AsCString unnecessary
>
>
> https://reviews.llvm.org/D33167
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
