[Lldb-commits] [PATCH] D66249: [JIT][Breakpoint] Add "BreakpointInjectedSite" and FCB Trampoline

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 2 inline comments as done.
mib added a comment.

In D66249#1642316 <https://reviews.llvm.org/D66249#1642316>, @labath wrote:

> A bunch more comments from me. So far, I've only tried to highlight the most obvious problems or style issues.
>
>   It's not clear to me whether this is still prototype code or not... If it isn't, I'll have a bunch more.. :)


This is prototype code that I'm trying to upstream to build upon.

>From the comments I already gathered, I started thinking of a better implementation of this feature (maybe v2).
There are still some parts that need do be done, such as resolving the variables on optimized code or patching dynamically the copied instructions.

But, I'd like to have a baseline that is upstream to start working on these parts.

**All feedback is welcome !** :)



================
Comment at: lldb/include/lldb/Breakpoint/BreakpointSite.h:54-56
+   static bool classof(const BreakpointSite *bp_site) {
+     return bp_site->getKind() == eKindBreakpointSite;
+   }
----------------
labath wrote:
> mib wrote:
> > labath wrote:
> > > This is weird. So, in OO terminology, `BreakpointInjectedSite` "is-a" `BreakpointSite`, but if you ask `llvm::isa<BreakpointSite>(injected_site)`, it will return false?
> > No, it returns **true**.
> Then does `llvm::isa<BreakpointInjectedSite>(injected_site)` return false? They can't both return true, given the current implementations (but they should, if the dynamic type of `injected_site` really is `BreakpointInjectedSite`...
`llvm::isa<BreakpointSite>(injected_site) ` & `llvm::isa<BreakpointInjectedSite>(injected_site)` both return **true**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66249