Yeah, I don't like this approach as much as I thought I would. My original
reason for making a new summary out of an old one was to re-use the existing
summary evaluation code. This eliminates that, and processSummaryOfInlined is
crippled since it only handles StopTrackingHard and NoRetHard. And there's a
lot of duplication elsewhere.
I'd rather see this:
RetainSummaryManager &Summaries = getSummaryManager(C);
const RetainSummary *Summ = Summaries.getSummary(Call, C.getState());
if (C.wasInlined && !Summ->appliesToInlined())
return;
checkSummary(*Summ, Call, C);
The only difference between the "hard" and "soft" versions is that NoRetHard
means /remove/ RefBindings, while NoRet means don't touch them. This could be
renamed to "Untracked" or "UntrackedSymbol".
- void updateSummaryForCall(const RetainSummary *&Summ,
- const CallEvent &Call);
+ void updateForCallbackArgument(const RetainSummary *&Summ,
+ const CallEvent &Call);
This is not a core point about the functionality of this patch, but I think
this is an inferior interface. The idea behind 'updateSummaryForCall' is that
it takes a context-less summary derived from an ObjCMethodDecl, or even just a
selector, and adds the context of the arguments and possibly additional
information about the receiver besides its type. The summary generation before
this point can be cached and re-used if we call the same method later, but
updateSummaryForCall is the time to make changes specific to this CallEvent.
Currently, the only such change is the presence or absence of a callback
parameter, but there's no reason that can't change in the future. Keeping this
as 'updateSummaryForCall' gives a proper opaque effect: given a summary for a
given decl, update it for this particular call. getSummary shouldn't be
thinking about the various ways in which a CallEvent could affect a base
summary.
I have some spot comments, but I'll save those for the next iteration of the
patch, or perhaps tomorrow morning.
Jordan
On Aug 20, 2012, at 16:59 , Anna Zaks <[email protected]> wrote:
> This resolves retain count checker false positives that are caused by
> inlining ObjC and other methods. Essentially, if we are passing an object to
> a method with "delegate" in the selector or a function pointer as another
> argument, we should stop tracking the other parameters/return value as far as
> the retain count checker is concerned.
>
> Submitting as a patch per Jordan's request.
>
> Anna.
> <Retain_Count_Stop_Tracking_Even_If_Inlined.diff>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
