[cfe-dev] r161350 breaks the intention of ParentMap::addStmt

[Jordan Rose]

On Oct 1, 2012, at 5:21 , Olaf Krzikalla <Olaf.Krzikalla at tu-dresden.de> wrote:

> Am 27.09.2012 08:40, schrieb John McCall:
>> OVEs model complex language features.  Where the language is
>> less complex, we don't use them.  Where we do use them, you
>> almost certainly need to think more carefully about the right thing to do.
> Well thats OK as long as this doesn't break code, if OVEs are not used. However in the particular case of parent map a rather basic piece of the AST is influenced by them. Thus I'd rather suggest to fix that.
> The fundamental flaw of the design is the unclear parent-child relation introduced by OVEs. You see, an OVE has no (implict) child. Thus, shall the OVE be a parent of the source expression of the OVE? IMHO not.
> 
> Said that I propose a revert of r161350 as well as r150894 in ParentMap. There must be another fix of <rdar://problem/10797980> (which I admittedly don't know).
> 
> To recap the example given by Jordan: currently the PseudoObjectExpr is as follows:
> 
> 0: $1_1.property = $2_1
> 1: 'self'
> 2: 'value'
> 3: [$1_2 setProperty:$2_2]
> 
> $X_X are the OVEs. There are 4 OVEs and each OVE points to either self or value. Couldn't we reorder it in the following way:
> 
> 0: self.property = value
> 1: $1_1
> 2: $2_1
> 3: [$1_2 setProperty:$2_2]
> 
> ? First this would solve the issues in parent map since the syntactic form now has no OVEs in it (and we do not need to treat OVEs at all in parent map). Second, whenever a PseudoObjectExpr must be treated specifically, also the OVEs in it can be handled specifically.

This makes things harder for CodeGen (and for the analyzer evaluation). Remember, currently CodeGen is able to make these assumptions:

(1) When you see an OpaqueValueExpr, its source expr has already been evaluated.
(2) To evaluate a PseudoObjectExpr, just evaluate each of its semantic exprs in order.

Changing the OVEs in the POE to refer into the ObjCPropertyRefExpr violates this, and makes $1_1 and $2_1 kind of useless.

I see what you mean about the problem with ParentMap, but the idea is that asking "what is the parent of this expression with multiple parents?" is not a good question. For POEs, the syntactic expression is probably the best choice. For BinaryConditionalOperator, there's a choice between the implicit cast-to-bool in the "condition", and the BCO itself for the "result".

Do you agree here? In your use case, what would the right parents be?
Jordan