jingham added inline comments.
================ Comment at: lldb/include/lldb/Target/StackFrameList.h:103-104 - void GetFramesUpTo(uint32_t end_idx); + /// Gets frames up to end_idx (which can be greater than the actual number of + /// frames.) Returns true if the function was interrupted, false otherwise. + bool GetFramesUpTo(uint32_t end_idx, bool allow_interrupt = true); ---------------- mib wrote: > bulbazord wrote: > > Is the range inclusive of `end_idx`? as in, is it [0, end_idx) or [0, > > end_idx]? > @bulbazord I believe this should be inclusive. > > @jingham This comment sounds like we only return `true` if the function was > interrupted, which is not the expected behavior, right ? The function used to return void, so it had no expected behavior before. I am defining the "bool" return here, and I define it to be "was_interrupted" as I say in the doc string. That's also why the return values you are questioning below are actually correct. This is NOT returning "success" it is returning "interrupted". ================ Comment at: lldb/source/Target/StackFrameList.cpp:448 if (m_frames.size() > end_idx || GetAllFramesFetched()) - return; + return false; ---------------- mib wrote: > I find it confusing the fail here given that we've succeeded at fetching the > frames GetFramesUpTo returns true if interrupted, false if not. This was not interrupted, so it should return false. I could switch this to "true if not interrupted" so that I could return true here, but that would be weird. This isn't a "success or fail" result, it's an "interrupted or not interrupted" result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150236/new/ https://reviews.llvm.org/D150236 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
