jingham added a comment.

Sorry, I sent a reply but did it by replying to the e-mail version of the 
patch, and the mail server choked on it for reasons that aren't clear to me...

Greg was most likely thinking that BreakpointLocation had a "Clear" method like 
the Target and Process, which we can call when WE know we're done with 
something, but we can't tell when it will actually get destroyed.  
BreakpointLocation hasn't needed such a thing.  In fact, if you wanted to fix 
this by adding a Clear method and removing the expression there, that would be 
fine too.  In the process & target case we have to know that we've cleared 
them, but since you're only throwing away a cache, you wouldn't need to do any 
extra work for BreakpointLocations.

However, solving the reference loop with a weak pointer in SBBreakpoint is fine 
as well.  The targets keep the breakpoints alive as long as the target exists, 
and about the only  sensible thing you can do with a breakpoint after it's 
target is gone, is print it's ID in an error "Hey, I tried to do something with 
breakpoint 1 but it didn't work because it's target was gone".  Now you will 
either have to keep that info redundantly on the side or say <UNKNOWN> rather 
than "1".

That doesn't seems worth keeping the breakpoint alive.


https://reviews.llvm.org/D30249



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

Reply via email to