[Lldb-commits] [PATCH] D48295: [WIP] Implement new ReturnMIStatus method of CMICmdBase class.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In https://reviews.llvm.org/D48295#1137595, @apolyakov wrote:

> Changed method's name from `ReturnMIStatus` to `HandleSBError`. Also added one more use case(see -exec-step's Execute function).
>
> The only thing that worries me is that if we want to specify handler for error case, we should do something like:
>
>   SBError error = ...
>   auto errorHandler = ...
>  
>   return HandleSBError(error, [] { return MIstatus::success; }, errorHandler);
>
>
> since in C++ we can't skip first optional argument and only pass second one.


Does it ever make sense to return something different than `MIstatus::failure` from the failure handler? Since HandleSBError sets the Error string, probably not. So we could change the error handler to a `const std::function<void()>`.

Then you can define the following overloads:

  bool CMICmdBase::HandleSBError(lldb::SBError error, const std::function<bool()> successHandler = [] { return MIstatus::success; }, const std::function<void()> errorHandler = [] { return MIstatus::failure; });
  bool CMICmdBase::HandleSBError(lldb::SBError error, const std::function<void()> errorHandler);
  // this one is for the second half of Execute()
  bool CMICmdBase::HandleMIstatus(bool status, const std::function<void()> errorHandler = [] { return MIstatus::success; });

By the way, I don't think it makes sense to pass any of the arguments by reference. Both SBError and std::function should be very cheap to pass by value.


https://reviews.llvm.org/D48295