jimingham wrote:

How right you are.  

I was thinking this was the ValueObject updated stuff, but it's the 
SyntheticChildrenFrontEnd::Update.  If I'm reading the code in 
ValueObjectSyntheticFilter.cpp aright, the return for that actually means the 
opposite of what you remembered.  If you return `false` from the 
SyntheticChildrenFrontEnd::Update, that means the VOSynthetic that uses this 
FrontEnd has to delete all its cached children, and recreate them when asked by 
GetChildAtIndex.

So for instance, in the case where a synthetic child front end just hands out a 
ValueObjectChild in the tree of the underlying ValueObject, then the child that 
gets cached in the VOSynthetic::m_synthetic_children_cache  knows how to update 
itself from its real parent, it doesn't need help from the 
ValueObjectSynthetic's FrontEndProvider to get the correct value.  So there's 
no point removing it from the cache and making a new child every time we stop, 
the one in the cache knows how to keep its value up to date.

But in the case where the children are, for instance, ValueObjectConstResults 
calculated by some means by the FrontEnd, to return `true` from `Update`, you 
would have to know that the FrontEnd at this stop point would produce the same 
VOConstResult as it did when you last stopped.  Or you would have to know how 
to manually update each of the cached children.  It would be possible for 
Update to do either of those things, but in general it seems like that's too 
much trouble and we really never try to figure that out. 

I am pretty sure the "Dereferenced" child of a ValueObject can update itself 
from the VO that generated it, in which case, the two formatters in this patch 
could have returned true and saved the work of recreating the child VO as the 
dereference.  But I haven't tested that theory, and I doubt just making up a 
new VO is worth the effort.  The only downside is if you keep throwing the 
children away at every stop, the synthetic children have no hope of reporting 
"IsChanged" correctly.

Anyway, not relevant to this patch.  I think returning false was wrong before, 
but that's just a small bit more work, so it's not worth worrying over.

https://github.com/llvm/llvm-project/pull/67069
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to