lemo added a comment. Thanks everyone for the feedback.
I see a common question is why doesn't DoResume() fail-fast itself, or why don't the callers propagate the error. It's a good question, since the knowledge if a subclass can resume or not is almost codified in there. The problem is that the state is changed prior to calling DoResume() - see Process::Resume(), ... m_public_run_lock.TrySetRunning() ..., thus the need for an earlier check. @sas: Yes, a flip approach is possible and I briefly considered the default CanResume() to return false exactly as you're suggesting. The downside is a more brittle composability - subclasses can't call the base implementation, which is usually a reasonable expectation if one wants to build additional functionality on top of the base one. So I prefer the version I sent out, but I could be persuaded either way. @jingham: 1. I'm not sure I understand the comment about "over-determining the problem", can you please elaborate? If this is about having two methods instead of just DoResume(), I explained above the rationale. 2. Sorry, but defensive programming is exactly the wrong thing, and the root cause of what I'm fixing here. If the internal state is inconsistent the _only_ valid solution is to fail-fast. I've seen people arguing around this for everything from embedded systems to distributed services and trying to limp along pretending things are not too bad only lead to more pain. @amccarth: I agree that DoResume() can fail for reasons that CanResume() can't predict, and that's prefectly fine - the error is returned (and hopefully it's propagated correctly). Having DoResume() called for a core or minidump is never ok though, and that's the fail-fast path. (thanks' for the clang-format tip, I'll do that shortly) @ https://reviews.llvm.org/D37651 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
