Re: [RCF 00/11] Visit varobj available children only in MI

[Keith Seitz <keiths@redhat.com>]

On 11/23/2013 06:09 PM, Yao Qi wrote:
> This patch series proposes a feature that GDB is able to visit varobj
> available children only, by adding a new option
> "--available-children-only" to commands -var-create,
> -var-info-num-children and -var-list-children.  This patch series was
> written by Pedro in one patch.  I read the patch three months ago, split
> it, add some minor things, and write test cases.  I wish I didn't
> break anything, including the rationale and the implementation :)

Hi, Yao,

I apologize for taking so long to get this review underway. I expected 
to have more time during the end-of-year holidays, but things didn't 
work out that way.

I have a few questions about this proposed feature. I understand you've 
inherited quite a bit of the code, and thank you for pursuing this 
submission.

Why was this feature implemented this way? Specifically, if some varobj 
children are not available from trace data, isn't the decision to filter 
this information a function of the user interface? Wouldn't it have 
sufficed to add a flag to the varobj designating the child as not 
collected in the trace experiment or unavailable?

I am definitely /not/ suggesting that this be re-implemented or 
re-designed. I just want to understand why this was implemented this 
way. Perhaps there is a significant speed advantage for large trace 
experiments or some other technical/legitimate reason for this 
implementation?

That aside, one other thing I'd like to ask about: the flag 
"--available-children-only" rather strikes me like a property of the 
varobj. Not altogether unlike the display format. Is there a reason a 
flag was chosen to implement this over, say, a (new) command like 
"-var-set-show-available-children-only" or requiring/allowing 
--available-children-only to be specified on the root varobj creation 
and "saved"/enforced for all subsequent commands on the varobj and its 
children?

The only rationale I can think of is if a UI wanted to query gdb/mi for 
varobjs with and without this option. Is that a common use case? Is 
there perhaps another use case which I have not considered?

Finally, I didn't see any mention of documentation updates. This change 
will require both a manual update and a NEWS entry, documenting the new 
feature.

I believe Joel has committed the MI "features" series; an update to this 
might be desirable [perhaps Joel might be able to offer an opinion].

Keith