[Lldb-commits] [PATCH] D98482: [lldb] Support for multiprocess extension [WIP]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm still bugged by the GetPidTid interface -- the pair of optionals is very complicated. What if we had this function take a `default_pid` argument (which would be equal to the currently selected process), and it returned that as the process id, in case the packet does not contain one? Then the interface could be something like `Optional<pair<pid_t, tid_t>> GetPidTid(pid_t default_pid)` (because tid automatically defaults to -1), which I think is more intuitive. In case we really need to differentiate between a pid not being present and an implicitly chosen pid, then the interface could be like `Optional<pair<Optional<pid_t>, tid_t>> GetPidTid()`, which still seems /slightly/ better as its easier to recognise invalid input. WDYT?

Regarding the `ReadTid` function, I think that sending the (error) response packet from within that is a bit too much. I think it would be better if that returned an Error/Expected and left the sending to the caller.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98482/new/

https://reviews.llvm.org/D98482