On Fri, May 17, 2013 at 9:52 PM, Jordan Rose <[email protected]> wrote: > > 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.
I noticed because of that warning and -Werror :) _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
