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

Jordan Rose jordan_rose at apple.com
Fri Sep 21 09:18:24 PDT 2012


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.

A PseudoObjectExpr is used when we want one expression to essentially rewrite to another one. The only use right now is for properties and object subscripting in Objective-C. For example, 'self.foo = bar' is rewritten to be '[self setFoo:bar]'. In this case we have the notion of a "syntactic" form and one or more "semantic" statements that are actually emitted by CodeGen.

Since the analyzer is using this ParentMap for diagnostic emission, we definitely want to make the canonical parent of 'bar' be the assignment operation that the user actually wrote, rather than the message send that will be executed. But we'd still like to be able to say that the message send is a child of the larger PseudoObjectExpr. As long as the semantic form comes first, the "don't update the map again" idea would be correct.

ANYWAY, after all that, it seems like a correct solution, if slightly more expensive, would be to keep track of which statements have been seen on this particular traversal. Another possibility is that there could be an explicit test for PseudoObjectExpr to make sure the semantic form is traversed last instead of first. Do you (or does anybody else) have opinions?

Jordan


On Sep 21, 2012, at 7:46 , Olaf Krzikalla <Olaf.Krzikalla at tu-dresden.de> wrote:

> Hi @clang,
> 
> the comment to ParentMap::addStmt states, that the parent relation of a Stmt can be _updated_ by a call to it (and I use it that way). r161350 breaks this functionality. Someone should have a look at this since I don't know how to deal with PseudoObjectExprs.
> 
> Best regards
> Olaf




More information about the cfe-dev mailing list