[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

[Alex Langford via Phabricator via lldb-commits]

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