xiaobai added a comment.

In D63181#1539291 <https://reviews.llvm.org/D63181#1539291>, @JDevlieghere 
wrote:

> Have you considered making just `AddExceptionPrecondition` virtual? Wouldn't 
> that solve the problem too, without the code duplication of making 
> `CreateExceptionBreakpoint` virtual?


I don't believe so. As I understand it, you might not have a process yet when 
you call `LanguageRuntime::CreateExceptionBreakpoint` from `Target`, so you 
won't have an instance of LanguageRuntime. You could create an instance of a 
LanguageRuntime object with a nullptr process just to invoke it as an instance 
method, but that seems like the wrong choice to me. All this combined has led 
me to believe that `AddExceptionPrecondition` needs to get a callback to a 
static function from PluginManager.

> Also, I think it's totally reasonable to hoist `BreakpointPrecondition` out 
> of `Breakpoint`. After having had to deal with the nuisance of not being able 
> to forward declare nested classes, I try to avoid them in general.

Yeah, I was kinda iffy on this, but if you (and possibly others) think this is 
reasonable, I might go with this option. The only thing that would change is 
the callback would return a `BreakpointPreconditionSP` instead of `void`, and 
`AddExceptionPrecondition` would be the one to modify the breakpoint. I don't 
think the core idea would change much but the implementation might be slightly 
nicer. Let me know if you prefer that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63181/new/

https://reviews.llvm.org/D63181



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to