jimingham wrote:
> > It seems a bit weird to have an Execution context with a thread and a stack
> > frame but not the process they belong to. I don't know that it would really
> > help either. Even if you removed the process so that you no longer had to
> > check for that being invalid, if a thread or frame had been set in the
> > execution context, you'd still have to check for them being from the wrong
> > process. So I don't think we would made things significantly simpler by
> > leaving out the process. I think it's easier to deal with it when we hand
> > them out. Jim
>
> Or we can have ExecutionContext have a single element so that it is always
> correct:
>
> ```
> ExecutionContext {
> ...
> std::shared_ptr<ExecutionContextScope> m_exe_scope_sp;
> ```
>
> Since target, proces, thread and frames all inherit from
> ExecutionContextScope, all we need to do is store one item. The
> ExecutionContextScope API has:
>
> ```
> virtual lldb::TargetSP CalculateTarget() = 0;
> virtual lldb::ProcessSP CalculateProcess() = 0;
> virtual lldb::ThreadSP CalculateThread() = 0;
> virtual lldb::StackFrameSP CalculateStackFrame() = 0;
> ```
>
> So then everything would always be correct.
The only time this will be incorrect in this way is if you make an execution
context filling in the current target & process and then hold onto it over the
process in the target changing. Internally, we only hold onto execution
context's while executing a command, and we return that to ourselves in the API
I fixed. So we really don't have to fix this anywhere else to solve actual
problems.
This seems like a much wider ranging change than is required to fix this bug.
I'm all for rethinking the
ExecutionContextScope/ExecutionContext/ExecutionContextRef nexus, but I don't
think that's a trivial rethink, and I don't have the time to undertake that
right now.
https://github.com/llvm/llvm-project/pull/65822
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
