mgorny added a comment. In D98482#2624973 <https://reviews.llvm.org/D98482#2624973>, @labath wrote:
> 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? I've originally wanted to precisely convey whatever was in the packet since it was a generic class, and I actually thought nested `Optional`s are going to be worse. But thinking about it, you're probably right — the spec explicitly says to assume `tid=-1` when only pid is provided, so I guess we can use a single `Optional` and just assume a default pid. However, I think I'm going to go a step further and eliminate `pair` as well, in favor of `llvm::Optional<lldb::tid_t> GetPidTid(lldb:pid_t& pid)`, with `pid` being modified if specified or otherwise left in its current (i.e. default) value. 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
