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

[Olaf Krzikalla]

Hi Jordan,

first, thanks for r164947. Now lets hope that we get the ParentMap case 
resolved.

Am 01.10.2012 19:04, schrieb Jordan Rose:
> (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.
OK, it looks as if we won't get to change this. So let us just 
concentrate on the parent map implementation.
I hope I've understood it now, but it looks to me that the problem stems 
from the last lines in addStmt, where OpaqueValueExpr::sourceExpr is 
treated like a child. Thus, the second test in the actual children loop 
should only fire, if the parent is already an OpaqueValueExpr.
What do you think of the attached simple patch? It resolves my problems 
(I have no OVEs in my code) and IMHO it also properly handles the update 
of OVEs (since these are updated unconditionally in the last three lines).
I have not tested the appropriate regressions but I have the strong 
feeling that they will work.

Best Olaf

-------------- next part --------------
Index: lib/AST/ParentMap.cpp
===================================================================
--- lib/AST/ParentMap.cpp	(revision 164365)
+++ lib/AST/ParentMap.cpp	(working copy)
@@ -26,7 +26,7 @@
       // Prefer the first time we see this statement in the traversal.
       // This is important for PseudoObjectExprs.
       Stmt *&Parent = M[*I];
-      if (!Parent) {
+      if (!Parent || !isa<OpaqueValueExpr>(Parent)) {
         Parent = S;
         BuildParentMap(M, *I);
       }