[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lit/tools/lldb-mi/exec/exec-next.test:19
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.
----------------
aprantl wrote:
> 0 feels like it might actually be a valid thread id on some systems.. perhaps use really high number instead?
I was surprised by that as well, so I tried a some experiments. I don't know how or why, but lldb-mi seems to use it's own notion of thread-ids, which are independent of os-level ids and always start with one. I guess they are just indexes into the list of threads. I don't know if that is intentional or what.


================
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384
+  if (nThreadId != UINT64_MAX) {
+    lldb::SBThread sbThread = rSessionInfo.GetProcess().GetThreadByID(nThreadId);
+    if (sbThread.IsValid())
----------------
polyakov.alex wrote:
> We can't test this branch until we have a way to get thread ID and then store it in some variable.
Yes, that's the issue I was alluding to. I am not going to block this patch over it or anything, but I want to make sure you are aware that you're hitting the limitations of the FileCheck test approach already.

That said, if what I said above about the thread-id's always being numbered starting from one is true, then the thread ids may be predictable enough to test using `-exec-next --thread 1`  and avoid this problem for now.


https://reviews.llvm.org/D47797