Re: [cdt-dev] DSF: ExprMetaGetVarInfo

[Marc Khouzam <marc.khouzam@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Vladimir Prus [mailto:vladimir@xxxxxxxxxxxxxxxx] 
> Sent: Wednesday, April 27, 2011 10:45 AM
> To: cdt-dev@xxxxxxxxxxx
> Cc: Marc Khouzam
> Subject: Re: [cdt-dev] DSF: ExprMetaGetVarInfo
> 
> On Wednesday, April 27, 2011 18:19:45 Marc Khouzam wrote:
> > > -----Original Message-----
> > > From: cdt-dev-bounces@xxxxxxxxxxx
> > > [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Vladimir Prus
> > > Sent: Wednesday, April 27, 2011 10:06 AM
> > > To: CDT General developers list.
> > > Subject: [cdt-dev] DSF: ExprMetaGetVarInfo
> > > 
> > > 
> > > Folks,
> > > 
> > > looking at current code, and I not entirely grasping why
> > > there is ExprMetaGetVarInfo
> > > class. It appears to be created only in
> > > MIVariableManager.queueCommand, when the
> > > command is ExprMetaGetVar. The latter, in turn, is only used
> > > in MIExpressions.getExpressionData.
> > > As soon as queueCommand gives us ExprMetaGetVarInfo, we
> > > create ExpressionDMData out of it.
> > > Would it not be more direct to have
> > > MIVariableManager.queueCommand directly create
> > > ExpressionDMData?
> > 
> > The idea was to keep MIVariableManager and MIExpressions
> > separated from each other.  Each meta varObj command
> > has its matching info command, and that is mostly how the
> > two classes communicate.  It may be that in the case
> > you mention it was not strictly necessary, I'm not entirely
> > sure.  However, I think that it makes the design more
> > consistent and flexible.
> 
> I am afraid I fail to see what we gain by this design. We had
> to add some additional properties, and we ended up adding
> them to both ExpressionDMData and ExprMetaGetVarInfo. So,
> ExprMetaGetVarInfo does not insulate MIExpressions from changes,
> it is just an extra object to modify.

If you look on HEAD, you'll see that ExprMetaGetVarInfo has more
fields than ExpressionDMData.  For instance, isCollectionHint and
isSafeToAskForAllChildren, which were added for prettyPrinting,
are used by MIExpressions, but don't need to be contained
in ExpressionDMData.

It is possible that in your case, the new fields needed to be
added to both classes, but it is not always the case.

Furthermore, keeping the two separate would help if we have to
deal with different versions of the expression service.  For
example, if we have different implementations of ExpressionDMData
for different services, something like, say, ExpressionDMData and
ExpressionDMData_7_4, we don't want MIVariableManager to have
to be versioned as well; it would be better to only version
MIExpressions.  Having ExprMetaGetVarInfo would help there too.


> I have similar questions about other meta commands, but given
> that say ExprMetaGetValueInfo is just a string, it does not
> cause as much problems -- although it seems that eliminating
> it would simplify the code as well.