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;
>>> + }
>>
>> 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
