[Lldb-commits] [PATCH] Add low-frame/high-frame options to -stack-list-arguments (MI)

[Hafiz Abid Qadeer]

Looks good apart from a few comments. Please handle those before committing.


================
Comment at: tools/lldb-mi/MICmdCmdStack.cpp:568
@@ +567,3 @@
+        nFrameLow = pArgFrameLow->GetFound() ? pArgFrameLow->GetValue() : 0;
+        nFrameHigh = pArgFrameHigh->GetFound() ? pArgFrameHigh->GetValue() + 1 : UINT32_MAX;
+    }
----------------
You have already checked the GetFound() in if. Why you need it again inside.

================
Comment at: tools/lldb-mi/MICmdCmdStack.cpp:575
@@ -557,1 +574,3 @@
+    }
+
     CMICmnLLDBDebugSessionInfo &rSessionInfo(CMICmnLLDBDebugSessionInfo::Instance());
----------------
As I understand, you are trying to check in this else that either low or high is set but not both. Please put a comment as it is not obvious.

================
Comment at: tools/lldb-mi/MICmdCmdStack.cpp:591
@@ -571,2 +590,3 @@
     const MIuint nFrames = thread.GetNumFrames();
-    for (MIuint i = 0; i < nFrames; i++)
+    nFrameHigh = std::min(nFrameHigh, nFrames);
+    for (MIuint i = nFrameLow; i < nFrameHigh; i++)
----------------
The description of these options says
"It is an error if low-frame is larger than the actual number of frames."

So that check should also be done here.

http://reviews.llvm.org/D8282

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/