jingham added a comment.

I don't understand how DiscardPlansConsultingMasterPlan in this patch works.

With your changes, every plan but the ObjC trampoline finder returns false from 
OkayToDiscard.  So if you are trying to discard plans up to the innermost 
master plan, you start from the youngest plan asking it if is "OkayToDiscard".  
Since no plans return false, the iteration will go all the way to the Base 
thread plan regardless of whether there were intervening master plans, which 
isn't right.  Why didn't you ask IsMasterPlan in that loop rather than 
OkayToDiscard?

Was there a problem with just saying "Everything but master plans can be 
discarded when we are discarding plans."  I'm not sure why the ObjC Trampoline 
step through thread plan needs to be a Master Plan.  It seems to me it's just 
another subsidiary plan to a step in plan or something like.  Maybe it would be 
better to eliminate the OkayToDiscard altogether, and figure out a better way 
to handle the ObjC trampoline, if that actually needs doing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96715

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

Reply via email to