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

[Olaf Krzikalla]

Hi Jordan,

Am 21.09.2012 18:18, schrieb Jordan Rose:
> Interesting, and sorry for the comment-incompatible update. Let me explain my rationale for that change.
>
> OpaqueValueExpr is intended to represent a value that is computed once but used in multiple places; the simplest example I've seen is for BinaryConditionalOperator (a GNU extension):
>
> int *x = y ?: z;
>
> In this expression, the condition of the operator is an ImplicitCastExpr (PointerToBool) around a DeclRefExpr (y), and the true-result is just the DeclRefExpr.* We don't want to compute the same value twice because of side effects, and nor do we want to traverse the source expression twice with a RecursiveASTVisitor, so we use OpaqueValueExpr to represent the evaluation of 'y'. In this particular case, it's not so clear whether the "canonical" parent of the DeclRefExpr is the ImplicitCastExpr or the BinaryConditionalOperator.
>
> * There is more going on here (LValueToRValue conversion, for one thing), but I'm doing this from memory and trying to keep it simple.
I'm concerned about instances in the AST with mutliple parents. I know 
that clang's AST represents more than just a literal AST. However I 
wouldn't go so far and allow multiple parents in the AST. That said, the 
IMHO correct solution is to remove getCond and getTrueExpr from 
AbstractConditionalOperator and handle the LHS of 
BinaryConditionalOperator differently. But according to the comments I 
don't think that my problem stems from OpaqueValueExpr.

> A PseudoObjectExpr is used when we want one expression to essentially rewrite to another one.
Hmm, I did a textual search for PseudoObjectExpr, but haven't found it 
anywhere but in the one comment in ParentMap.cpp. Looks as if it is gone 
again. Maybe in that case the first part (causing the troubles) of 
BuildParentMap can be reverted anyway(?)

Best regards
Olaf