[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

[Jim Ingham via Phabricator via lldb-commits]

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