clayborg wrote:
> @clayborg as far as I can tell, the ball is currently in your court to
> respond to @jimingham ... thanks in advance
Sounds like Jim is on the fence with a SetDirection + existing APIs and adding
a direction to the calls. I am fine with either approach as long as Jim is ok
with it because Jim is the code owner of the thread plans so he has better
knowledge. Let me know if I missed any comment I should respond to other than
this.
It really comes down to how the GUI and command line commands work for reverse
debugging. So a few questions:
- Do GUI debuggers have different buttons for reverse stepping and running? Or
is there a direction button and then the existing process control buttons
remain the same (reverse continue, reverse step in/out/over)
- For command line do we have different comments for forward and reverse, or a
direction command followed by normal commands?
If the GUI and command line have separate buttons and commands, then we should
probably go with adding the direction to each API. This means duplicating each
API and adding a direction enum, then taking the old API and forwarding to the
new one. So for:
```
void SBThread::StepOver(lldb::RunMode stop_other_threads, SBError &error);
```
we would now have:
```
void SBThread::StepOver(lldb::RunMode stop_other_threads, SBError &error,
lldb::RunDirection direction) {
...
}
void SBThread::StepOver(lldb::RunMode stop_other_threads, SBError &error) {
return StepOver(stop_other_threads, error, eRunForward);
}
```
Another thing we tend to do if we start getting too many overloads it to create
an options class. Right now for step over we have two overloads:
```
void SBThread::StepOver(lldb::RunMode stop_other_threads);
void SBThread::StepOver(lldb::RunMode stop_other_threads, SBError &error);
```
Now we will add a third. If we keep needing more options in the future our
overloads could start adding up. So what we do to avoid this is make an options
class like:
```
class SBThreadStepOptions {
bool GetStopOtherThreads();
void SetStopOtherThreads(bool);
lldb::RunDirection GetRunDirection();
void SetRunDirection(lldb::RunDirection direction);
}
```
And then we have the SBThread::StepXXX calls use this options class
```
SBError SBThread::StepOver(const SBThreadStepOptions &options);
```
This is good because we can add accessors to the `SBThreadStepOptions` class
without needing to ever add another `SBThread::StepOver` overload in the
future. I could see us adding more options to the `SBThreadStepOptions` API in
the future and then our public API for `SBThread::StepOver` doesn't need to
change.
https://github.com/llvm/llvm-project/pull/99736
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
