On May 17, 2013, at 19:45 , Matt Beaumont-Gay <[email protected]> wrote:
> On Fri, May 17, 2013 at 7:27 PM, Jordan Rose <[email protected]> wrote: >> Author: jrose >> Date: Fri May 17 21:27:09 2013 >> New Revision: 182187 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=182187&view=rev >> Log: >> [analyzer] "Fix" ParentMap to handle non-syntactic OpaqueValueExprs. >> >> Constructs like PseudoObjectExpr, where an expression can appear more than >> once in the AST, use OpaqueValueExprs to guard against inadvertent >> re-processing of the shared expression during AST traversal. The most >> common form of this is to share expressions between the syntactic >> "as-written" form of, say, an Objective-C property access 'obj.prop', and >> the underlying "semantic" form '[obj prop]'. >> >> However, some constructs can produce OpaqueValueExprs that don't appear in >> the syntactic form at all; in these cases the ParentMap wasn't ever >> traversing >> the children of these expressions. This patch fixes that by checking to see >> if an OpaqueValueExpr's child has ever been traversed before. There's also a >> bit of reset logic when visiting a PseudoObjectExpr to handle the case of >> updating the ParentMap, which some external clients depend on. >> >> This still isn't exactly the right fix because we probably want the parent >> of the OpaqueValueExpr itself to be its location in the syntactic form if >> it's syntactic and the PseudoObjectExpr or BinaryConditionalOperator itself >> if it's semantic. Whe I originally wrote the code to do this, I didn't >> realize >> that OpaqueValueExprs themselves are shared in the AST, not just their source >> expressions. This patch doesn't change the existing behavior so as not to >> break anything inadvertently relying on it; we'll come back to this later. >> >> Modified: >> cfe/trunk/lib/AST/ParentMap.cpp >> >> Modified: cfe/trunk/lib/AST/ParentMap.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ParentMap.cpp?rev=182187&r1=182186&r2=182187&view=diff >> ============================================================================== >> --- cfe/trunk/lib/AST/ParentMap.cpp (original) >> +++ cfe/trunk/lib/AST/ParentMap.cpp Fri May 17 21:27:09 2013 >> @@ -33,6 +33,11 @@ static void BuildParentMap(MapTy& M, Stm >> assert(OVMode == OV_Transparent && "Should not appear alongside OVEs"); >> PseudoObjectExpr *POE = cast<PseudoObjectExpr>(S); >> >> + // If we are rebuilding the map, clear out any existing state. >> + if (M[POE->getSyntacticForm()]) >> + for (Stmt::child_range I = S->children(); I; ++I) >> + M[*I] = 0; >> + >> M[POE->getSyntacticForm()] = S; >> BuildParentMap(M, POE->getSyntacticForm(), OV_Transparent); >> >> @@ -62,13 +67,19 @@ static void BuildParentMap(MapTy& M, Stm >> >> break; >> } >> - case Stmt::OpaqueValueExprClass: >> - if (OVMode == OV_Transparent) { >> - OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S); >> + case Stmt::OpaqueValueExprClass: { >> + // FIXME: This isn't correct; it assumes that multiple OpaqueValueExprs >> + // share a single source expression, but in the AST a single >> + // OpaqueValueExpr is shared among multiple parent expressions. >> + // The right thing to do is to give the OpaqueValueExpr its syntactic >> + // parent, then not reassign that when traversing the semantic >> expressions. >> + OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(S); >> + if (OVMode == OV_Transparent || !M[OVE->getSourceExpr()]) { > > Some unicode garbage snuck in here... Sorry about that! I just looked and Clang actually gave me a warning ("treating Unicode character as whitespace"). Unicode character support continues to haunt me even now. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
