aprantl added a comment.
> I agree with Leonard, for the StepOver overload that returns errors, just
> make the mode parameter required. That will reduce confusion.
Works for me, too.
================
Comment at: source/API/SBThread.cpp:1136
bool result = false;
if (exe_ctx.HasThreadScope()) {
Process::StopLocker stop_locker;
----------------
apolyakov wrote:
> @aprantl do you think that we should use early exit here? There is printing
> to log at the end of the function, so in case of early exit, we must
> duplicate `log->Printf()`. Is it worth it?
Duplicating the log message is probably not worth it.
================
Comment at: source/API/SBThread.cpp:1141
result = true;
} else {
if (log)
----------------
It looks like the error string should be set here as well?
================
Comment at: source/API/SBThread.cpp:1146
}
- }
+ } else error.SetErrorString("this SBThread object is invalid");
if (log)
----------------
I think clang-format would do this as:
```
} else
error.SetErrorString("this SBThread object is invalid");
```
================
Comment at: source/API/SBThread.cpp:1171
} else {
if (log)
log->Printf("SBThread(%p)::Resume() => error: process is running",
----------------
same here.
================
Comment at: source/API/SBThread.cpp:1179
static_cast<void *>(exe_ctx.GetThreadPtr()), result);
return result;
}
----------------
Looking at the implementation of SBThread::Resume and SBThread::Suspend, the
only difference seems to be the API called and the log message. If there is a
third command with a similar implementation, it might be a fun exercise to
factor that out into a helper function that takes std::function<> and a log
string argument...
https://reviews.llvm.org/D47991
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
