[cfe-dev] r161350 breaks the intention of ParentMap::addStmt
rjmccall at apple.com
Wed Sep 26 23:40:52 PDT 2012
On Sep 26, 2012, at 11:32 AM, Jordan Rose wrote:
> On Sep 25, 2012, at 20:14 , Philip Craig <philipjcraig at gmail.com> wrote:
>> On Wed, Sep 26, 2012 at 2:18 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>>> Sorry, it isn't PseudoObjectExpr that's the problem; it's OpaqueValueExpr. (I just realized that now, meaning my comment is wrong.) The source expr of an OpaqueValueExpr isn't directly accessible as a child to avoid multiple traversal, but the rest of that commit taught ParentMap how to look through an OpaqueValueExpr. That probably isn't the right thing to do.
>>> To illustrate the problem, here are the pieces inside 'self.property = value' (roughly):
>>> 0: $1.property = $2
>>> 1: 'self'
>>> 2: 'value'
>>> 3: [$1 setProperty:$2]
>>> $1 and $2 represent OpaqueValueExprs that refer back to the real expressions in child slots 1 and 2. Child slot 0 is the "syntactic" form and is not used by CodeGen or the analysis engine, but it /is/ used for diagnostics. For the analyzer to print the best diagnostics, we want the parent expr of 'value' to be the assignment expression (0), and the parent expr of 'self' to be the ObjCPropertyRefExpr /inside/ the assignment ($1.property). Having the parent be the entire PseudoObjectExpr is possible but less than ideal.
>> Is there a reason why the syntactic form is split up like that? Is it
>> possible to have only two child expressions for PseudoObjectExpr,
>> syntactic and semantic, where the syntactic form doesn't have any
>> OpaqueValueExprs, and the OpaqueValueExprs in the semantic form point
>> within the syntactic form. If this was possible, then the ParentMap
>> wouldn't need any special handling for OpaqueValueExpr at all. (I'm
>> not familiar with codegen or the use of PseudoObjectExpr, so this
>> sugggestion may be way off.)
> I'm not 100% sure, but I believe it's because this way the CodeGen for a PseudoObjectExpr just runs through the semantic exprs, and never has to look inside any OpaqueValueExprs. By the time an OpaqueValueExpr is reached in the CodeGen traversal, it is guaranteed to have already been evaluated. The analyzer does the same thing.
> I believe John is the one who designed PseudoObjectExpr, so maybe he has something to add here, but I think I got the gist of it.
Philip's proposal would no doubt help some tools that cared only about
the syntactic form. Unfortunately, it would make serialization and
deserialization much more complicated, because it relies on expression
pointers matching exactly at arbitrary points in the expression hierarchy.
It would also complicate tools that wish to understand what parts of the
syntactic form are actually being evaluated.
I have heard a lot of complaints from people who find that OVEs
complicate their code. In pretty much every instance, their proposal
for making OVEs less invasive would actually have left their tool
doing the wrong thing — at best double-traversing expressions,
and often something more insidious.
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.
More information about the cfe-dev