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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to