On Sep 21, 2012, at 9:24 AM, Jordan Rose <[email protected]> wrote:
> > On Sep 20, 2012, at 23:00 , Ted Kremenek <[email protected]> wrote: > >>> >>> >>>> } >>>> >>>> AnalysisDeclContext *AnalysisDeclContextManager::getContext(const Decl *D) >>>> { >>>> + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { >>>> + FD->hasBody(FD); >>>> + D = FD; I think FD->hasBody(D); + a comment will make this more readable. >>>> + } >>> >>> So there is apparently a Decl::getBody that does the right thing (i.e. >>> returns 0 if there is no body). Can we just use that? >> >> We're not trying to get the body here. We're trying to update the D to be >> the one that has the body (if one exists), and use that as the Decl* we use >> to construct the AnalysisDeclContext. Maybe getBody() looks cleaner, but it >> functionality isn't really any different. > > Oops, right. What I really want is getDefinition, which currently doesn't > exist, but easily could. > > >> Note that apparently not updating 'D' here correctly broke a ton of stuff. > > Yes. Yes it does. > > > >>> >>>> + // Everything checks out. Create a fake body that just calls the block. >>>> + // This is basically just an AST dump of: >>>> + // >>>> + // void dispatch_sync(dispatch_queue_t queue, void (^block)(void)) { >>>> + // block(); >>>> + // } >>>> + // >>>> + DeclRefExpr *DR = DeclRefExpr::CreateEmpty(C, false, false, false, >>>> false); >>>> + DR->setDecl(const_cast<ParmVarDecl*>(PV)); >>>> + DR->setValueKind(VK_LValue); >>>> + ImplicitCastExpr *ICE = ImplicitCastExpr::Create(C, Ty, >>>> CK_LValueToRValue, >>>> + DR, 0, VK_RValue); >>>> + CallExpr *CE = new (C) CallExpr(C, ICE, ArrayRef<Expr*>(), C.VoidTy, >>>> + VK_RValue, SourceLocation()); >>>> + return CE; >>>> +} >>> >>> The name for ArrayRef<Expr*>() is >> >> Hmm? > > Oops. I meant to go look up a nice alias because the template is ugly, and in > fact MultiExprArg is compatible (it's MutableArrayRef<Expr*>), but this does > match the form in CallExpr's constructor. > > >>> It is probably not a good idea to make a function body that isn't a >>> CompoundStmt or CXXTryStmt. >> >> Why? I know how the CFGBuilder works. This isn't an issue. The CFGBuilder >> just flattens out the CompoundStmt. That concept doesn't even exist in the >> CFG. >> >>> Right now I don't think anything depends on it besides the diagnostics, >>> which don't apply here, but still. > > I guess if it's ONLY diagnostics that depend on it, it doesn't matter. > (PathDiagnosticLocation::createDeclBegin has a FIXME about 'try'.) > > > >> >>> >>> >>>> switch (piece->getKind()) { >>>> case PathDiagnosticPiece::Call: { >>>> PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece); >>>> @@ -142,8 +148,17 @@ >>>> } >>>> // Recursively clean out the subclass. Keep this call around if >>>> // it contains any informative diagnostics. >>>> - if (!RemoveUneededCalls(call->path, R)) >>>> + PathDiagnosticCallPiece *NewCallWithLoc = >>>> + call->getLocation().asLocation().isValid() >>>> + ? call : CallWithLoc; >>> >>> I think there should actually be an assertion here -- only inlined >>> functions can be synthesized, so there will always be a non-synthesized >>> call with a valid location. >> >> Not true. The synthesized function itself can call other functions. That >> call doesn't have a valid location. > > Right. I meant that if !call->getLocation().asLocation().isValid(), assert > CallWithLoc. Or rather, just assert NewCallWithLoc after this, and then > remove one of the tests from the 'if' below... > > >>> >>>> + if (!RemoveUneededCalls(call->path, R, NewCallWithLoc)) >>>> continue; >>>> + >>>> + if (NewCallWithLoc == CallWithLoc && CallWithLoc) { >>>> + call->callEnter = CallWithLoc->callEnter; >>>> + } >>> >>> Extra braces? Also, when does callExit get set? >> >> Good point. We'll need a way to propagate back the call exit location from >> the bottom. > > Well, in the normal traversal, the exit information gets set first. In > RemoveUneededCalls (hm, we have a typo there), we already have both entry and > exit information, so we should be able to just set it here as well. > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
